Skip to content
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

[AST] Sink ParamDecl flag collection logic from Parse to AST #79503

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Feb 20, 2025

  • Collect flags in ParamDecl::setTypeRepr().

  • Update ParamSpecifierRequest::evaluate to handle non-implicit ParamDecl without TypeRepr (i.e. untyped closure parameter), instead of setSpecifier(::Default) manually in Parse.

  • (ASTGen) Separate BridgedParamDecl.setTypeRepr(_:) from BridgedParamDecl.createParsed(_:) aligning with C++ API. The majority of the creations don't set the typerepr.

@@ -276,7 +275,6 @@ extension ASTGenVisitor {
params.reserveCapacity(node.parameters.count)
for (index, node) in node.parameters.enumerated() {
let param = self.generate(closureParameter: node, at: index)
param.setSpecifier(.default)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was a mistake because this set the closure parameter specifier to .default even if the TypeRepr has been set. So closures like { (arg: inout T) in .. } wasn't correctly generated.

@rintaro
Copy link
Member Author

rintaro commented Feb 20, 2025

@swift-ci Please smoke test

@rintaro rintaro changed the title [AST] Sink the flag collection logic from Parse to AST [AST] Sink ParamDecl flag collection logic from Parse to AST Feb 20, 2025
@@ -8712,6 +8712,45 @@ ParamDecl *ParamDecl::createParsed(
return decl;
}

void ParamDecl::setTypeRepr(TypeRepr *repr) {
TyReprAndFlags.setPointer(repr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert that the type repr hasn't been set before?

* Collect flag in `ParamDecl::setTypeRepr()`.

* [ASTGen] Separate `BridgedParamDecl.setTypeRepr(_:)` from
`BridgedParamDecl.createParsed(_:)` aligning with C++ API. The majority
of the creations don't set the typerepr.

* Update `ParamSpecifierRequest::evaluate` to handle non-implicit
`ParamDecl` without `TypeRepr` (i.e. untyped closure parameter), instead
of `setSpecifier(::Default)` manually in Parse.
@rintaro rintaro force-pushed the astgen-paramdecl-typerepr branch from 86f421a to 8833ff5 Compare February 20, 2025 18:06
@rintaro
Copy link
Member Author

rintaro commented Feb 20, 2025

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Feb 20, 2025

swiftlang/swift-corelibs-foundation#5174
@swift-ci Please smoke test

@rintaro rintaro enabled auto-merge February 21, 2025 02:51
@rintaro
Copy link
Member Author

rintaro commented Feb 21, 2025

@swift-ci Please smoke test Windows

@rintaro rintaro merged commit 6805e15 into swiftlang:main Feb 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants