Adding clone_attrs attribute.

[plotfi]

Summary

This is just experimental for the time being. Posting to phab to get some comments.

[plotfi]

This is pretty incomplete. Please ignore for now.

[aaron.ballman]

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

[plotfi]

Thanks for the feedback on corner cases @aaron.ballman, this will give me more concrete things to think about here.

At the moment I mainly thinking about a case:

typedef int foo;
enum : foo {} __attribute__((__clone_attrs_from__(foo)));

This is mostly because of how NS_OPTIONS enums on Darwin are defined through a macro.
We have been looking into breaking up how the typedef and the enum are defined for better behavior with C++.
The only challenge is that if an NS_OPTIONS macro is used to declare an enum and an attribute is specified either before or after, it will then only apply to either the typedef or the enum.

Thats the reason for a clone_attrs attribute.

I'll update description soon.

In D128249#3598507, @aaron.ballman wrote:

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

[aaron.ballman]

In D128249#3599551, @plotfi wrote:

Thanks for the feedback on corner cases @aaron.ballman, this will give me more concrete things to think about here.

At the moment I mainly thinking about a case:

typedef int foo;
enum : foo {} __attribute__((__clone_attrs_from__(foo)));

This is mostly because of how NS_OPTIONS enums on Darwin are defined through a macro.
We have been looking into breaking up how the typedef and the enum are defined for better behavior with C++.
The only challenge is that if an NS_OPTIONS macro is used to declare an enum and an attribute is specified either before or after, it will then only apply to either the typedef or the enum.

Thats the reason for a clone_attrs attribute.

Ah, thank you for the explanation. That is an interesting case. Another potential idea to consider is the idea of "inheriting" attributes from a related interface. e.g., a derived class which inherits attributes from the base, or an enum with a fixed underlying type inheriting attributes from the fixed type, etc. I've not given it a lot of thought, but it seems like there's a relationship there that might make sense for attributes in general. As an example:

struct [[gnu::packed]] S {
  char c;
  int i;
};

struct T {
  char c;
  int i;
};

template <typename U>
struct [[inherits_attributes]] V : U {
  double d;
};

Where instantiating V<S> would result in both V and S being packed, but instantiating V<T> would not pack either. (Again, I've not put a ton of thought into it, but it's a possible alternative design worth considering.)

I'll update description soon.

In D128249#3598507, @aaron.ballman wrote:

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

clang/include/clang/Basic/Attr.td

 }
 
 def SwiftPrivate : InheritableAttr {
   let Spellings = [GNU<"swift_private">];
   let Documentation = [SwiftPrivateDocs];
   let SimpleHandler = 1;
 }
 
+def CloneAttrsFrom : InheritableAttr {
+  let Spellings = [GNU<"clone_attrs_from">];
+  let Args = [DeclArgument<Named, "from">];
+  let Subjects = SubjectList<[Tag, TypedefName],
+                             ErrorDiag>;
+  let Documentation = [CloneAttrsFromDocs];
+}
+
 def NoDeref : TypeAttr {
   let Spellings = [Clang<"noderef">];
   let Documentation = [NoDerefDocs];
 }
 
 def ReqdWorkGroupSize : InheritableAttr {
   // Does not have a [[]] spelling because it is an OpenCL-related attribute.
   let Spellings = [GNU<"reqd_work_group_size">];

clang/include/clang/Basic/AttrDocs.td

 framework client but are still made available for use within the framework or
 Swift SDK overlay.
 
 The purpose of this attribute is to permit a more idomatic implementation of
 declarations in Swift while hiding the non-idiomatic one.
   }];
 }
 
