Skip to content

Name lookup and type checking fixes for freestanding declaration macros #64803

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Mar 31, 2023
Merged

Name lookup and type checking fixes for freestanding declaration macros #64803

merged 12 commits into from
Mar 31, 2023

Conversation

DougGregor
Copy link
Member

  • Explanation: Introduce two significant fixes for freestanding declaration macros, ensuring that their arguments aren't type-checked twice, and fixing name lookup within those macro arguments.
  • Scope: Affects type-checking only for freestanding declaration macros.
  • Issue: rdar://107388151
  • Risk: Low; only affects freestanding declaration macros.
  • Testing: Additional tests added.
  • Reviews: @hborla and @rxwei
  • Original pull request: [Macros] Name lookup and type checking fixes for freestanding declaration macros #64761

DougGregor added 12 commits March 30, 2023 21:03
@DougGregor
[Macros] Move most macro definition checking into ASTGen
ed07d15 
In preparation for supporting macros that are defined in terms of other
macros, adopt macro definition checking provided by the
`MacroDeclSyntax.checkDefinition` operation (implemented in
swift-syntax). Use this in lieu of the checking on the C++ side.

This required me to finally fix an issue with the source ranges for
Fix-Its, where we were replacing the leading/trailing trivia of nodes
along with the node itself, even though that's incorrect: we should
only replce the node itself, and there are other Fix-It kinds for
replacing leading or trailing trivia.

(cherry picked from commit 49277f7)
@DougGregor
Only define the various Macros features when the compiler supports them
0dd514f 
We were enabling the `$Macros` feature unconditionally, even when the
compiler itself doesn't support macros (because it's missing
swift-syntax). Change this to only enable the feature when the
compiler supports it.

(cherry picked from commit 73a2041)
@DougGregor
[Macros] Start recording expanded macro definitions and replacements
0d73fc2 
Handle a trivial macro defined in terms of another macro.

(cherry picked from commit 9292231)
@DougGregor
[Macros] Fix name lookup of macro parameters in the definition.
29f6012 
This enables macros to be defined in terms of other macros.

(cherry picked from commit 40b3321)
@DougGregor
[Traversal] Walk into macro initializers more carefully
f3e3a72 
These expressions are a little tricky, because we don't always
type-check them in the way that clients would expect. We might want to
change the representation here, but until then... don't walk the
untypechecked ones.

(cherry picked from commit aac0406)
@DougGregor
[Macros] Add (de-)serialization for expanded macro definitions
3612a35 
(cherry picked from commit 38230b7)
@DougGregor
[Macros] Add a test ensuring that we print expanded macro definitions
2f41367 
This should complete the implementation of expanded macro
definitions, rdar://104044325.

(cherry picked from commit cfbdae5)
@DougGregor
Require the Swift swift parser for a few tests affected by macros
f7721ef 
(cherry picked from commit 30f1794)
@DougGregor
Disable macros-related tests harder
9f3843b 
(cherry picked from commit 68b367b)
@DougGregor
[Macros] Avoid type-checking a macro expansion declaration's argument…
ffa5766 
…s twice

Instead of trying to work around a double-type-check of macro expansion
declaration arguments, avoid type checking them twice in the first
place. We're caching the `ResolveMacroRequest` output in a somewhat
dodgy manner to account for the
`MacroExpansionDecl`/`MacroExpansionExpr` cloning.

(cherry picked from commit dd2d851)
@DougGregor
[Macros] Fix name lookup in macro expansion declaration arguments.
517c194 
Teach ASTScope to create child nodes for each of the arguments of a
macro expansion declaration, so that we can perform name lookup into
them. Otherwise, we cannot have nontrivial closures in arguments to a
macro expansion declaration.

Fixes rdar://107388151.

(cherry picked from commit 82de47c)
@DougGregor
[Macros] Share storage between linked MacroExpansion(Decl|Expr).
6cb79a5 
Sometimes we build a `MacroExpansionDecl` from a `MacroExpansionExpr`.
Sometimes we do it the other way. In both cases, we risk the two
copies of must-by-shared data (macro arguments, resolved macro
reference, etc.) getting out-of-sync.

Instead, share the storage between the two representations when we
create one from the other, so that they cannot get out-of-sync. This
allows us to eliminate the extremely-dodgy `cacheOutput` call earlier.

(cherry picked from commit 583ca47)
@DougGregor DougGregor requested a review from a team as a code owner March 31, 2023 04:29
@DougGregor
Copy link
Member Author

Note that this is 3 commits on top of #64802, but cannot be separated due to conflicts.

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor DougGregor merged commit 3f60a1d into swiftlang:release/5.9 Mar 31, 2023
@DougGregor DougGregor deleted the freestanding-macro-lookup-and-single-type-check-5.9 branch March 31, 2023 13:45
@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
Reviewers

@nkcsgexi nkcsgexi nkcsgexi approved these changes

Assignees
No one assigned
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DougGregor @nkcsgexi @AnthonyLatsis