Merge pull request swiftlang#77962 from DougGregor/importer-no-cache-custom-swift-attr

[Clang importer] Don't cache swift_attr source files that have CustomAttrs with arguments

Author: DougGregor

include/swift/AST/Attr.h

@@ -508,13 +508,17 @@ class DeclAttribute : public AttributeBase {
 
   /// Create a copy of this attribute.
   DeclAttribute *clone(ASTContext &ctx) const;
+
+  /// Determine whether we can clone this attribute.
+  bool canClone() const;
 };
 
 #define UNIMPLEMENTED_CLONE(AttrType)    \
 AttrType *clone(ASTContext &ctx) const { \
     llvm_unreachable("unimplemented");   \
     return nullptr;                      \
-  }
+  }                                      \
+bool canClone() const { return false; }
 
 /// Describes a "simple" declaration attribute that carries no data.
 template<DeclAttrKind Kind>
@@ -1916,9 +1920,13 @@ class CustomAttr final : public DeclAttribute {
 
   /// Create a copy of this attribute.
   CustomAttr *clone(ASTContext &ctx) const {
+    assert(argList == nullptr &&
+           "Cannot clone custom attribute with an argument list");
     return create(ctx, AtLoc, getTypeExpr(), initContext, argList, isImplicit());
   }
 
+  bool canClone() const { return argList == nullptr; }
+
 private:
   friend class CustomAttrNominalRequest;
   void resetTypeInformation(TypeExpr *repr);

lib/AST/Attr.cpp

@@ -370,6 +370,17 @@ DeclAttribute *DeclAttribute::clone(ASTContext &ctx) const {
   }
 }
 
