Swiftify fixes for 6.2 #81827
Merged
Swiftify fixes for 6.2 #81827
+1,266
−479
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
|
@swift-ci please test |
hnrklssn
force-pushed
the
swiftify-fixes-6.2
branch
from
2aa3ee0 to
1d06a01
Compare
|
@swift-ci please test |
…on parameters with reference type (swiftlang#81634) Update availability for CxxSpan<->Span, fix lifetimebound on parameters with reference type Because swift-ide-test doesn't care about typechecking, std-span-interface.swift passed despite containing 2 separate errors. This updates the test file to properly exercise the entire compilation pipeline for the macro expansions, by running swift-frontend -emit-module and calling each macro expansion. The first issue was that CxxSpan initializers taking [Mutable]Span still had their availability set to Swift 6.2+, even after back-deploying caused [Mutable]Span to have availability back to Swift 5.0. Since _SwiftifyImport expansions copy the availbility of Span, this resulted in the macro expansions calling unavailable initializers. Interestingly enough, this manifested itself in the form of a tripped assert in SIL verification, because although we do now typecheck the expansions from _SwiftifyImport, the compilation can still keep going after `shouldEmitFunctionBody` returns false: the macro expansion declaration is still there, but is now missing its definition, despite not being external. The second issue was when parameters with C++ reference types were annotated with `[[clang::lifetimebound]]`. For parameters with a type that is `Escapable`, this is normally done using `@lifetime(borrow foo)`. However C++ reference parameters are imported as `inout`, which requires the `@lifetime(&foo)` syntax. rdar://151493400 rdar://151678415 (cherry picked from commit 262a53f)
Parameters can be named with keywords without escaping, because it's unambiguous in the grammar that they are parameters. They still need to escaped when referred to inside the function body however. This escapes all references to parameters using backticks. Parameter names are also checked for clashes with the function name - in such cases the parameter is renamed in the same way as unnamed parameters. rdar://151024645 (cherry picked from commit ebe2c60)
…lang#81579) Previously we would only add @_disfavoredOverload if the only type changed was the return type, because in any other case it is unambiguous which overload to call. However it is still ambiguous when storing the function as a value rather than calling the function, unless explicit type annotations are used. To avoid breaking any existing code, this patch adds @_disfavoredOverload to every overload generated by @_SwiftifyImport. rdar://151206394 (cherry picked from commit 0f312ad)
) Previously we did not remove count parameters if any count parameters were shared between count expressions, or if any count expression contained operations. Buffer sizes were also just checked to be larger than or equal than the given count. We now extract the count from Spans/BufferPointers whenever possible, and store that value in a variable at the start of the function. If multiple parameters share the same count, a bounds check is emitted to make sure that they have the same size. Subspans can be used if one span is larger than necessary. The message in the bounds check is changed so that it includes the expected and actual value, to aid in debugging. This patch also fixes some incorrect indentation, and adds the Whitespace.swift test case to act as a regression test in case the indentation changes, since the other test cases don't use significant whitespace. rdar://151488820 rdar://151511090 rdar://146333006 rdar://147715799 (cherry picked from commit f5fa481)
hnrklssn
force-pushed
the
swiftify-fixes-6.2
branch
from
1d06a01 to
58366a8
Compare
|
@swift-ci please test |
DougGregor
approved these changes
Jun 4, 2025
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.
This is stacked on top of #81640 to avoid merge conflicts. The changes in that PR are not included in the rest of the description.
This contains the results of multiple PRs, because they would all result in merge conflicts with each other if they weren't stacked.
CxxSpan initializers taking [Mutable]Span still
had their availability set to Swift 6.2+, even after back-deploying
caused [Mutable]Span to have availability back to Swift 5.0. Since
_SwiftifyImport expansions copy the availbility of Span, this resulted
in the macro expansions calling unavailable initializers.
For parameters with a type
that is
Escapable, their lifetime is normally borrowed using@lifetime(borrow foo). However C++ reference parameters are imported asinout, whichrequires the
@lifetime(&foo)syntax. Update _SwiftifyImport to use this syntax when applicable.Parameters can be named with keywords without escaping, because it's
unambiguous in the grammar that they are parameters. They still need to
escaped when referred to inside the function body however. This test parses parameter names, and escapes references to parameters using backticks when they are keywords.
Previously we would only add @_disfavoredOverload if the only type
changed was the return type, because in any other case it is unambiguous
which overload to call. However it is still ambiguous when storing the
function as a value rather than calling the function, unless explicit
type annotations are used.
To avoid breaking any existing code we now add
@_disfavoredOverload to every overload generated by @_SwiftifyImport.
Previously we did not remove count parameters if any count parameters
were shared between count expressions, or if any count expression
contained operations. Buffer sizes were also just checked to be larger
than or equal than the given count.
We now extract the count from Spans/BufferPointers whenever possible,
and store that value in a variable at the start of the function. If
multiple parameters share the same count, a bounds check is emitted to
make sure that they have the same size. Subspans can be used if one span
is larger than necessary.
Scope:
These are important changes to make sure that we don't break existing code once SafeInteropWrappers is enabled by default. SafeInteropWrappers cannot be enabled by default without them. The last change is important to make sure that we don't break source compatibility later by changing the signature of safe overloads.
Issues:
rdar://151493400
rdar://151678415
rdar://151024645
rdar://151206394
rdar://151488820
rdar://151511090
rdar://146333006
rdar://147715799
Original PRs:
[Swiftify] Update availability for CxxSpan<->Span, fix lifetimebound on parameters with reference type #81634
[Swiftify] Escape param decl refs #81550
[Swiftify] Always annotate overloads with @_disfavoredOverload #81579
[Swiftify] Always remove count parameters when possible #81585
Risk:
These should not affect code that does not enable SafeInteropWrappers. The main risk will be once we enable SafeInteropWrappers by default, if the fixes we've landed are not enough.
Testing:
We will test these changes extensively before enabling them by default.
Reviewers:
@j-hui
@DougGregor