[AST] A couple of PatternBindingInitializer cleanups #70591
Merged
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
hamishknight
force-pushed
the
doritos-and-fritos
branch
from
693a1b4 to
806beef
Compare
hamishknight
force-pushed
the
doritos-and-fritos
branch
from
806beef to
3d3b739
Compare
This stores the same state as PatternBindingInitializer, we can use that instead.
This stores the same state as DefaultArgumentInitializer, use that instead.
Switch from promising a DeclContext to a PatternBindingInitializer. This has a couple of benefits: - It eliminates a few places where we were force `cast`'ing to PatternBindingInitializer. - It improves the clarity of what's being stored, it's not whatever the parent context of the initializer is, it's specifically the PatternBindingInitializer context if it exists.
Use the PatternBindingInitializer context if we have one. This also uncovered a parser issue where we would mistakenly create a PatternBindingInitializer in top-level code after parsing the initializers.
Most clients only want to set one of the two parameters, split it into `setPattern` and `setInitContext` (the latter of which now handles calling `setBinding`).
And add some assertions to ensure we don't attempt to re-set the binding or index (the parent context can unfortunately change currently).
This could result in us changing the binding index after-the-fact.
hamishknight
force-pushed
the
doritos-and-fritos
branch
from
3d3b739 to
0b18d28
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
hamishknight
requested review from
hborla,
slavapestov,
xedin,
ahoppen,
bnbarham,
CodaFi,
DougGregor and
rintaro
as code owners
slavapestov
approved these changes
Jan 17, 2024
| @@ -115,8 +113,6 @@ enum class DeclContextKind : unsigned { | |||
| /// \see SerializedLocalDeclContext. | |||
There was a problem hiding this comment.
This comment is out of date. Is DeclContextKind::SerializedLocal also dead now?
There was a problem hiding this comment.
Yeah looks like we can probably just rip out SerializedLocalDeclContext at this point, I'll do that in a follow-up
CodaFi
approved these changes
Jan 17, 2024
|
Source compat failures are unrelated |
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.
Remove
SerializedPatternBindingInitializer&SerializedDefaultArgumentInitializersince we can deserializePatternBindingInitializer&DefaultArgumentInitializerdirectly (as I understand it, theSerializedXXXdecl contexts only exist for cases where we don't want to serialize e.g a fullClosureExpr/TopLevelDecl, that doesn't apply here). This then allows us to clean upPatternBindingDecl's handling ofPatternBindingInitializera bit.