-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.9] SILGen and SIL type lowering for vanishing tuples #64894
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
rjmccall
merged 8 commits into
swiftlang:release/5.9
from
rjmccall:vanishing-tuple-silgen-5.9
Apr 6, 2023
Merged
[5.9] SILGen and SIL type lowering for vanishing tuples #64894
rjmccall
merged 8 commits into
swiftlang:release/5.9
from
rjmccall:vanishing-tuple-silgen-5.9
Apr 6, 2023
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
AbstractionPattern. The old logic was wrong for vanishing tuples: a non-tuple substituted type might correspond to multiple lowered parameters if it's the result of substituting into a vanishing tuple that also contains empty pack expansions. Test to follow later in this PR.
Fixes a host of problems with stored type members with types dependent on a type parameter pack; I ran into it specifically with vanishing tuples. Test to follow.
Test to follow.
Fixes rdar://107459964 and rdar://107478603.
hborla
approved these changes
Apr 4, 2023
@swift-ci Please test |
@swift-ci Please clean test macOS |
@swift-ci Please test macOS |
I guess the name of the game is to just keep building until I get a non-broken builder |
@swift-ci Please test macOS |
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 language says that
( type-tuple-body )
is a tuple type only iftype-tuple-body
is not a single non-expansion component; otherwise it is merely parentheses. We follow this same idea in substitution: substitution of type parameter packs can change the number of components in a tuple, and the result is no longer a tuple if there is a single non-expansion component. These "vanishing" tuples pose an annoyance for the lowering of types and expressions: lowering must often consider the unsubstituted type of declarations (the "abstraction pattern"), and it has historically been true that the substituted type will be a tuple if the unsubstituted type is. There is therefore quite a bit of code in lowering that assumes that e.g. it can check whether the substituted type is a tuple before it checks the unsubstituted type. This code must be fixed to be primarily driven by the unsubstituted type. (The original tuple structure is usually very significant! For example, if we take a formal parameter of type(Int, repeat each T)
, we will actually have two lowered parameters, one of which completely disappears from the substituted type ifT
is substituted with an empty pack.)5.9 version of #64887
Explanation: as above
Scope: widespread crashes in SILGen when substitution turns a tuple containing pack expansions into an underlying element
Issue: rdar://107459964 and rdar://107478603
Risk: Low; the patch touches many common code paths, but the changes are modest and should only affect code using pack expansions
Testing: Regression tests added