Skip to content

[5.9] [Macros] Fix existential diagnostics for declaration macro expansions #66152

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

Merged
merged 1 commit into from
May 26, 2023

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented May 25, 2023

Cherry-pick of #66132


  • Explanation: Declarations produced by freestanding declaration macros were incorrectly rejected when they declared a conformance to a "PAT" protocol.
  • Scope: Existential type checking
  • Risk: Low. This prevented ExistentialTypeVisitor from visiting macro-expanded decls as childrens of a MacroExpansionDecl. They'd be checked as auxiliary decls by checkExistentialTypes instead, just like other decls.
  • Testing: Added test cases that cover macro expansions, nested macro expansions, macro arguments, and macro generic arguments.
  • Issue: rdar://109779928
  • Reviewers: @DougGregor

@rxwei rxwei requested a review from a team as a code owner May 25, 2023 21:31
@rxwei
Copy link
Contributor Author

rxwei commented May 25, 2023

@swift-ci please test

`TypeChecker::checkExistentialTypes` has decl-specific logic and backs out without invoking `ExistentialTypeVisitor` on `TypeDecl`s, but `MacroExpansionDecl` isn't a type decl and would fall into `ExistentialTypeVisitor` which will visit its expansion. The caller of `checkExistentialTypes` already visits auxiliary decls, so here we should only visit arguments and generic arguments of a macro.

Fixes a case where a declaration macro produces a type that conforms to a PAT. We now also run existential diagnostics on generic arguments on a `MacroExpansionDecl` that was previously not handled.

Note: I tried bailing out in `walkToDeclPre` but we should really visit macro arguments and generic arguments. Not walking expansions in `ExistentialTypeVisitor` is also not the right fix because we need to be able to visit macro expansions in arguments.

rdar://109779928
@rxwei
Copy link
Contributor Author

rxwei commented May 26, 2023

@swift-ci please test

@rxwei rxwei merged commit 3b08abb into swiftlang:release/5.9 May 26, 2023
@rxwei rxwei deleted the 109779928-59 branch May 26, 2023 18:06
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