Merge pull request #85363 from hamishknight/hattrick

[Sema] Avoid member attribute cycle in ResolveMacroRequest

Author: hamishknight

include/swift/AST/Attr.h

@@ -2359,6 +2359,10 @@ class CustomAttr final : public DeclAttribute {
 
   ASTContext &getASTContext() const;
 
+  /// If \c true, we should prefer a property wrapper if one exists for the
+  /// given attribute over a macro.
+  bool shouldPreferPropertyWrapperOverMacro() const;
+
   /// Retrieve the NominalTypeDecl the CustomAttr refers to, or \c nullptr if
   /// it doesn't refer to one (which can be the case for e.g macro attrs).
   NominalTypeDecl *getNominalDecl() const;

lib/AST/Attr.cpp

@@ -3167,6 +3167,20 @@ ASTContext &CustomAttr::getASTContext() const {
   return getOwner().getDeclContext()->getASTContext();
 }
 
+bool CustomAttr::shouldPreferPropertyWrapperOverMacro() const {
+  // If we have a VarDecl in a local context, prefer to use a property wrapper
+  // if one exists. This is necessary since we don't properly support peer
+  // declarations in local contexts, so want to use a property wrapper if one
+  // exists.
+  if (auto *D = getOwner().getAsDecl()) {
+    if ((isa<VarDecl>(D) || isa<PatternBindingDecl>(D)) &&
+        D->getDeclContext()->isLocalContext()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 NominalTypeDecl *CustomAttr::getNominalDecl() const {
   auto &eval = getASTContext().evaluator;
   auto *mutThis = const_cast<CustomAttr *>(this);

lib/AST/NameLookup.cpp

@@ -4020,20 +4020,6 @@ GenericParamListRequest::evaluate(Evaluator &evaluator, GenericContext *value) c
       parsedGenericParams->getRAngleLoc());
 }
 
-static bool shouldPreferPropertyWrapperOverMacro(CustomAttrOwner owner) {
-  // If we have a VarDecl in a local context, prefer to use a property wrapper
-  // if one exists. This is necessary since we don't properly support peer
-  // declarations in local contexts, so want to use a property wrapper if one
-  // exists.
-  if (auto *D = owner.getAsDecl()) {
-    if ((isa<VarDecl>(D) || isa<PatternBindingDecl>(D)) &&
-        D->getDeclContext()->isLocalContext()) {
-      return true;
-    }
-  }
-  return false;
-}
-
 NominalTypeDecl *CustomAttrNominalRequest::evaluate(Evaluator &evaluator,
                                                     CustomAttr *attr) const {
   auto owner = attr->getOwner();
@@ -4048,7 +4034,7 @@ NominalTypeDecl *CustomAttrNominalRequest::evaluate(Evaluator &evaluator,
   auto macroName = (macro) ? macro->getNameRef() : DeclNameRef();
   auto macros = namelookup::lookupMacros(dc, moduleName, macroName,
                                          getAttachedMacroRoles());
-  auto shouldPreferPropWrapper = shouldPreferPropertyWrapperOverMacro(owner);
+  auto shouldPreferPropWrapper = attr->shouldPreferPropertyWrapperOverMacro();
   if (!macros.empty() && !shouldPreferPropWrapper)
     return nullptr;
 

lib/Sema/TypeCheckMacros.cpp

@@ -2242,8 +2242,12 @@ ResolveMacroRequest::evaluate(Evaluator &evaluator,
   // So bail out to prevent diagnostics from the contraint system.
   if (auto *attr = macroRef.getAttr()) {
     // If we already resolved this CustomAttr to a nominal, this isn't for a
-    // macro.
-    if (attr->getNominalDecl())
+    // macro. This can only currently be the case for property wrappers, so
+    // limit the check here to avoid request cycles for member attribute macros
+    // in cases where the attribute refers to a nested type in the type it's
+    // attached to, since the qualified lookup there needs to expand member
+    // attributes.
+    if (attr->shouldPreferPropertyWrapperOverMacro() && attr->getNominalDecl())
       return ConcreteDeclRef();
 
     auto foundMacros = namelookup::lookupMacros(dc, macroRef.getModuleName(),

test/NameLookup/rdar163961797.swift

@@ -0,0 +1,10 @@
+// RUN: %target-typecheck-verify-swift
+
+// Make sure we don't end up with a request cycle here, since looking up 'S.Actor'
+// requires expanding member attribute macros for 'S'.
+@S.Actor
+struct S {
+  @globalActor actor Actor: GlobalActor {
+    public static let shared = S.Actor()
+  }
+}