+bool DeclAttribute::canClone() const {
+  switch (getKind()) {
+#define DECL_ATTR(_,CLASS, ...)                                \
+  case DeclAttrKind::CLASS:                                    \
+    if (&CLASS##Attr::canClone == &DeclAttribute::canClone)    \
+      return true;                                             \
+    return static_cast<const CLASS##Attr *>(this)->canClone();
+#include "swift/AST/DeclAttr.def"
+  }
+}
+
 const BackDeployedAttr *
 DeclAttributes::getBackDeployed(const ASTContext &ctx,
                                 bool forTargetVariant) const {

lib/ClangImporter/ImportDecl.cpp

@@ -8244,14 +8244,17 @@ bool importer::hasSameUnderlyingType(const clang::Type *a,
 
 SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
     ModuleDecl &module,
-    StringRef attributeText
+    StringRef attributeText,
+    bool cached
 ) {
-  auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
+  if (cached) {
+    auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
 
-  // Check whether we've already created a source file.
-  for (auto sourceFile : sourceFiles) {
-    if (sourceFile->getParentModule() == &module)
-      return *sourceFile;
+    // Check whether we've already created a source file.
+    for (auto sourceFile : sourceFiles) {
+      if (sourceFile->getParentModule() == &module)
+        return *sourceFile;
+    }
   }
 
   // Create a new buffer with a copy of the attribute text,
@@ -8273,7 +8276,11 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
   // Create the source file.
   auto sourceFile = new (SwiftContext)
       SourceFile(module, SourceFileKind::Library, bufferID);
-  sourceFiles.push_back(sourceFile);
+
+  if (cached) {
+    auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
+    sourceFiles.push_back(sourceFile);
+  }
 
   return *sourceFile;
 }
@@ -8450,17 +8457,52 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
         continue;
       }
 
-      // Dig out a source file we can use for parsing.
-      auto &sourceFile = getClangSwiftAttrSourceFile(
-          *MappedDecl->getDeclContext()->getParentModule(),
-          swiftAttr->getAttribute());
+      bool cached = true;
+      while (true) {
+        // Dig out a source file we can use for parsing.
+        auto &sourceFile = getClangSwiftAttrSourceFile(
+            *MappedDecl->getDeclContext()->getParentModule(),
+            swiftAttr->getAttribute(),
+            cached);
+
+        auto topLevelDecls = sourceFile.getTopLevelDecls();
+
+        // If we're using the cached version, check whether we can correctly
+        // clone the attribute.
+        if (cached) {
+          bool hasNonclonableAttribute = false;
+          for (auto decl : topLevelDecls) {
+            if (hasNonclonableAttribute)
+              break;
+
+            for (auto attr : decl->getAttrs()) {
+              if (!attr->canClone()) {
+                hasNonclonableAttribute = true;
+                break;
+              }
+            }
+          }
+
+          // We cannot clone one of the attributes. Go back and build a new
+          // source file without caching it.
+          if (hasNonclonableAttribute) {
+            cached = false;
+            continue;
+          }
+        }
 
-      // Collect the attributes from the synthesized top-level declaration in
-      // the source file.
-      auto topLevelDecls = sourceFile.getTopLevelDecls();
-      for (auto decl : topLevelDecls) {
-        for (auto attr : decl->getAttrs())
-          MappedDecl->getAttrs().add(attr->clone(SwiftContext));
+        // Collect the attributes from the synthesized top-level declaration in
+        // the source file. If we're using a cached copy, clone the attribute.
+        for (auto decl : topLevelDecls) {
+          SmallVector<DeclAttribute *, 2> attrs(decl->getAttrs().begin(),
+                                                decl->getAttrs().end());
+          for (auto attr : attrs) {
+            MappedDecl->getAttrs().add(cached ? attr->clone(SwiftContext)
+                                              : attr);
+          }
+        }
+
+        break;
       }
     }
 

lib/ClangImporter/ImporterImpl.h

@@ -1057,7 +1057,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
 
   /// Retrieve the placeholder source file for use in parsing Swift attributes
   /// in the given module.
-  SourceFile &getClangSwiftAttrSourceFile(ModuleDecl &module, StringRef attributeText);
+  SourceFile &getClangSwiftAttrSourceFile(
+      ModuleDecl &module, StringRef attributeText, bool cached);
 
   /// Utility function to import Clang attributes from a source Swift decl to
   /// synthesized Swift decl.

test/Inputs/clang-importer-sdk/usr/include/completion_handler_globals.h

@@ -1,9 +1,11 @@
 #if __SWIFT_ATTR_SUPPORTS_MACROS
 #define ADD_ASYNC __attribute__((swift_attr("@macro_library.AddAsync")))
 #define ADD_ASYNC_FINAL __attribute__((swift_attr("@macro_library.AddAsyncFinal")))
+#define DO_SOMETHING_DOTTED __attribute__((swift_attr("@AcceptDotted(.something)")))
 #else
 #define ADD_ASYNC
 #define ADD_ASYNC_FINAL
+#define DO_SOMETHING_DOTTED
 #endif
 
 void async_divide(double x, double y, void (^ _Nonnull completionHandler)(double x)) ADD_ASYNC;
@@ -15,6 +17,9 @@ void computer_divide(const SlowComputer *computer, double x, double y, void (^ _
   ADD_ASYNC
   __attribute__((swift_name("SlowComputer.divide(self:_:_:completionHandler:)")));
 
+void f1(double x) DO_SOMETHING_DOTTED;
+void f2(double x) DO_SOMETHING_DOTTED;
+void f3(double x) DO_SOMETHING_DOTTED;
 
 #if __OBJC__
 @import Foundation;

test/Macros/Inputs/macro_library.swift

@@ -59,3 +59,10 @@ public macro AddAsync() = #externalMacro(module: "MacroDefinition", type: "AddAs
 
 @attached(peer, names: overloaded)
 public macro AddAsyncFinal() = #externalMacro(module: "MacroDefinition", type: "AddAsyncMacro")
+
+public enum Something {
+case something
+}
+
+@attached(peer, names: overloaded)
+public macro AcceptedDotted(_: Something) = #externalMacro(module: "MacroDefinition", type: "EmptyPeerMacro")

test/Macros/expand_on_imported.swift

@@ -20,6 +20,10 @@ import macro_library
 func testAll(x: Double, y: Double, computer: SlowComputer) async {
   let _: Double = await async_divide(1.0, 2.0)
   let _: Double = await computer.divide(x, y)
+
+  f1(3.14159)
+  f2(3.14159)
+  f3(3.14159)
 }
 
 // CHECK: define{{.*}}@"$sSC12async_divideyS2d_SdtYaF"