Introduce a new Initializer subclass for the arguments of custom attributes #78030
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
DougGregor
requested review from
hborla,
xedin,
ahoppen,
bnbarham,
CodaFi,
hamishknight,
rintaro and
slavapestov
as code owners
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
hamishknight
approved these changes
Dec 6, 2024
slavapestov
reviewed
Dec 6, 2024
| assert(varDC == CS.DC || (varDC && isa<AbstractClosureExpr>(varDC)) && | ||
| assert((varDC == CS.DC || | ||
| (varDC && isa<AbstractClosureExpr>(varDC)) || | ||
| varDC->isChildContextOf(CS.DC)) && |
There was a problem hiding this comment.
can varDC ever be null? If it can then you'll crash here, otherwise if not, you don't need the null check before the isa
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
…ibutes Since the introduction of custom attributes (as part of property wrappers), we've modeled the context of expressions within these attributes as PatternBindingInitializers. These PatternBindingInitializers would get wired in to the variable declarations they apply to, establishing the appropriate declaration context hierarchy. This worked because property wrappers only every applied to---you guessed it!---properties, so the PatternBindingInitializer would always get filled in. When custom attributes were extended to apply to anything for the purposes of macros, the use of PatternBindingInitializer became less appropriate. Specifically, the binding declaration would never get filled in (it's always NULL), so any place in the compiler that accesses the binding might have to deal with it being NULL, which is a new requirement. Few did, crashes ensued. Rather than continue to play whack-a-mole with the abused PatternBindingInitializer, introduce a new CustomAttributeInitializer to model the context of custom attribute arguments. When the attributes are assigned to a declaration that has a PatternBindingInitializer, we reparent this new initializer to the PatternBindingInitializer. This helps separate out the logic for custom attributes vs. actual initializers. Fixes swiftlang#76409 / rdar://136997841
…operties We don't need to share them, and it's far simpler if we don't.
DougGregor
force-pushed
the
custom-attribute-initializer-declcontext
branch
from
d72515a to
02a20de
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
rintaro
reviewed
Dec 7, 2024
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
Blah, silly typos crept in. One more round... |
Thank you again, Rintaro
DougGregor
force-pushed
the
custom-attribute-initializer-declcontext
branch
from
7db46b7 to
d004d24
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
|
Thank you! This will unblock using certain API patterns in Swift Testing. |
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.
Since the introduction of custom attributes (as part of property wrappers), we've modeled the context of expressions within these attributes as PatternBindingInitializers. These
PatternBindingInitializers would get wired in to the variable declarations they apply to, establishing the appropriate declaration context hierarchy. This worked because property wrappers only every applied to---you guessed it!---properties, so the
PatternBindingInitializer would always get filled in.
When custom attributes were extended to apply to anything for the purposes of macros, the use of PatternBindingInitializer became less appropriate. Specifically, the binding declaration would never get filled in (it's always NULL), so any place in the compiler that accesses the binding might have to deal with it being NULL, which is a new requirement. Few did, crashes ensued.
Rather than continue to play whack-a-mole with the abused PatternBindingInitializer, introduce a new CustomAttributeInitializer to model the context of custom attribute arguments. When the attributes are assigned to a declaration that has a PatternBindingInitializer, we reparent this new initializer to the PatternBindingInitializer. This helps separate out the logic for custom attributes vs. actual initializers.
Fixes #76409 / rdar://136997841