Skip to content

[5.9][Macros] Fix type-checking local pattern bindings in macro-expanded closures. #65317

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 2 commits into from
Apr 20, 2023

Conversation

hborla
Copy link
Member

@hborla hborla commented Apr 20, 2023

  • Explanation: PreCheckExpr, ConstraintGenerator, and other walkers do not walk into macro expansions. However, the implementation of this macro walking behavior in ASTWalker would skip any declaration that appears inside any macro expansion buffer. This is incorrect for cases where the parent is in the same macro expansion buffer, because the local declaration is not inside a new macro expansion. This caused bogus errors when type checking expanded macro expressions containing closures with local declarations, because pre-check and constraint generation mistakenly skipped local pattern bindings.
  • Scope: Only impacts macros.
  • Issue: rdar://108148075
  • Risk: Low.
  • Testing: Added a new test case with code that previously emitted an error when type checking a closure inside a macro expansion.
  • Reviewer: @DougGregor @bnbarham
  • Main branch PR: [Macros] Fix type-checking local pattern bindings in macro-expanded closures. #65313

hborla added 2 commits April 20, 2023 08:12
…rations

inside closures while type checking a macro expansion.

PreCheckExpr, ConstraintGenerator, and other walkers do not walk into macro
expansions. However, the implementation of this macro walking behavior in
ASTWalker would skip any declaration that appears inside any macro expansion
buffer. This is incorrect for cases where the parent is in the same macro
expansion buffer, because the local declaration is not inside a new macro
expansion. This caused bogus errors when type checking expanded macro expressions
containing closures with local declarations, because pre-check and constraint
generation mistakenly skipped local pattern bindings.

(cherry picked from commit 265c8a4)
This method was misleading. The majority of callers (all but one!) don't want
to unconditionally treat all locations in any macro expansion buffer the
same way, because the code also must handle nested macro expansions. There
is one part of SourceKit (and possibly others) that really do want to ignore
all macro expansions, but those can be handled within SourceKit / IDE code,
because I don't believe this utility is useful in the frontend.

(cherry picked from commit b958e43)
@hborla hborla requested a review from a team as a code owner April 20, 2023 15:15
@hborla
Copy link
Member Author

hborla commented Apr 20, 2023

@swift-ci please test

@hborla hborla merged commit c78491e into swiftlang:release/5.9 Apr 20, 2023
@hborla hborla deleted the 5.9-fix-macro-expansion-walking branch April 20, 2023 21:16
@AnthonyLatsis AnthonyLatsis added the 🍒 release cherry pick Flag: Release branch cherry picks label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants