[Swiftify] Support __sized_by on byte-sized pointee types #81752
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
hnrklssn
requested review from
delcypher,
Xazax-hun,
DougGregor,
egorzhdan,
j-hui and
susmonteiro
hnrklssn
requested review from
bnbarham,
hamishknight,
rintaro,
zoecarver,
fahadnayyar,
beccadax and
ian-twilightcoder
as code owners
|
@swift-ci please smoke test |
hnrklssn
force-pushed
the
swiftify-sized-sizedby
branch
from
5b5f5dd to
f3ff249
Compare
|
@swift-ci please smoke test |
Previously we would emit a macro that would error on expansion when trying to add a safe wrapper to a function with __sized_by on a type that mapped to UnsafePointer<T> instead of UnsafeRawPointer or OpaquePointer. __sized_by is acceptable when used on byte-sized pointee types, so this adds machinery in the macro expansion to support that. Meanwhile on the ClangImporter side, we add a check so that __sized_by on pointee types with a size is ignored if that size is larger than 1 byte. When _SwiftifyImport applies .sizedBy to a pointer of type UnsafePointer<T> it will still map it to a RawSpan/UnsafeRawBufferPointer in the safe overload. The assumption is that any API using __sized_by is dealing with raw bytes, so raw pointers are a better Swift abstraction than UnsafePointer<CChar> etc. It also lets the user avoid doing a scary pointer cast from some potentially larger-than-byte-sized pointer to a byte-sized pointer. Casts to RawPointers are generally safer and more ergonomic. rdar://150966684 rdar://150966021
This enables -strict-memory-safety -warnings-as-errors on the Swift side to verify that the macro expansions don't cause any warnings and that they use `unsafe` correctly. On the clang side it enables -Xcc -Werror. To reduce noise in the test output and pass -Werror cleanly it also enables -Xcc -Wno-nullability-completeness. This will make it easier to detect mistakes when writing tests, because warnings stand out whereas previously they've been drowned out in the noise.
hnrklssn
force-pushed
the
swiftify-sized-sizedby
branch
from
f3ff249 to
4a72c1e
Compare
|
@swift-ci please smoke test |
hnrklssn
commented
May 29, 2025
|
@swift-ci please smoke test |
Xazax-hun
reviewed
Jun 4, 2025
|
@swift-ci please smoke test |
delcypher
approved these changes
Jun 4, 2025
| let type = peelOptionalType(getParam(signature, index).type) | ||
| if type.canRepresentBasicType(type: OpaquePointer.self) { | ||
| return ExprSyntax("OpaquePointer(\(baseAddress))") | ||
| } | ||
| if isSizedBy { | ||
| if let pointeeType = getPointeeType(type) { |
There was a problem hiding this comment.
Are there any cases where isSizedBy is true but getPointeeType fails?
There was a problem hiding this comment.
The only cases I'm aware of are:
- if it's applied to an
OpaquePointer, which has an early exit above - if clang would stop stripping typedef'd pointer types when applying
__sized_by - if it's already applied to an
UnsafeRawPointer
Otherwise it should always be anUnsafe[Mutable]Pointer<T>. If it's not,getPointeeTypewill just return nil
Xazax-hun
approved these changes
Jun 5, 2025
hnrklssn
added a commit
to hnrklssn/swift
that referenced
this pull request
Jun 10, 2025
[Swiftify] Support __sized_by on byte-sized pointee types (cherry picked from commit 89b09a6)
hnrklssn
added a commit
to hnrklssn/swift
that referenced
this pull request
Jun 10, 2025
[Swiftify] Support __sized_by on byte-sized pointee types (cherry picked from commit 89b09a6)
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.
[Swiftify] Support __sized_by on byte-sized pointee types
Previously we would emit a macro that would error on expansion when
trying to add a safe wrapper to a function with __sized_by on a type
that mapped to UnsafePointer instead of UnsafeRawPointer or
OpaquePointer. __sized_by is acceptable when used on byte-sized pointee
types, so this adds machinery in the macro expansion to support that.
Meanwhile on the ClangImporter side, we add a check so that __sized_by
on pointee types with a size is ignored if that size is larger than 1
byte.
When _SwiftifyImport applies .sizedBy to a pointer of type
UnsafePointer it will still map it to a
RawSpan/UnsafeRawBufferPointer in the safe overload. The assumption is
that any API using __sized_by is dealing with raw bytes, so raw pointers
are a better Swift abstraction than UnsafePointer etc. It also
lets the user avoid doing a scary pointer cast from some potentially
larger-than-byte-sized pointer to a byte-sized pointer. Casts to
RawPointers are generally safer and more ergonomic.
rdar://150966684
rdar://150966021
[Swiftify] Enable warnings-as-errors in interop test cases (NFC)
This enables -strict-memory-safety -warnings-as-errors on the Swift side
to verify that the macro expansions don't cause any warnings and that
they use
unsafecorrectly. On the clang side it enables -Xcc -Werror.To reduce noise in the test output and pass -Werror cleanly it also
enables -Xcc -Wno-nullability-completeness. This will make it easier to
detect mistakes when writing tests, because warnings stand out whereas
previously they've been drowned out in the noise.