[Macros] Freestanding macro expansions in code blocks #63217

rxwei · 2023-01-25T17:12:39Z

Parse freestanding macro expansions as declarations rather than expressions when it's not part of a statement or expression (i.e. "top-level"). When it's a freestanding expression macro,
Store the macro expansion result as a BraceStmt (mirroring CodeBlockItemListSyntax as in [Macros] Freestanding macros expand to CodeBlockItemList; parse top-level macro expansion decls swift-syntax#1268) to allow freestanding macros to produce any code block item.
Teach name lookup to look into the result of macro expansions. Plumb NLOptions through DeclContext lookup so that we can break the dependency cycle when we look up macros.

TODOs:

Combine @expression and @declaration(freestanding) to a single @freestanding attribute.
Fix local expansions.

Parse freestanding macro expansions as declarations rather than expressions when it's not part of a statement or expression (i.e. "top-level"). Teach name lookup to look into the result of macro expansions.

…or/swift into declaration-macro-name-lookup

…name-lookup

lib/Sema/TypeCheckMacros.cpp

DougGregor · 2023-01-26T19:15:17Z

include/swift/AST/Decl.h

@@ -8452,7 +8453,8 @@ class MacroExpansionDecl : public Decl {
  SourceLoc LeftAngleLoc, RightAngleLoc;
  ArrayRef<TypeRepr *> GenericArgs;
  ArgumentList *ArgList;
-  ArrayRef<Decl *> Rewritten;
+  // An implicit brace statement serving as the container for the expansion.
+  BraceStmt *Rewritten = nullptr;


Brace statements imply a nested scope, which probably isn't what we want semantically. Can this become an ArrayRef<ASTNode> to mimic the [CodeBlockItemSyntax] we recently agreed upon for the macro implementation interface?

Makes sense. I was relying on BraceStmt merely as a convenient container, and the BraceStmt was never visited as a child of MacroExpransionDecl; ASTWalker directly walks the BraceStmt's children instead. I think it's better than ArrayRef<ASTNode> because we can distinguish between nullptr (expansion failure) and an empty list from the return of swift::expandFreestandingMacro(). Otherwise I would need to make it return a bool and produce indirect results into a ASTNode vector. Thoughts?

DougGregor · 2023-01-26T19:19:50Z

lib/AST/ASTScope.cpp

+        if (med->getDeclContext()->getContextKind() ==
+            DeclContextKind::FileUnit)
+          return true;
+      } else if (auto *md = dyn_cast<MacroDecl>(decl)) {


Why check for a macro declaration here? It's only macro expansions that we need to worry about, right?

When we populate SourceLookupCache with top-level macro expansion results, type-checking a MacroDecl's arguments will run into the same circular dependency problem as type-checking a top-level MacroExpansionDecl's arguments. So I needed to check for both MD and MED to break the loop.

DougGregor · 2023-01-26T19:23:16Z

lib/AST/ASTWalker.cpp

@@ -430,11 +430,13 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
  }

  bool visitMacroExpansionDecl(MacroExpansionDecl *MED) {
-    if (MED->getArgs() && doIt(MED->getArgs()))


I wonder if we need a customization point here, because it seems like some things really want the see the original source, other things want to see the rewritten source, and some will want to see both.

DougGregor · 2023-01-26T19:25:20Z

lib/AST/Module.cpp

@@ -229,6 +233,7 @@ SourceLookupCache &SourceFile::getCache() const {
 template<typename Range>
 void SourceLookupCache::addToUnqualifiedLookupCache(Range decls,
                                                    bool onlyOperators) {
+  SmallVector<MacroExpansionDecl *, 4> delayedMacroExpansions;


This looks unused?

DougGregor · 2023-01-26T19:30:41Z

lib/AST/Module.cpp

+    if (rewritten)
+      for (auto node : rewritten->getElements())
+        if (auto *decl = node.dyn_cast<Decl *>())
+          addToUnqualifiedLookupCache(TinyPtrVector<Decl *>{decl}, false);


The macro expansion could produce other macro expansion declarations, and we should make sure to account for those. It currently looks like addToUnqualifiedLookupCache could add to DelayedMacroExpansions while we're iterating over it. We'll either not see those added macro expansion declarations (because of the clear() below) or we'll crash if the vector gets reallocated.

DougGregor · 2023-01-26T19:46:08Z

lib/Sema/TypeCheckDeclPrimary.cpp

+  MacroRoles viableRoles(MacroRole::Declaration);
+  // In a code block context, a macro expansion decl can also expand to an
+  // arbitrary code item.
+  if (dc->isLocalContext() ||
+      dc->getContextKind() == DeclContextKind::FileUnit) {
+    viableRoles |= MacroRole::Expression;
+    viableRoles |= MacroRole::CodeItem;
+  }


This is definitely an important piece of logic. What roles are acceptable, and where?

DougGregor · 2023-01-26T19:47:49Z

lib/Sema/TypeCheckDeclPrimary.cpp

-      cs.setType(arg, TypeChecker::typeCheckExpression(arg, dc));
+    SmallVector<Argument, 2> newArgs;
+    newArgs.reserve(args->size());
+    for (auto arg : *args) {


There's similar logic to this for attached macros, where we need to type-check @macroname(args...). We should unify the logic with that.

Fixed in #63247. ResolveAttachedMacroRequest's approach of forming and type-checking a OverloadedDeclRefExpr was very clever. I was able to remove all of the custom constraint generation code.

DougGregor · 2023-01-26T19:49:09Z

lib/Sema/TypeChecker.h

  IncludeUsableFromInline = 1 << 2,
+  /// Disable macro expansions.
+  DisableMacroExpansions = 1 << 3,


These "disable macro expansions" flags end up in a lot of places. Are they all necessary, or can some be re-derived from ASTScope and location info?

The need for DisableMacroExpansions is all related to default literal type lookup. Default literal type lookup tries to find a literal type name in the module scope, which, without the added flag, triggers top-level macro expansions. DefaultTypeRequest also doesn't have source location to check AST scope information, so we'd have to plumb through the locations from every call site, which I did give a try. That added location, however, caused default literal type lookup, which uses unqualified lookup at in the module scope context, to look into type members, at which point it felt like opening a can of worms and it was no simpler than adding DisableMacroExpansions.

DougGregor · 2023-01-26T19:49:59Z

test/Macros/parsing.swift

@@ -48,3 +47,7 @@ macro am1: Void
 )
 macro am2: Void
 // expected-error@-1{{macro 'am2' requires a definition}}
+
+@expression @declaration(freestanding) @declaration(attached)


FYI, @declaration(freestanding) has been replaced with @freestanding(declaration) on main.

DougGregor · 2023-01-26T19:52:08Z

lib/Sema/TypeCheckDeclPrimary.cpp

+    }
+    auto codeBlockItemList = BraceStmt::createImplicit(ctx, {expansion});
+    MED->setRewritten(codeBlockItemList);
+    TypeChecker::postTypeCheckMacroExpansion(mee, innermostDC);


It's interesting that this doesn't happen as part of type-checking the outer context. Is this because at some outer level (say, walking all of the code items in the enclosing BraceStmt) we should also be walking into macro expansions? Similar to my comment earlier about having a semantic query that flattens all of the macro expansions so you walk a semantic view vs. the syntactic view?

The BraceStmt isn't the problem because it was only a means of code block storage in MacroExpansionDecl, and was never visited by any walker (the walker skips this level). In #63553 I placed BraceStmt with ArrayRef<ASTNode> and the need for postTypeCheck... still remains.

I think the reason is because those post-checking passes are all triggered from the AST verifier, but the verifier isn't triggering macro expansions.

rxwei requested review from hborla, xedin, slavapestov, zoecarver, hyp, egorzhdan, ahoppen, bnbarham, CodaFi, DougGregor, rintaro and artemcm as code owners January 25, 2023 17:12

xedin removed their request for review January 25, 2023 18:52

[Macros] Freestanding macro expansions in code blocks

283c5cd

Parse freestanding macro expansions as declarations rather than expressions when it's not part of a statement or expression (i.e. "top-level"). Teach name lookup to look into the result of macro expansions.

rxwei force-pushed the declaration-macro-name-lookup branch from fc2f277 to 283c5cd Compare January 26, 2023 04:55

rxwei added 4 commits January 26, 2023 18:08

CodeItem macro role

dd876f4

Merge branch 'freestanding-macro-attr-cleanup' of github.com:DougGreg…

41f1170

…or/swift into declaration-macro-name-lookup

Merge fixes.

0fc4648

Fix merge again.

b6552ed

rxwei force-pushed the declaration-macro-name-lookup branch from f133e42 to b6552ed Compare January 26, 2023 15:56

Merge branch 'main' of github.com:apple/swift into declaration-macro-…

373bc85

…name-lookup

rxwei commented Jan 26, 2023

View reviewed changes

lib/Sema/TypeCheckMacros.cpp Outdated Show resolved Hide resolved

rxwei force-pushed the declaration-macro-name-lookup branch 2 times, most recently from d4598c2 to a11e791 Compare January 26, 2023 16:58

Fix messed up indentation in TypeCheckMacros.cpp

f030142

rxwei force-pushed the declaration-macro-name-lookup branch from a11e791 to f030142 Compare January 26, 2023 17:50

DougGregor reviewed Jan 26, 2023

View reviewed changes

ResolveMacroRequest

7d5878f

rxwei mentioned this pull request Feb 9, 2023

[Macros] Top-level freestanding macros #63553

Merged

rxwei closed this Feb 9, 2023

[Macros] Freestanding macro expansions in code blocks #63217

[Macros] Freestanding macro expansions in code blocks #63217

Uh oh!

Conversation

rxwei commented Jan 25, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxwei Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxwei Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rxwei Jan 27, 2023 •

edited

Loading

rxwei Feb 9, 2023 •

edited

Loading