[Swiftify] don't unwrap pointers for UP! when not needed#89455
Open
hnrklssn wants to merge 12 commits into
Open
[Swiftify] don't unwrap pointers for UP! when not needed#89455hnrklssn wants to merge 12 commits into
hnrklssn wants to merge 12 commits into
Conversation
These mirror the existing __counted_by / __sized_by test files and document the current behavior: __counted_by_or_null and __sized_by_or_null are currently imported identically to their non-or_null counterparts. A follow-up change will differentiate the two when combined with _Nullable: __counted_by _Nullable should become a non-optional Span / UnsafeBufferPointer, while __counted_by_or_null _Nullable should remain an optional.
This makes it possible to differentiate OrNull variants from the base attributes. For now they do the same thing as the base variant, but they will diverge in a later commit. Clone the test structure of the base variants to track regressions separately.
This passes the appropriate information from ClangImporter to _SwiftifyImport for counted_by_or_null and sized_by_or_null. The generated safe wrappers are still identical to the counted_by and sized_by versions. That will be addressed in another commit.
The _SwiftifyImport macro expansion used a closure for the return value to create an expression from an if statement. This is because the constructor for Span family types requires a non-null pointer. To safely unwrap the optional pointer we handle the null case separately. This restructures the macro such that it can return the null case in an early exit if statement before the main return statement. This has no functional change for the generated code.
Safe wrappers only need lifetimes for ~Escapable types, not UnsafeBufferPointers. This has no effect on the generated code.
counted_by and sized_by can only be null if the count parameter is 0. Since Span and UnsafeBufferPointer can also have a null pointer with count 0, there is no need for an extra layer of Optional here in the safe wrapper. counted_by_or_null and sized_by_or_null still map to Optional since they are always allowed to be null, regardless of count. Differentiating between the "nil" and "empty span" cases will be important in a later commit when we implement counted_by_or_null's bound checking semantics.
A pointer annotated `__counted_by_or_null(len)` is allowed to be null even when `len` is non-zero (unlike `__counted_by`). For a basic function with a single parameter this doesn't make much of a difference for safe wrappers, but when multiple pointers sharing the same count it does. Previously we would simply extract the length from the first parameter using `p1?.count ?? 0`. However this would result in a bounds check failure if `p2` was a non-null non-empty buffer. Instead we now check all the buffers sharing a count against the first non-null buffer. If there are any non-nullable buffers to choose from we simply take the count from one of those. If all are nullable the count is extracted with a chained expression of all the buffers: `p1?.count ?? p2?.count ?? ... ?? 0`, and then the individual buffer counts are compared to that. Return pointers already correctly return nil if the underlying function returned a null pointer. rdar://177835043
CI runs with pre-Swift-6.1 compiler, so it does not support trailing commas.
Safe wrapper expansions included an explicit unwrap of the baseAddress of the span parameter unless the underlying pointer parameter was explicitly `_Nullable`. Since the baseAddress can only be null if the count is 0, and the pointer is annotated with counted_by, it is not allowed to dereference the zero-length buffer, even if the nullability is not set (i.e. the underlying function parameter is imported with an implicitly unwrapped optional type). For consistency, this also adds the same early exit handling for the return value as for nullable return values. rdar://177555277
hnrklssn
requested review from
a team,
Xazax-hun,
beccadax,
egorzhdan,
ian-twilightcoder,
j-hui,
patrykstefanski and
susmonteiro
as code owners
Member
Author
|
@swift-ci please smoke test |
Member
Author
|
This PR is based on top of #89395. Only the last commit belongs to this PR. |
Member
Author
|
@swift-ci please smoke test |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Safe wrapper expansions included an explicit unwrap of the baseAddress
of the span parameter unless the underlying pointer parameter was
explicitly
_Nullable. Since the baseAddress can only be null if thecount is 0, and the pointer is annotated with counted_by, it is not
allowed to dereference the zero-length buffer, even if the nullability
is not set (i.e. the underlying function parameter is imported with an
implicitly unwrapped optional type).
For consistency, this also adds the same early exit handling for the
return value as for nullable return values.
rdar://177555277