[Macros] Requestify member attribute macro expansion directly.

Having a request for semantic declaration attributes while still allowing
the original linked list to be mutated directly can cause use-after-frees
on pointers to old decl attributes. For now, introduce a request that
expands member attribute macros with a side effect of adding the new
attributes to the list. We can revisit this once attribute mutation via
getAttrs() is audited across the frontend.

Author: hborla

include/swift/AST/Attr.h

@@ -2613,55 +2613,6 @@ class DeclAttributes {
   SourceLoc getStartLoc(bool forModifiers = false) const;
 };
 
-/// Semantic attributes that are applied to a declaration.
-///
-/// This attribute list can include attributes that are not written in
-/// source on a declaration, such as attributes that are applied during
-/// macro expansion or inferred from the declaration context.
-class SemanticDeclAttributes {
-  llvm::SmallVector<DeclAttribute *, 4> attrList;
-
-public:
-  SemanticDeclAttributes() {}
-
-  /// Add a constructed DeclAttribute to this list.
-  void add(DeclAttribute *attr) {
-    attrList.push_back(attr);
-  }
-
-  using SemanticAttrList = llvm::SmallVectorImpl<DeclAttribute *>;
-
-  template <typename AttrType, bool AllowInvalid>
-  using AttributeKindRange =
-      OptionalTransformRange<iterator_range<SemanticAttrList::const_iterator>,
-                             ToAttributeKind<AttrType, AllowInvalid>,
-                             SemanticAttrList::const_iterator>;
-
-  template <typename AttrType, bool AllowInvalid = false>
-  AttributeKindRange<AttrType, AllowInvalid> getAttributes() const {
-    return AttributeKindRange<AttrType, AllowInvalid>(
-        make_range(attrList.begin(), attrList.end()),
-                   ToAttributeKind<AttrType, AllowInvalid>());
-  }
-
-  /// Retrieve the first attribute of the given attribute class.
-  template <typename AttrType>
-  const AttrType *getAttribute(bool allowInvalid = false) const {
-    for (auto attr : attrList)
-      if (auto *specificAttr = dyn_cast<AttrType>(attr))
-        if (specificAttr->isValid() || allowInvalid)
-          return specificAttr;
-
-    return nullptr;
-  }
-
-  /// Determine whether there is an attribute with the given attribute class.
-  template <typename AttrType>
-  bool hasAttribute(bool allowInvalid = false) const {
-    return getAttribute<AttrType>(allowInvalid) != nullptr;
-  }
-};
-
 /// TypeAttributes - These are attributes that may be applied to types.
 class TypeAttributes {
   // Get a SourceLoc for every possible attribute that can be parsed in source.

include/swift/AST/Decl.h

@@ -850,11 +850,10 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
     return Attrs;
   }
 
-  /// Returns the semantic attributes attached to this declaration.
-  ///
-  /// \c getSemanticAttrs is intended to be a requestified replacement
-  /// for \c getAttrs
-  SemanticDeclAttributes getSemanticAttrs() const;
+  /// Returns the semantic attributes attached to this declaration,
+  /// including attributes that are generated as the result of member
+  /// attribute macro expansion.
+  DeclAttributes getSemanticAttrs() const;
 
   /// Returns the innermost enclosing decl with an availability annotation.
   const Decl *getInnermostDeclWithAvailability() const;

include/swift/AST/TypeCheckRequests.h

@@ -742,26 +742,6 @@ class AttachedPropertyWrappersRequest :
   bool isCached() const { return true; }
 };
 
-/// Request the semantic attributes attached to the given declaration.
-class AttachedSemanticAttrsRequest :
-    public SimpleRequest<AttachedSemanticAttrsRequest,
-                         SemanticDeclAttributes(Decl *),
-                         RequestFlags::Cached> {
-public:
-  using SimpleRequest::SimpleRequest;
-
-private:
-  friend SimpleRequest;
-
-  // Evaluation.
-  SemanticDeclAttributes
-  evaluate(Evaluator &evaluator, Decl *decl) const;
-
-public:
-  // Caching
-  bool isCached() const { return true; }
-};
-
 /// Request the raw (possibly unbound generic) type of the property wrapper
 /// that is attached to the given variable.
 class AttachedPropertyWrapperTypeRequest :
@@ -3840,6 +3820,24 @@ class ExpandMacroExpansionDeclRequest
   bool isCached() const { return true; }
 };
 
