[AST] Bring BoundNameAliastype up to parity with NameAliasType #15499
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
Serialization can’t handle them, and I’m not up for fixing it just yet.
This doesn't have a specific effect now, because all of these places are likely to only see NameAliasType, but it is refactoring with the intent of eliminating NameAliasType entirely.
With the exception of “has type variable”, which affects the arena used for storage of a BoundNameAliasType, only propagate recursive properties from the underlying type to a BoundNameAliasType, because the other properties (e.g., “has archetype” or “has type parameter”) pulled from syntactic sugar don’t apply.
When we’re performing type transformations involving types with type parameters or archetypes, drop the sugar rather than performing potentially-harmful transformations on it.
We currently accept code that refers to `ProtocolName.TypeName` when `TypeName` is a typealias whose underlying type doesn’t refer to any type parameters, which is awful. We should ban it, but until we do… don’t represent the protocol in sugar or form substitutions, because they won’t work.
The generic signature used to perform substitutions for a BoundNameAliasType in ill-formed code might not line up with the generic signature in the typealias. Separately record the signature we used to build the BoundNameAliasType to make the AST more robust against such issues. And, of course, we should always profile all of the fields, lest we run into odd unintentional collisions.
|
This pull request contains everything from #15497 except flipping the final switch over to |
|
@swift-ci please smoke test and merge |
|
@swift-ci please test source compatibility |
|
Prior source-compatibility test with all of these changes passed, but... better safe than sorry. |
|
@swift-ci please smoke test macOS |
|
The LLDB failure might be because LLDB doesn't know anything about |
|
Closing this out; all of these commits are part of #15497, which avoids the LLDB issue by eventually renaming |
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.
Fix a number of small issues with
BoundNameAliasTypeto bring it up to parity withNameAliasType, so the former will be able to replace the latter. This includes:BoundNameAliasTypes profiling (forFoldingSet) and formation more robust against ill-formed ASTsBoundNameAliasTypeeverywhere that we handle aNameAliasType, including for special cases where we currently always get aNameAliasType. This part is pure staging.