-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Swift bridging] Consistently use correctly-defined Swift(U)Int for bridging #68278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The C type tha corresponds to Swift's pointer-sized `Int` and `UInt` types varies from one platform to the next. The canonical C types are `ptrdiff_t` and `size_t`, but we're in the depths of the compiler we can't include the C library headers that provide them because they introduce cyclic module dependencies. Sigh. SwiftShims has some logic to compute these types. However, SwiftShims is part of the Swift runtime, not the compiler, so those headers cannot be included here. So, we clone the logic and simplify it somewhat for our use case. This fixes truncation issues on Windows, where the uses of `unsigned long` and `long` for Swift(U)Int are incorrect.
@swift-ci please smoke test |
compnerd
reviewed
Sep 1, 2023
…t/UInt 64-bit Windows defines both _WIN64 and _WIN32, so the logic here would always end up defining 32-bit C types for Swift's `Int` and `UInt`. Fix the ordering to check for 64-bit first, then 32-bit second. Note that the SwiftShims version of this code has always been wrong, but it's completely benign because SwiftShims is only used in the Swift runtime itself, which is built with Clang (on all platforms), and doesn't need to go through this code path. Still, we fix it in both places, so we don't get a nasty surprise if the SwiftShims version of the header later gets included in a non-Clang C++ compiler.
@swift-ci please smoke test |
This was referenced Sep 1, 2023
compnerd
approved these changes
Sep 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks again @DougGregor!
@swift-ci please smoke test |
compnerd
reviewed
Sep 1, 2023
Co-authored-by: Saleem Abdulrasool <[email protected]>
@swift-ci please smoke test |
compnerd
reviewed
Sep 2, 2023
Co-authored-by: Saleem Abdulrasool <[email protected]>
@swift-ci please smoke test |
The type definitions do not uniformly import as `Swift.Int` and `Swift.UInt`. Add some casts to accommodate the type conversions.
@swift-ci please smoke test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
SwiftInt
andSwiftUInt
types in the compiler provideC
equivalents to Swift'sInt
andUInt
. Consistently use these types in the C/Swift bridging layer, to avoid painful truncations and data-size mismatches.This is harder than it should be. The canonical C types are
ptrdiff_t
andsize_t
, but we're in the depths of the compiler we can't include the C library headers that provide them because they introduce cyclic module dependencies. Sigh.SwiftShims has some logic to compute these types. However, SwiftShims is part of the Swift runtime, not the compiler, so those headers cannot be included here. So, we clone the logic and simplify it somewhat for our use case.
This fixes truncation issues on Windows, where the uses of
unsigned long
andlong
for Swift(U)Int are incorrect.