[Macros] Remove the macro role argument to ResolveMacroRequest.

hborla · hborla · commit c33c925e81f8 · 2023-02-14T22:38:53.000-08:00
The macro role argument presented an opportunity for callers to accidentally
invoke this request twice for the same macro with slightly different macro
roles passed in, which resulted in re-typechecking the macro arguments.
Instead, derive the corresponding macro roles from the macro reference syntax.
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -3162,6 +3162,9 @@ class UnresolvedMacroReference {
   ArrayRef<TypeRepr *> getGenericArgs() const;
   ArgumentList *getArgs() const;
 
+  /// Returns the macro roles corresponding to this macro reference.
+  MacroRoles getMacroRoles() const;
+
   friend bool operator==(const UnresolvedMacroReference &lhs,
                          const UnresolvedMacroReference &rhs) {
     return lhs.getOpaqueValue() == rhs.getOpaqueValue();
@@ -3178,7 +3181,7 @@ void simple_display(llvm::raw_ostream &out,
 /// Resolve a given custom attribute to an attached macro declaration.
 class ResolveMacroRequest
     : public SimpleRequest<ResolveMacroRequest,
-                           MacroDecl *(UnresolvedMacroReference, MacroRoles,
+                           MacroDecl *(UnresolvedMacroReference,
                                        DeclContext *),
                            RequestFlags::Cached> {
 public:
@@ -3189,7 +3192,7 @@ class ResolveMacroRequest
 
   MacroDecl *
   evaluate(Evaluator &evaluator, UnresolvedMacroReference macroRef,
-           MacroRoles roles, DeclContext *dc) const;
+           DeclContext *dc) const;
 
 public:
   bool isCached() const { return true; }
diff --git a/lib/AST/ASTMangler.cpp b/lib/AST/ASTMangler.cpp
@@ -3804,7 +3804,7 @@ void ASTMangler::appendMacroExpansionContext(
     auto *macroDecl = evaluateOrDefault(
         ctx.evaluator,
         ResolveMacroRequest{const_cast<CustomAttr *>(attr),
-                            getAttachedMacroRoles(), outerExpansionDC},
+                            outerExpansionDC},
         nullptr);
     if (macroDecl)
       baseName = macroDecl->getBaseName();
@@ -3898,7 +3898,7 @@ std::string ASTMangler::mangleAttachedMacroExpansion(
   StringRef macroName;
   auto *macroDecl = evaluateOrDefault(
       decl->getASTContext().evaluator,
-      ResolveMacroRequest{attr, role, macroDeclContext},
+      ResolveMacroRequest{attr, macroDeclContext},
       nullptr);
   if (macroDecl)
     macroName = macroDecl->getName().getBaseName().userFacingName();
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
@@ -2382,8 +2382,7 @@ bool CustomAttr::isAttachedMacro(const Decl *decl) const {
 
   auto *macroDecl = evaluateOrDefault(
       ctx.evaluator,
-      ResolveMacroRequest{const_cast<CustomAttr *>(this),
-                          getAttachedMacroRoles(), dc},
+      ResolveMacroRequest{const_cast<CustomAttr *>(this), dc},
       nullptr);
 
   return macroDecl != nullptr;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -406,7 +406,7 @@ void Decl::forEachAttachedMacro(MacroRole role,
     auto customAttr = const_cast<CustomAttr *>(customAttrConst);
     auto *macroDecl = evaluateOrDefault(
         ctx.evaluator,
-        ResolveMacroRequest{customAttr, getAttachedMacroRoles(), dc},
+        ResolveMacroRequest{customAttr, dc},
         nullptr);
 
     if (!macroDecl)
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -1523,7 +1523,7 @@ populateLookupTableEntryFromMacroExpansions(ASTContext &ctx,
     if (!med)
       continue;
     auto macro = evaluateOrDefault(
-        ctx.evaluator, ResolveMacroRequest{med, MacroRole::Declaration, dc},
+        ctx.evaluator, ResolveMacroRequest{med, dc},
         nullptr);
     if (!macro)
       continue;
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
@@ -1742,6 +1742,19 @@ ArgumentList *UnresolvedMacroReference::getArgs() const {
   llvm_unreachable("Unhandled case");
 }
 
+MacroRoles UnresolvedMacroReference::getMacroRoles() const {
+  if (pointer.is<MacroExpansionExpr *>())
+    return MacroRole::Expression;
+
+  if (pointer.is<MacroExpansionDecl *>())
+    return getFreestandingMacroRoles();
+
+  if (pointer.is<CustomAttr *>())
+    return getAttachedMacroRoles();
+
+  llvm_unreachable("Unsupported macro reference");
+}
+
 void swift::simple_display(llvm::raw_ostream &out,
                            const UnresolvedMacroReference &ref) {
   if (ref.getDecl())
diff --git a/lib/IDE/SourceEntityWalker.cpp b/lib/IDE/SourceEntityWalker.cpp
@@ -698,7 +698,6 @@ bool SemaAnnotator::handleCustomAttributes(Decl *D) {
       // If this attribute resolves to a macro, index that.
       ASTContext &ctx = D->getASTContext();
       ResolveMacroRequest req{const_cast<CustomAttr *>(customAttr),
-                              getAttachedMacroRoles(),
                               D->getInnermostDeclContext()};
       if (auto macroDecl = evaluateOrDefault(ctx.evaluator, req, nullptr)) {
         Type macroRefType = macroDecl->getDeclaredInterfaceType();
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -5365,7 +5365,12 @@ namespace {
       ConcreteDeclRef macroRef = resolveConcreteDeclRef(macro, locator);
       E->setMacroRef(macroRef);
 
-      if (E->getMacroRoles().contains(MacroRole::Expression) &&
+      // For now, only expand macro expansion expressions that fulfill
+      // `MacroRole::Expression` exactly. Freestanding code item macros
+      // have a `getMacroRoles()` value equal to `getFreestandingMacroRoles()`,
+      // which includes expression macros, and they are expanded in a separate
+      // request.
+      if (E->getMacroRoles() == MacroRole::Expression &&
           !cs.Options.contains(ConstraintSystemFlags::DisableMacroExpansions)) {
         if (auto newExpr = expandMacroExpr(dc, E, macroRef, expandedType)) {
           E->setRewritten(newExpr);
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -3568,7 +3568,7 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
   if (!nominal) {
     // Try resolving an attached macro attribute.
     auto *macro = evaluateOrDefault(
-        Ctx.evaluator, ResolveMacroRequest{attr, getAttachedMacroRoles(), dc},
+        Ctx.evaluator, ResolveMacroRequest{attr, dc},
         nullptr);
     if (macro || !attr->isValid())
       return;
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -3782,7 +3782,7 @@ ExpandMacroExpansionDeclRequest::evaluate(Evaluator &evaluator,
   MacroDecl *macro;
   if (auto *args = MED->getArgs()) {
     macro = evaluateOrDefault(
-        ctx.evaluator, ResolveMacroRequest{MED, MacroRole::Declaration, dc},
+        ctx.evaluator, ResolveMacroRequest{MED, dc},
         nullptr);
   }
   else {
diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp
@@ -460,7 +460,7 @@ static bool isFromExpansionOfMacro(SourceFile *sourceFile, MacroDecl *macro,
       auto *decl = expansion.dyn_cast<Decl *>();
       auto &ctx = decl->getASTContext();
       auto *macroDecl = evaluateOrDefault(ctx.evaluator,
-          ResolveMacroRequest{macroAttr, role, decl->getDeclContext()},
+          ResolveMacroRequest{macroAttr, decl->getDeclContext()},
           nullptr);
       if (!macroDecl)
         return false;
@@ -1206,9 +1206,9 @@ swift::expandPeers(CustomAttr *attr, MacroDecl *macro, Decl *decl) {
 MacroDecl *
 ResolveMacroRequest::evaluate(Evaluator &evaluator,
                               UnresolvedMacroReference macroRef,
-                              MacroRoles roles,
                               DeclContext *dc) const {
   auto &ctx = dc->getASTContext();
+  auto roles = macroRef.getMacroRoles();
   auto foundMacros = TypeChecker::lookupMacros(
       dc, macroRef.getMacroName(), SourceLoc(), roles);
   if (foundMacros.empty())
diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift
@@ -1002,3 +1002,13 @@ public struct NewTypeMacro: MemberMacro {
     ]
   }
 }
+
+public struct EmptyMacro: MemberMacro {
+  public static func expansion(
+    of node: AttributeSyntax,
+    providingMembersOf decl: some DeclGroupSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return []
+  }
+}
diff --git a/test/Macros/macro_expand_synthesized_members.swift b/test/Macros/macro_expand_synthesized_members.swift
@@ -39,3 +39,14 @@ public struct MyString {}
 let myString = MyString("hello")
 print(MyString.RawValue.self)
 print(myString.rawValue)
+
+struct Base {
+  static func member() -> Base { .init() }
+}
+
+@attached(member) macro empty(
+  _ : Base
+) = #externalMacro(module: "MacroDefinition", type: "EmptyMacro")
+
+@empty(.member())
+struct TestMacroTypechecking {}

Original file line number	Diff line number	Diff line change
`@@ -3782,7 +3782,7 @@ ExpandMacroExpansionDeclRequest::evaluate(Evaluator &evaluator,`
`3782`	`3782`	`MacroDecl *macro;`
`3783`	`3783`	`if (auto *args = MED->getArgs()) {`
`3784`	`3784`	`macro = evaluateOrDefault(`
`3785`		`- ctx.evaluator, ResolveMacroRequest{MED, MacroRole::Declaration, dc},`
	`3785`	`+ ctx.evaluator, ResolveMacroRequest{MED, dc},`
`3786`	`3786`	`nullptr);`
`3787`	`3787`	`}`
`3788`	`3788`	`else {`