Formalize the notion of "can clone" so we don't try to clone an attribute that won't work

Author: DougGregor

include/swift/AST/Attr.h

@@ -504,13 +504,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>
@@ -1908,9 +1912,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"
+  }
+}
+
 bool
 DeclAttributes::isUnavailableInSwiftVersion(
   const version::Version &effectiveVersion) const {

lib/ClangImporter/ImportDecl.cpp

@@ -8437,26 +8437,29 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
               break;
 
             for (auto attr : decl->getAttrs()) {
-              if (auto customAttr = dyn_cast<CustomAttr>(attr)) {
-                if (customAttr->getArgs() != nullptr) {
-                  hasNonclonableAttribute = true;
-                  break;
-                }
+              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.
+        // the source file. If we're using a cached copy, clone the attribute.
         for (auto decl : topLevelDecls) {
-          for (auto attr : decl->getAttrs()) {
-            MappedDecl->getAttrs().add(attr->clone(SwiftContext));
+          SmallVector<DeclAttribute *, 2> attrs(decl->getAttrs().begin(),
+                                                decl->getAttrs().end());
+          for (auto attr : attrs) {
+            MappedDecl->getAttrs().add(cached ? attr->clone(SwiftContext)
+                                              : attr);
           }
         }