[Swiftify] enable safe wrappers for std::span without typedef#88621
Open
hnrklssn wants to merge 4 commits intoswiftlang:mainfrom
Open
[Swiftify] enable safe wrappers for std::span without typedef#88621hnrklssn wants to merge 4 commits intoswiftlang:mainfrom
hnrklssn wants to merge 4 commits intoswiftlang:mainfrom
Conversation
This test was missing matcher for one safe wrapper. Add --implicit-check-not to prevent this going forward.
When safe interop was stabilized, lifetimebound deliberately not stabilized. The check for lifetimebound on the implicit "this" parameter was accidentally not gated behind the experimental feature check, however. Now lifetimebound on implicit "this" is no longer in the stable feature set. rdar://175396896
We previously prevented generating safe wrappers on functions with template instantiations in the signature, unless those instantiations were hidden behind a typedef. This is because those template instantiations cannot be expressed in Swift syntax, so the macro would expand to a function with invalid syntax. For the std::span parameter itself however, that type is replaced with a Swift Span in the safe wrapper, so it's not an issue in the type signature. We would previously use the std::span type name from the original signature to cast from Span to std::span, which does not work with the raw std::span type. This uses `.init` instead when this is the case, which avoids spelling out the type. When possible, we still use the concrete type name, for clarity and by providing information we have accessible the typechecker needs to do less work. Resolves swiftlang#86339 rdar://167712240
These tests never use bridging headers or define BRIDGING_HEADER, so there's not reason for them to check for this define.
hnrklssn
changed the title
hnrklssn
requested review from
DougGregor,
Xazax-hun,
egorzhdan,
j-hui,
patrykstefanski and
susmonteiro
Member
Author
|
@swift-ci please test |
1 similar comment
Contributor
|
@swift-ci please test |
Member
Author
|
Oh I forgot to mention: this PR is based on top of #88620 |
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
2 participants
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.
We previously prevented generating safe wrappers on functions with
template instantiations in the signature, unless those instantiations
were hidden behind a typedef. This is because those template
instantiations cannot be expressed in Swift syntax, so the macro
would expand to a function with invalid syntax.
For the std::span parameter itself however, that type is replaced with a
Swift Span in the safe wrapper, so it's not an issue in the type
signature. We would previously use the std::span type name from the
original signature to cast from Span to std::span, which does not work
with the raw std::span type. This uses
.initinstead when this is thecase, which avoids spelling out the type. When possible, we still use
the concrete type name, for clarity and by providing information we have
accessible the typechecker needs to do less work.
This expands the scope of safe interop wrappers a bit: previously it was limited to std::span parameters where the type was hidden behind a typedef, which people don't typically do unless to deliberately support Swift interop. This is no longer needed. However the parameter still needs a
__noescapeannotation, which is also not very widespread outside of Swift interop, since upstream currently Clang emits an error for__noescapeonstd::span. But__noescapeis expected to see more widespread use in the future, and we are working with upstream Clang to support it onstd::span, so it'll be more realistic for future code that didn't intentionally go out of its way to support safe Swift interop to end up with safe wrappers.rdar://167712240
C++ Interop should not require "type alias"es for
std::span<->Spanconversions #86339The risk of breaking current code is low: current macro expansions are not changed, and it's unlikely for existing code to accidentally get safe wrappers. But going forward it does expand the exposure surface for safe interop, increasing the impact of regressions in safe interop wrappers.
Lit regression tests. Will run a SWB and include these changes in a sweep of OSS code before merging, but I have not done that yet.