-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[ASTGen] ParamDecl generation improvements #79456
Conversation
@swift-ci Please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup!
@@ -1515,10 +1514,6 @@ class Parser { | |||
/// all the arguments, not just those that have default arguments. | |||
unsigned claimNextIndex() { return NextIndex++; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like claimNextIndex
is unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also be nice to see if we could remove HasDefaultArgument
, seems like the diagnostic logic there could either be plumbed down or check the ParameterList itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, DefaultArgumentInfo
does mostly nothing anymore. I guess HasDefaultArgument
diagnostic can be sinked into TyoeCheckDeclPrimary
. I will try it in followups, those diagnostics are needed in ASTGen mode too.
884b0ce
to
a23e061
Compare
Rework ParamDecl contextualization.
a23e061
to
43eeb14
Compare
@swift-ci Pease smoke test |
@swift-ci Please smoke test |
@swift-ci please smoke test Linux |
1 similar comment
@swift-ci please smoke test Linux |
DefaultArgumentInitializer
decl context for the default argument value.Modified the flow in C++ parser too. It used to be:
initContext
's parent viaDefaultArgumentInitializer::changeFunction
ParamDecl.initContext
in the same functionThis seemed pretty complicated, especially when we can:
ParameterList::setDeclContextOfParamDecls
set the initContext's parent too.Generate attributes on
ParamDecl
sSimple independent change. We didn't generate and set the attributes on function parameters.