+def CloneAttrsFromDocs : Documentation {
+  let Category = DocCatDecl;
+  let Heading = "clone_attrs_from";
+  let Content = [{
+The ``clone_attrs_from`` attribute indicates that the declaration to which the
+attribute appertains contains the same attributes as another named decl.
+  }];
+}
+
 def OMPDeclareSimdDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "#pragma omp declare simd";
   let Content = [{
 The ``declare simd`` construct can be applied to a function to enable the creation
 of one or more versions that can process multiple arguments using SIMD
 instructions from a single invocation in a SIMD loop. The ``declare simd``
 directive is a declarative directive. There may be multiple ``declare simd``

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_attribute_sentinel_not_variadic : Warning<
   InGroup<IgnoredAttributes>;
 def warn_deprecated_ignored_on_using : Warning<
   "%0 currently has no effect on a using declaration">,
   InGroup<IgnoredAttributes>;
 def err_attribute_sentinel_less_than_zero : Error<
   "'sentinel' parameter 1 less than zero">;
 def err_attribute_sentinel_not_zero_or_one : Error<
   "'sentinel' parameter 2 not 0 or 1">;
+def err_attribute_clone_attrs_from_arg_not_named_decl : Error<
+  "'clone_attrs_from' argument %select{|%1 |%1 }0is not a %select{||single }0named decl">;
+def warn_clone_attrs_from_ext : Warning<
+  "Clang does not allow the 'clone_attrs_from' attribute argument to be anything other "
+  "than a simple identifier">,
+  InGroup<GccCompat>;
 def warn_cleanup_ext : Warning<
   "GCC does not allow the 'cleanup' attribute argument to be anything other "
   "than a simple identifier">,
   InGroup<GccCompat>;
 def err_attribute_cleanup_arg_not_function : Error<
   "'cleanup' argument %select{|%1 |%1 }0is not a %select{||single }0function">;
 def err_attribute_cleanup_func_must_take_one_arg : Error<
   "'cleanup' function %0 must take 1 parameter">;

clang/lib/Sema/SemaDeclAttr.cpp

static void handleMinVectorWidthAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (Existing && Existing->getVectorWidth() != VecWidth) {
     S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
     return;
   }
 
   D->addAttr(::new (S.Context) MinVectorWidthAttr(S.Context, AL, VecWidth));
 }
 
+static void handleCloneAttrsFrom(Sema &S, Decl *D, const ParsedAttr &AL) {
+
+  if (AL.getNumArgs() != 1) {
+    S.Diag(D->getLocation(), diag::err_attribute_clone_attrs_from_arg_not_named_decl) << 0;
+    return;
+  }
+
+  Expr *E = AL.getArgAsExpr(0);
+  SourceLocation Loc = E->getExprLoc();
+
+  auto *DRE = dyn_cast<DeclRefExpr>(E);
+  if (!DRE) {
+    S.Diag(Loc, diag::err_attribute_clone_attrs_from_arg_not_named_decl) << 0;
+    return;
+  }
+
+  if (DRE->hasQualifier())
+    S.Diag(Loc, diag::warn_clone_attrs_from_ext);
+
+  NamedDecl *ND = dyn_cast<NamedDecl>(DRE->getDecl());
+  if (!ND) {
+    DeclarationNameInfo NI = DRE->getNameInfo();
+    S.Diag(Loc, diag::err_attribute_clone_attrs_from_arg_not_named_decl)
+        << 1 << NI.getName();
+    return;
+  }
+
+  ND->addAttr(::new (S.Context) CloneAttrsFromAttr(S.Context, AL, ND));
+  D->addAttr(::new (S.Context) CloneAttrsFromAttr(S.Context, AL, ND));
+}
+
 static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   Expr *E = AL.getArgAsExpr(0);
   SourceLocation Loc = E->getExprLoc();
   FunctionDecl *FD = nullptr;
   DeclarationNameInfo NI;
 
   // gcc only allows for simple identifiers. Since we support more than gcc, we
   // will warn the user.
case ParsedAttr::AT_SwiftNewType:
     break;
   case ParsedAttr::AT_SwiftAsync:
     handleSwiftAsyncAttr(S, D, AL);
     break;
   case ParsedAttr::AT_SwiftAsyncError:
     handleSwiftAsyncError(S, D, AL);
     break;
 
+  case ParsedAttr::AT_CloneAttrsFrom:
+    handleCloneAttrsFrom(S, D, AL);
+    break;
+
   // XRay attributes.
   case ParsedAttr::AT_XRayLogArgs:
     handleXRayLogArgsAttr(S, D, AL);
     break;
 
   case ParsedAttr::AT_PatchableFunctionEntry:
     handlePatchableFunctionEntryAttr(S, D, AL);
     break;

clang/test/AST/attr-swift_bridge.m

-// RUN: %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
-struct __attribute__((__swift_bridge__("BridgedS"))) S;
-// CHECK: RecordDecl {{.*}} struct S
-// CHECK: SwiftBridgeAttr {{.*}} "BridgedS"
+int XXXX;
 
-struct S {
-};
-
-// CHECK: RecordDecl {{.*}} struct S definition
-// CHECK: SwiftBridgeAttr {{.*}} Inherited "BridgedS"
+struct __attribute__((__clone_attrs_from__(XXXX))) S;