+/// Expand all member attribute macros attached to the given
+/// declaration.
+class ExpandMemberAttributeMacros
+    : public SimpleRequest<ExpandMemberAttributeMacros,
+                           bool(Decl *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  bool evaluate(Evaluator &evaluator, Decl *decl) const;
+
+public:
+  bool isCached() const { return true; }
+};
+
 /// Resolve an external macro given its module and type name.
 class ExternalMacroDefinitionRequest
     : public SimpleRequest<ExternalMacroDefinitionRequest,

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -29,9 +29,6 @@ SWIFT_REQUEST(TypeChecker, AttachedPropertyWrapperTypeRequest,
 SWIFT_REQUEST(TypeChecker, AttachedPropertyWrappersRequest,
               llvm::TinyPtrVector<CustomAttr *>(VarDecl *), Cached,
               NoLocationInfo)
-SWIFT_REQUEST(TypeChecker, AttachedSemanticAttrsRequest,
-              SemanticDeclAttributes(Decl *), Cached,
-              NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, CallerSideDefaultArgExprRequest,
               Expr *(DefaultArgumentExpr *), SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, CheckInconsistentImplementationOnlyImportsRequest,
@@ -455,6 +452,9 @@ SWIFT_REQUEST(TypeChecker, ExternalMacroDefinitionRequest,
 SWIFT_REQUEST(TypeChecker, ExpandMacroExpansionDeclRequest,
               ArrayRef<Decl *>(MacroExpansionDecl *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, ExpandMemberAttributeMacros,
+              bool(Decl *),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, SynthesizeRuntimeMetadataAttrGenerator,
               Expr *(CustomAttr *, ValueDecl *),
               Cached, NoLocationInfo)

lib/AST/Decl.cpp

@@ -365,11 +365,13 @@ StringRef Decl::getDescriptiveKindName(DescriptiveDeclKind K) {
   llvm_unreachable("bad DescriptiveDeclKind");
 }
 
-SemanticDeclAttributes Decl::getSemanticAttrs() const {
+DeclAttributes Decl::getSemanticAttrs() const {
   auto mutableThis = const_cast<Decl *>(this);
-  return evaluateOrDefault(getASTContext().evaluator,
-                           AttachedSemanticAttrsRequest{mutableThis},
-                           SemanticDeclAttributes());
+  evaluateOrDefault(getASTContext().evaluator,
+                    ExpandMemberAttributeMacros{mutableThis},
+                    false);
+
+  return getAttrs();
 }
 
 const Decl *Decl::getInnermostDeclWithAvailability() const {

lib/Sema/TypeCheckAttr.cpp

@@ -7385,45 +7385,6 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
   return renamedDecl;
 }
 
-SemanticDeclAttributes
-AttachedSemanticAttrsRequest::evaluate(Evaluator &evaluator, Decl *decl) const {
-  // For now, this returns the same thing as 'getAttrs' using
-  // the SemanticDeclAttribtues representation.
-  SemanticDeclAttributes semanticAttrs;
-  for (auto attr : decl->getAttrs()) {
-    semanticAttrs.add(attr);
-  }
-
-  auto *parentDecl = decl->getDeclContext()->getAsDecl();
-  if (!parentDecl)
-    return semanticAttrs;
-
-  auto parentAttrs = parentDecl->getSemanticAttrs();
-  for (auto customAttrConst: parentAttrs.getAttributes<CustomAttr>()) {
-    auto customAttr = const_cast<CustomAttr *>(customAttrConst);
-    auto customAttrDecl = evaluateOrDefault(
-        evaluator,
-        CustomAttrDeclRequest{
-          customAttr,
-          parentDecl->getInnermostDeclContext()
-        },
-        nullptr);
-    if (!customAttrDecl)
-      continue;
-
-    auto macroDecl = customAttrDecl.dyn_cast<MacroDecl *>();
-    if (!macroDecl)
-      continue;
-
-    if (!macroDecl->getMacroRoles().contains(MacroRole::MemberAttribute))
-      continue;
-
-    expandAttributes(customAttr, macroDecl, decl, semanticAttrs);
-  }
-
-  return semanticAttrs;
-}
-
 template <typename ATTR>
 static void forEachCustomAttribute(
     ValueDecl *decl,

lib/Sema/TypeCheckMacros.cpp

@@ -308,6 +308,41 @@ ExternalMacroDefinitionRequest::evaluate(
   return ExternalMacroDefinition{nullptr};
 }
 
+bool ExpandMemberAttributeMacros::evaluate(Evaluator &evaluator,
+                                           Decl *decl) const {
+  auto *parentDecl = decl->getDeclContext()->getAsDecl();
+  if (!parentDecl)
+    return false;
+
+  bool addedAttributes = false;
+  auto parentAttrs = parentDecl->getSemanticAttrs();
+  for (auto customAttrConst: parentAttrs.getAttributes<CustomAttr>()) {
+    auto customAttr = const_cast<CustomAttr *>(customAttrConst);
+    auto customAttrDecl = evaluateOrDefault(
+        evaluator,
+        CustomAttrDeclRequest{
+          customAttr,
+          parentDecl->getInnermostDeclContext()
+        },
+        nullptr);
+
+    if (!customAttrDecl)
+      continue;
+
+    auto macroDecl = customAttrDecl.dyn_cast<MacroDecl *>();
+    if (!macroDecl)
+      continue;
+
+    if (!macroDecl->getMacroRoles().contains(MacroRole::MemberAttribute))
+      continue;
+
+    addedAttributes |= expandAttributes(customAttr, macroDecl, decl);
+  }
+
+  return addedAttributes;
+}
+
+
 /// Determine whether the given source file is from an expansion of the given
 /// macro.
 static bool isFromExpansionOfMacro(SourceFile *sourceFile, MacroDecl *macro) {
@@ -856,8 +891,7 @@ void swift::expandAccessors(
 // FIXME: Almost entirely duplicated code from `expandAccessors`.
 // Factor this out into an `expandAttachedMacro` function, with
 // arguments for the PrettyStackTrace string, 'attachedTo' decl, etc.
-void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
-                             SemanticDeclAttributes &result) {
+bool swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member) {
   auto *dc = member->getInnermostDeclContext();
   ASTContext &ctx = dc->getASTContext();
   SourceManager &sourceMgr = ctx.SourceMgr;
@@ -867,43 +901,43 @@ void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
   auto attrSourceFile =
     moduleDecl->getSourceFileContainingLocation(attr->AtLoc);
   if (!attrSourceFile)
-    return;
+    return false;
 
   auto declSourceFile =
       moduleDecl->getSourceFileContainingLocation(member->getStartLoc());
   if (!declSourceFile)
-    return;
+    return false;
 
   Decl *parentDecl = member->getDeclContext()->getAsDecl();
   if (!parentDecl)
-    return;
+    return false;
 
   auto parentDeclSourceFile =
     moduleDecl->getSourceFileContainingLocation(parentDecl->getLoc());
   if (!parentDeclSourceFile)
-    return;
+    return false;
 
   // Evaluate the macro.
   NullTerminatedStringRef evaluatedSource;
 
   if (isFromExpansionOfMacro(attrSourceFile, macro) ||
       isFromExpansionOfMacro(declSourceFile, macro)) {
     member->diagnose(diag::macro_recursive, macro->getName());
-    return;
+    return false;
   }
 
   auto macroDef = macro->getDefinition();
   switch (macroDef.kind) {
   case MacroDefinition::Kind::Undefined:
   case MacroDefinition::Kind::Invalid:
     // Already diagnosed as an error elsewhere.
-    return;
+    return false;
 
   case MacroDefinition::Kind::Builtin: {
     switch (macroDef.getBuiltinKind()) {
     case BuiltinMacroKind::ExternalMacro:
       // FIXME: Error here.
-      return;
+      return false;
     }
   }
 
@@ -923,29 +957,29 @@ void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
                         macro->getName()
       );
       macro->diagnose(diag::decl_declared_here, macro->getName());
-      return;
+      return false;
     }
 
     // Make sure macros are enabled before we expand.
     if (!ctx.LangOpts.hasFeature(Feature::Macros)) {
       member->diagnose(diag::macro_experimental);
-      return;
+      return false;
     }
 
 #if SWIFT_SWIFT_PARSER
     PrettyStackTraceDecl debugStack("expanding attribute macro", member);
 
     auto astGenAttrSourceFile = attrSourceFile->exportedSourceFile;
     if (!astGenAttrSourceFile)
-      return;
+      return false;
 
     auto astGenDeclSourceFile = declSourceFile->exportedSourceFile;
     if (!astGenDeclSourceFile)
-      return;
+      return false;
 
     auto astGenParentDeclSourceFile = parentDeclSourceFile->exportedSourceFile;
     if (!astGenParentDeclSourceFile)
-      return;
+      return false;
 
     Decl *searchDecl = member;
     if (auto *var = dyn_cast<VarDecl>(member))
@@ -961,13 +995,13 @@ void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
         astGenParentDeclSourceFile, parentDecl->getStartLoc().getOpaquePointerValue(),
         &evaluatedSourceAddress, &evaluatedSourceLength);
     if (!evaluatedSourceAddress)
-      return;
+      return false;
     evaluatedSource = NullTerminatedStringRef(evaluatedSourceAddress,
                                               (size_t)evaluatedSourceLength);
     break;
 #else
     member->diagnose(diag::macro_unsupported);
-    return;
+    return false;
 #endif
   }
   }
@@ -1029,6 +1063,7 @@ void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
   PrettyStackTraceDecl debugStack(
       "type checking expanded declaration macro", member);
 
+  bool addedAttributes = false;
   auto topLevelDecls = macroSourceFile->getTopLevelDecls();
   for (auto decl : topLevelDecls) {
     // FIXME: We want to type check decl attributes applied to
@@ -1040,7 +1075,10 @@ void swift::expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
 
     // Add the new attributes to the semantic attribute list.
     for (auto *attr : decl->getAttrs()) {
-      result.add(attr);
+      addedAttributes = true;
+      member->getAttrs().add(attr);
     }
   }
+
+  return addedAttributes;
 }

lib/Sema/TypeCheckMacros.h

@@ -51,8 +51,7 @@ void expandAccessors(
 
 /// Expand the attributes for the given member declaration based
 /// on the custom attribute that references the given macro.
-void expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member,
-                      SemanticDeclAttributes &result);
+bool expandAttributes(CustomAttr *attr, MacroDecl *macro, Decl *member);
 
 } // end namespace swift