Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis closed this revision.
eugenis added a comment.

r252648.
Thanks everyone for the very detailed review!


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39751.
eugenis added a comment.

Rebase.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/attr-notail.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+
+class A;
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  void f4(); // expected-note{{previous definition is here}}
+  static int zz; // expected-note{{previous definition is here}}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+__attribute__((internal_linkage)) void A::f4() {} // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'f4'}}
+
+__attribute__((internal_linkage)) int A::zz; // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'zz'}}
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
+
+void g(int a [[clang::internal_linkage]]) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  int x [[clang::internal_linkage]]; // expected-warning{{'internal_linkage' attribute on a non-static local variable is ignored}}
+  static int y [[clang::internal_linkage]];
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+// expected-note{{previous definition is here}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}} \
+// expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'var4'}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-notail.c
===
--- test/Sema/attr-notail.c
+++ test/Sema/attr-notail.c
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-int callee0() __attribute__((not_tail_called,always_inline)); // expected-error{{'not_tail_called' and 'always_inline' attributes are not compatible}}
-int callee1() __attribute__((always_inline,not_tail_called)); // expected-error{{'always_inline' and 'not_tail_called' attributes are not 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

Richard, are you OK with this?


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/Sema/SemaDeclAttr.cpp:3407
@@ +3406,3 @@
+ : ExpectedVariableOrFunction);
+D->dropAttr();
+  }

aaron.ballman wrote:
> Why is this dropping AlwaysInlineAttr instead of returning a nullptr?
A copy/paste error. Fixed.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39389.
eugenis marked an inline comment as done.
eugenis added a comment.

Added the new warning to a -W group.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+
+class A;
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  void f4(); // expected-note{{previous definition is here}}
+  static int zz; // expected-note{{previous definition is here}}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+__attribute__((internal_linkage)) void A::f4() {} // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'f4'}}
+
+__attribute__((internal_linkage)) int A::zz; // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'zz'}}
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
+
+void g(int a [[clang::internal_linkage]]) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  int x [[clang::internal_linkage]]; // expected-warning{{'internal_linkage' attribute on a non-static local variable is ignored}}
+  static int y [[clang::internal_linkage]];
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+// expected-note{{previous definition is here}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}} \
+// expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'var4'}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:6609
@@ +6608,3 @@
+
+  if (NewVD->hasLocalStorage() && NewVD->hasAttr()) {
+Diag(NewVD->getLocation(), diag::warn_internal_linkage_local_storage);

Is there a reason this change cannot go into SemaDeclAttr.cpp instead? That way 
we don't bother to attach the attribute in the first place.


Comment at: lib/Sema/SemaDeclAttr.cpp:3407
@@ +3406,3 @@
+ : ExpectedVariableOrFunction);
+D->dropAttr();
+  }

Why is this dropping AlwaysInlineAttr instead of returning a nullptr?


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39249.

Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+
+class A;
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  void f4(); // expected-note{{previous definition is here}}
+  static int zz; // expected-note{{previous definition is here}}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+__attribute__((internal_linkage)) void A::f4() {} // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'f4'}}
+
+__attribute__((internal_linkage)) int A::zz; // expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'zz'}}
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
+
+void g(int a [[clang::internal_linkage]]) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  int x [[clang::internal_linkage]]; // expected-warning{{'internal_linkage' attribute on a non-static local variable is ignored}}
+  static int y [[clang::internal_linkage]];
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+// expected-note{{previous definition is here}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}} \
+// expected-error{{'internal_linkage' attribute does not appear on the first declaration of 'var4'}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}}
-int baz() __attribute__((__cold__)) 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39245.

Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+
+class A;
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  void f4(); // expected-note{{previous definition is here}}
+  static int zz; // expected-note{{previous definition is here}}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+__attribute__((internal_linkage)) void A::f4() {} // expected-error{{internal_linkage attribute does not appear on the first declaration of 'f4'}}
+
+__attribute__((internal_linkage)) int A::zz; // expected-error{{internal_linkage attribute does not appear on the first declaration of 'zz'}}
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+int hh(int a [[clang::internal_linkage]]) {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+// expected-note{{previous definition is here}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}} \
+// expected-error{{internal_linkage attribute does not appear on the first declaration of 'var4'}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}}
-int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}}
+int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
+int baz() 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

How do I check if a Var is a local variable?



Comment at: test/CodeGenCXX/attribute_internal_linkage.cpp:35-36
@@ +34,4 @@
+__attribute__((internal_linkage)) void A::f3() {
+}
+
+// Forward declaration w/o an attribute.

OK, done. Still allowing forward declaration of a class w/o the attribute 
though, it looks harmless.



Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: test/SemaCXX/internal_linkage.cpp:23
@@ +22,3 @@
+
+__attribute__((internal_linkage)) void A::f4() {} // 
expected-error{{'internal_linkage' attribute does not appear on the first 
declaration of 'f4'}}
+

Btw, this triggers on libc++ (if macro-replacing always_inline with 
internal_linkage) 400 times. No problem, should be easy to clean up.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

Hm, the current implementation allows all of the following:

void f(int a [[clang::internal_linkage]]) {  // 1

  int b [[clang::internal_linkage]];   // 2
  static int c [[clang::internal_linkage]];  // 3

}

I'll fix (1). Is it OK to allow (2) and (3)? The attribute has no effect 
because the declarations already have internal linkage, so I'd say it behaves 
as documented.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: test/CodeGenCXX/attribute_internal_linkage.cpp:34-35
@@ +33,4 @@
+
+__attribute__((internal_linkage)) void A::f3() {
+}
+

We should reject this. We're just causing problems for ourselves if we allow 
the linkage to change on a redeclaration.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Reid Kleckner via cfe-commits]

rnk added a comment.

This is an interesting test case, though:

  inline int foo() {
static int __attribute__((internal_linkage)) x;
return x++;
  }

If foo gets inlined, those call sites will use and update 'x'. If foo is not 
inlined, one definition of foo will win, and every caller will use its version 
of 'x'.

We could emit a warning, but I kind of don't care. If you're using 
internal_linkage, you are operating outside the rules of C++. You're expecting 
multiple copies of these things to be emitted.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

I would like to see one more test, just to make sure that a Var subject doesn't 
also allow it on a parameter:

  void f(int a [[clang::internal_linkage]]);

Aside from that, LGTM!


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Aaron Ballman via cfe-commits]

On Tue, Nov 3, 2015 at 7:23 PM, Richard Smith  wrote:
> rsmith added inline comments.
>
> 
> Comment at: include/clang/Basic/AttrDocs.td:1628
> @@ +1627,3 @@
> +The ``internal_linkage`` attribute changes the linkage type of the 
> declaration to internal.
> +This is similar to C-style ``static``, but can be used on classes and class 
> methods
> +This can be used to contain the ABI of a C++ library by excluding unwanted 
> class methods from the export tables.
> 
> Missing period at end of sentence.
>
> 
> Comment at: include/clang/Basic/AttrDocs.td:1628
> @@ +1627,3 @@
> +The ``internal_linkage`` attribute changes the linkage type of the 
> declaration to internal.
> +This is similar to C-style ``static``, but can be used on classes and class 
> methods
> +This can be used to contain the ABI of a C++ library by excluding unwanted 
> class methods from the export tables.
> 
> rsmith wrote:
>> Missing period at end of sentence.
> Please also say what it means if the attribute is applied to a class.
>
> 
> Comment at: lib/AST/Decl.cpp:635-641
> @@ -634,2 +634,9 @@
>assert(!isa(D) && "Didn't expect a FieldDecl!");
>
> +  for (const DeclContext *DC = D->getDeclContext();
> +   !isa(DC); DC = DC->getParent()) {
> +const NamespaceDecl *ND = dyn_cast(DC);
> +if (ND && ND->getAttr())
> +  return LinkageInfo::internal();
> +  }
> +
> 
> Dead code?
>
> 
> Comment at: lib/AST/Decl.cpp:1362-1367
> @@ -1346,4 +1361,8 @@
>  }
> -assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
> +// Linkages may also differ if one of the declarations has
> +// InternalLinkageAttr.
> +assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
> +   (Old->hasAttr() !=
> +D->hasAttr()));
>  #endif
>
> 
> We should not introduce another case where the linkage of an entity can 
> change after its first declaration. It seems reasonable to require this 
> attribute to be on the first declaration of the function.
>
> 
> Comment at: lib/Sema/SemaDeclAttr.cpp:3391
> @@ +3390,3 @@
> +  unsigned AttrSpellingListIndex) {
> +  if (checkAttrMutualExclusion(*this, D, Range, Ident))
> +return nullptr;
> 
> Aaron, could we move the mutual exclusion functionality to TableGen? 
> (Separately from this patch, of course.) It looks like there are now 6 
> attributes that could use the "simple attribute" codepath if we did so.

Yes, I think that would be a good idea. Ideally, I would like the
tablegen to look like:

class Attr {
  // ...
  list MutuallyExclusive = [];
}

def Hot {
  // ...
  let MutuallyExclusive = [Cold];
}

def Cold {
  // ...
  let MutuallyExclusive = [Hot];
}

I'll put this on my list of refactorings to do.

~Aaron

>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D13925
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/AST/Decl.cpp:635-641
@@ -634,2 +634,9 @@
   assert(!isa(D) && "Didn't expect a FieldDecl!");
 
+  for (const DeclContext *DC = D->getDeclContext();
+   !isa(DC); DC = DC->getParent()) {
+const NamespaceDecl *ND = dyn_cast(DC);
+if (ND && ND->getAttr())
+  return LinkageInfo::internal();
+  }
+

rsmith wrote:
> Dead code?
Right. Removed.


Comment at: lib/AST/Decl.cpp:1362-1367
@@ -1346,4 +1361,8 @@
 }
-assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
+// Linkages may also differ if one of the declarations has
+// InternalLinkageAttr.
+assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
+   (Old->hasAttr() !=
+D->hasAttr()));
 #endif
 

rsmith wrote:
> We should not introduce another case where the linkage of an entity can 
> change after its first declaration. It seems reasonable to require this 
> attribute to be on the first declaration of the function.
This is strange, I can no longer trigger this code path.
I wonder if the change that added an attribute check to isExternallyVisible 
made this special case unnecessary?
Reverting this chunk.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/AttrDocs.td:1628
@@ +1627,3 @@
+The ``internal_linkage`` attribute changes the linkage type of the declaration 
to internal.
+This is similar to C-style ``static``, but can be used on classes and class 
methods
+This can be used to contain the ABI of a C++ library by excluding unwanted 
class methods from the export tables.

Missing period at end of sentence.


Comment at: include/clang/Basic/AttrDocs.td:1628
@@ +1627,3 @@
+The ``internal_linkage`` attribute changes the linkage type of the declaration 
to internal.
+This is similar to C-style ``static``, but can be used on classes and class 
methods
+This can be used to contain the ABI of a C++ library by excluding unwanted 
class methods from the export tables.

rsmith wrote:
> Missing period at end of sentence.
Please also say what it means if the attribute is applied to a class.


Comment at: lib/AST/Decl.cpp:635-641
@@ -634,2 +634,9 @@
   assert(!isa(D) && "Didn't expect a FieldDecl!");
 
+  for (const DeclContext *DC = D->getDeclContext();
+   !isa(DC); DC = DC->getParent()) {
+const NamespaceDecl *ND = dyn_cast(DC);
+if (ND && ND->getAttr())
+  return LinkageInfo::internal();
+  }
+

Dead code?


Comment at: lib/AST/Decl.cpp:1362-1367
@@ -1346,4 +1361,8 @@
 }
-assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
+// Linkages may also differ if one of the declarations has
+// InternalLinkageAttr.
+assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
+   (Old->hasAttr() !=
+D->hasAttr()));
 #endif
 

We should not introduce another case where the linkage of an entity can change 
after its first declaration. It seems reasonable to require this attribute to 
be on the first declaration of the function.


Comment at: lib/Sema/SemaDeclAttr.cpp:3391
@@ +3390,3 @@
+  unsigned AttrSpellingListIndex) {
+  if (checkAttrMutualExclusion(*this, D, Range, Ident))
+return nullptr;

Aaron, could we move the mutual exclusion functionality to TableGen? 
(Separately from this patch, of course.) It looks like there are now 6 
attributes that could use the "simple attribute" codepath if we did so.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Aaron Ballman via cfe-commits]

On Tue, Nov 3, 2015 at 7:44 PM, Richard Smith  wrote:
> On Tue, Nov 3, 2015 at 4:37 PM, Aaron Ballman via cfe-commits
>  wrote:
>>
>> On Tue, Nov 3, 2015 at 7:23 PM, Richard Smith 
>> wrote:
>> > rsmith added inline comments.
>> >
>> > 
>> > Comment at: include/clang/Basic/AttrDocs.td:1628
>> > @@ +1627,3 @@
>> > +The ``internal_linkage`` attribute changes the linkage type of the
>> > declaration to internal.
>> > +This is similar to C-style ``static``, but can be used on classes and
>> > class methods
>> > +This can be used to contain the ABI of a C++ library by excluding
>> > unwanted class methods from the export tables.
>> > 
>> > Missing period at end of sentence.
>> >
>> > 
>> > Comment at: include/clang/Basic/AttrDocs.td:1628
>> > @@ +1627,3 @@
>> > +The ``internal_linkage`` attribute changes the linkage type of the
>> > declaration to internal.
>> > +This is similar to C-style ``static``, but can be used on classes and
>> > class methods
>> > +This can be used to contain the ABI of a C++ library by excluding
>> > unwanted class methods from the export tables.
>> > 
>> > rsmith wrote:
>> >> Missing period at end of sentence.
>> > Please also say what it means if the attribute is applied to a class.
>> >
>> > 
>> > Comment at: lib/AST/Decl.cpp:635-641
>> > @@ -634,2 +634,9 @@
>> >assert(!isa(D) && "Didn't expect a FieldDecl!");
>> >
>> > +  for (const DeclContext *DC = D->getDeclContext();
>> > +   !isa(DC); DC = DC->getParent()) {
>> > +const NamespaceDecl *ND = dyn_cast(DC);
>> > +if (ND && ND->getAttr())
>> > +  return LinkageInfo::internal();
>> > +  }
>> > +
>> > 
>> > Dead code?
>> >
>> > 
>> > Comment at: lib/AST/Decl.cpp:1362-1367
>> > @@ -1346,4 +1361,8 @@
>> >  }
>> > -assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
>> > +// Linkages may also differ if one of the declarations has
>> > +// InternalLinkageAttr.
>> > +assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
>> > +   (Old->hasAttr() !=
>> > +D->hasAttr()));
>> >  #endif
>> >
>> > 
>> > We should not introduce another case where the linkage of an entity can
>> > change after its first declaration. It seems reasonable to require this
>> > attribute to be on the first declaration of the function.
>> >
>> > 
>> > Comment at: lib/Sema/SemaDeclAttr.cpp:3391
>> > @@ +3390,3 @@
>> > +  unsigned AttrSpellingListIndex) {
>> > +  if (checkAttrMutualExclusion(*this, D, Range,
>> > Ident))
>> > +return nullptr;
>> > 
>> > Aaron, could we move the mutual exclusion functionality to TableGen?
>> > (Separately from this patch, of course.) It looks like there are now 6
>> > attributes that could use the "simple attribute" codepath if we did so.
>>
>> Yes, I think that would be a good idea. Ideally, I would like the
>> tablegen to look like:
>>
>> class Attr {
>>   // ...
>>   list MutuallyExclusive = [];
>> }
>>
>> def Hot {
>>   // ...
>>   let MutuallyExclusive = [Cold];
>> }
>>
>> def Cold {
>>   // ...
>>   let MutuallyExclusive = [Hot];
>> }
>>
>> I'll put this on my list of refactorings to do.
>
>
> It'd be nice if we could also diagnose inconsistencies in this (eg, Hot says
> it's exclusive with Cold and Cold doesn't agree).

Already planned (as a warning, because you can infer the correct
behavior anyway). Also, this can generate some documentation
automatically for those attributes.

~Aaron

>
>>
>> ~Aaron
>>
>> >
>> >
>> > Repository:
>> >   rL LLVM
>> >
>> > http://reviews.llvm.org/D13925
>> >
>> >
>> >
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39135.
eugenis marked 2 inline comments as done.

Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}}
-int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}}
+int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
+int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
Index: test/CodeGenCXX/attribute_internal_linkage.cpp
===
--- /dev/null
+++ test/CodeGenCXX/attribute_internal_linkage.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((internal_linkage)) void f() {}
+// CHECK-DAG: define internal void @_ZL1fv
+
+class A {
+public:
+  static int y __attribute__((internal_linkage));
+  

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39124.
eugenis marked 5 inline comments as done.
eugenis added a comment.

Disabled the new attribute on namespaces.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}}
-int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}}
+int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
+int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
Index: test/CodeGenCXX/attribute_internal_linkage.cpp
===
--- /dev/null
+++ test/CodeGenCXX/attribute_internal_linkage.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((internal_linkage)) void f() {}
+// CHECK-DAG: define internal void @_ZL1fv
+
+class A 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Richard Smith via cfe-commits]

On Tue, Nov 3, 2015 at 4:37 PM, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Nov 3, 2015 at 7:23 PM, Richard Smith 
> wrote:
> > rsmith added inline comments.
> >
> > 
> > Comment at: include/clang/Basic/AttrDocs.td:1628
> > @@ +1627,3 @@
> > +The ``internal_linkage`` attribute changes the linkage type of the
> declaration to internal.
> > +This is similar to C-style ``static``, but can be used on classes and
> class methods
> > +This can be used to contain the ABI of a C++ library by excluding
> unwanted class methods from the export tables.
> > 
> > Missing period at end of sentence.
> >
> > 
> > Comment at: include/clang/Basic/AttrDocs.td:1628
> > @@ +1627,3 @@
> > +The ``internal_linkage`` attribute changes the linkage type of the
> declaration to internal.
> > +This is similar to C-style ``static``, but can be used on classes and
> class methods
> > +This can be used to contain the ABI of a C++ library by excluding
> unwanted class methods from the export tables.
> > 
> > rsmith wrote:
> >> Missing period at end of sentence.
> > Please also say what it means if the attribute is applied to a class.
> >
> > 
> > Comment at: lib/AST/Decl.cpp:635-641
> > @@ -634,2 +634,9 @@
> >assert(!isa(D) && "Didn't expect a FieldDecl!");
> >
> > +  for (const DeclContext *DC = D->getDeclContext();
> > +   !isa(DC); DC = DC->getParent()) {
> > +const NamespaceDecl *ND = dyn_cast(DC);
> > +if (ND && ND->getAttr())
> > +  return LinkageInfo::internal();
> > +  }
> > +
> > 
> > Dead code?
> >
> > 
> > Comment at: lib/AST/Decl.cpp:1362-1367
> > @@ -1346,4 +1361,8 @@
> >  }
> > -assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
> > +// Linkages may also differ if one of the declarations has
> > +// InternalLinkageAttr.
> > +assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
> > +   (Old->hasAttr() !=
> > +D->hasAttr()));
> >  #endif
> >
> > 
> > We should not introduce another case where the linkage of an entity can
> change after its first declaration. It seems reasonable to require this
> attribute to be on the first declaration of the function.
> >
> > 
> > Comment at: lib/Sema/SemaDeclAttr.cpp:3391
> > @@ +3390,3 @@
> > +  unsigned AttrSpellingListIndex) {
> > +  if (checkAttrMutualExclusion(*this, D, Range,
> Ident))
> > +return nullptr;
> > 
> > Aaron, could we move the mutual exclusion functionality to TableGen?
> (Separately from this patch, of course.) It looks like there are now 6
> attributes that could use the "simple attribute" codepath if we did so.
>
> Yes, I think that would be a good idea. Ideally, I would like the
> tablegen to look like:
>
> class Attr {
>   // ...
>   list MutuallyExclusive = [];
> }
>
> def Hot {
>   // ...
>   let MutuallyExclusive = [Cold];
> }
>
> def Cold {
>   // ...
>   let MutuallyExclusive = [Hot];
> }
>
> I'll put this on my list of refactorings to do.
>

It'd be nice if we could also diagnose inconsistencies in this (eg, Hot
says it's exclusive with Cold and Cold doesn't agree).


> ~Aaron
>
> >
> >
> > Repository:
> >   rL LLVM
> >
> > http://reviews.llvm.org/D13925
> >
> >
> >
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 39128.
eugenis added a comment.

One more test for forward declarations.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/attr-coldhot.c
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  };
+};
+
+namespace Z __attribute__((internal_linkage)) { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+}
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
+
+[[clang::internal_linkage]] int h() {}
+
+enum struct __attribute__((internal_linkage)) E { // expected-warning{{'internal_linkage' attribute only applies to variables, functions and classes}}
+  a = 1,
+  b = 2
+};
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+__attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_linkage' attribute only applies to variables and functions}}
+};
+
+__attribute__((internal_linkage("foo"))) int g() {} // expected-error{{'internal_linkage' attribute takes no arguments}}
Index: test/Sema/attr-coldhot.c
===
--- test/Sema/attr-coldhot.c
+++ test/Sema/attr-coldhot.c
@@ -6,5 +6,7 @@
 int var1 __attribute__((__cold__)); // expected-warning{{'__cold__' attribute only applies to functions}}
 int var2 __attribute__((__hot__)); // expected-warning{{'__hot__' attribute only applies to functions}}
 
-int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}}
-int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}}
+int qux() __attribute__((__hot__)) __attribute__((__cold__)); // expected-error{{'__hot__' and 'cold' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
+int baz() __attribute__((__cold__)) __attribute__((__hot__)); // expected-error{{'__cold__' and 'hot' attributes are not compatible}} \
+// expected-note{{conflicting attribute is here}}
Index: test/CodeGenCXX/attribute_internal_linkage.cpp
===
--- /dev/null
+++ test/CodeGenCXX/attribute_internal_linkage.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((internal_linkage)) void f() {}
+// CHECK-DAG: define internal void @_ZL1fv
+
+class A {
+public:
+  static int y 

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/Attr.td:2125
@@ +2124,3 @@
+
+def InternalLinkage : InheritableAttr {
+  let Spellings = [GNU<"internal_linkage">, CXX11<"clang", 
"internal_linkage">];

`InheritableAttr` doesn't seem right for the case when this is applied to a 
namespace.


Comment at: lib/AST/Decl.cpp:1358-1362
@@ -1346,3 +1357,7 @@
 }
-assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
+// Linkages may also differ if one of the declarations has
+// InternalLinkageAttr.
+assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
+   (Old->hasAttr() !=
+D->hasAttr()));
 #endif

We shouldn't allow the linkage to be changed after the first declaration. What 
cases cause problems here?


Comment at: lib/Sema/Sema.cpp:539
@@ -538,3 +538,3 @@
 
-if (!ND->isExternallyVisible()) {
+if (!ND->isExternallyVisible() || ND->hasAttr()) {
   S.Diag(ND->getLocation(), diag::warn_undefined_internal)

Hmm, what should `isExternallyVisible` return for a declaration with this 
attribute? Presumably it should be `false`...


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a reviewer: rnk.
rnk added a comment.
This revision is now accepted and ready to land.

I'm happy with this. Richard, what do you think?


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman requested changes to this revision.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
This revision now requires changes to proceed.

I would like to hold off on adding the namespace attribute. There were 
persuasive reasons to not have attributes on namespaces that was discussed in 
EWG in Kona, and this is a feature we could add on later if there's sufficient 
design consensus.



Comment at: include/clang/Basic/Attr.td:2125
@@ +2124,3 @@
+
+def InternalLinkage : InheritableAttr {
+  let Spellings = [GNU<"internal_linkage">, CXX11<"clang", 
"internal_linkage">];

rsmith wrote:
> `InheritableAttr` doesn't seem right for the case when this is applied to a 
> namespace.
Long and unusual is not a problem; that's why you can manually specify the 
diagnostic enum. This should be using Subjects unless it's realy onerous 
(or impossible).


Comment at: include/clang/Basic/AttrDocs.td:1619
@@ +1618,3 @@
+  let Content = [{
+The ``internal_linkage`` attribute changes the linkage type of the declaration 
to internal.
+  }];

Some examples would be nice, as well as an explanation as to why someone might 
want to do this.


Comment at: lib/Sema/SemaDeclAttr.cpp:3306
@@ +3305,3 @@
+
+  if (InternalLinkageAttr *Internal = D->getAttr()) {
+Diag(Range.getBegin(), diag::err_attributes_are_not_compatible)

Please use checkAttrMutualExclusion instead (and augment it with the 
note_conflicting_attribute).


Comment at: lib/Sema/SemaDeclAttr.cpp:3320
@@ +3319,3 @@
+
+  if (CommonAttr *Common = D->getAttr()) {
+Diag(Range.getBegin(), diag::err_attributes_are_not_compatible)

Please use checkAttrMutualExclusion instead.


Comment at: test/SemaCXX/internal_linkage.cpp:6
@@ +5,3 @@
+public:
+  int x __attribute__((internal_linkage)); // 
expected-warning{{'internal_linkage' attribute ignored}}
+  static int y __attribute__((internal_linkage));

This does not seem like a useful diagnostic as it doesn't tell the user *why* 
it was ignored.


Comment at: test/SemaCXX/internal_linkage.cpp:25
@@ +24,2 @@
+void A::f1() {
+}

Missing tests for internal_linkage accepting an argument (which should 
diagnose).
Missing tests for what happens with forward declarations and redeclarations 
that do not have matching attribute specifiers.
Missing tests for using the C++ spelling of the attribute.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

In http://reviews.llvm.org/D13925#276626, @majnemer wrote:

> No diagnostic is issued for the following C test case:
>
>   int x __attribute__((internal_linkage));
>   int x __attribute__((common));
>   int *f() { return  }


Thanks for noticing! Added missing attribute merging logic in SemaAttr.cpp.



Comment at: include/clang/Basic/Attr.td:2112
@@ +2111,3 @@
+
+def InternalLinkage : InheritableAttr {
+  let Spellings = [GCC<"internal_linkage">];

Because the list of subjects is long and unusual, and Subjects field does not 
support arbitrary lists - all combinations must have a specific diagnostic line 
elsewhere in the code.

This is checked in handleInternalLinkageAttr instead.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 38679.

Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute ignored}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute ignored}}
+  };
+};
+
+namespace Z __attribute__((internal_linkage)) {
+}
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+int var4 __attribute__((common)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}}
+int var4 __attribute__((internal_linkage)); // expected-note{{conflicting attribute is here}}
+
+int var5 __attribute__((internal_linkage)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}}
+int var5 __attribute__((common)); // expected-note{{conflicting attribute is here}}
+
+ __attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S {
+};
Index: test/CodeGenCXX/attribute_internal_linkage.cpp
===
--- /dev/null
+++ test/CodeGenCXX/attribute_internal_linkage.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((internal_linkage)) static void f() {}
+// CHECK-DAG: define internal void @_ZL1fv
+
+class A {
+public:
+  static int y __attribute__((internal_linkage));
+// CHECK-DAG: @_ZN1A1yE = internal global
+  void f1() __attribute__((internal_linkage));
+// CHECK-DAG: define internal void @_ZN1A2f1Ev
+  void f2() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1A2f2Ev
+  void f3();
+// CHECK-DAG: define internal void @_ZN1A2f3Ev
+  static void f4() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1A2f4Ev
+  A() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1AC1Ev
+// CHECK-DAG: define internal void @_ZN1AC2Ev
+  ~A() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1AD1Ev
+// CHECK-DAG: define internal void @_ZN1AD2Ev
+};
+
+int A::y;
+
+void A::f1() {
+}
+
+__attribute__((internal_linkage)) void A::f3() {
+}
+
+// Internal_linkage on a namespace affects everything within.
+namespace ZZZ __attribute__((internal_linkage)) {
+int x;
+// CHECK-DAG: @_ZNL3ZZZL1xE = internal global
+void f() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1fEv
+class A {
+public:
+  A() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AC1Ev
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AC2Ev
+  ~A() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AD1Ev
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AD2Ev
+  void g() {};
+// CHECK-DAG: define internal void @_ZNL3ZZZL1A1gEv
+};
+}
+
+// Internal_linkage on a class affects all its members.
+class __attribute__((internal_linkage)) B {
+public:
+  B() {}
+  // CHECK-DAG: define internal void @_ZNL1BC1Ev
+  // CHECK-DAG: define internal void @_ZNL1BC2Ev
+  ~B() {}
+  // CHECK-DAG: define internal void @_ZNL1BD1Ev
+  // CHECK-DAG: define internal void @_ZNL1BD2Ev
+  void f() {};
+  // CHECK-DAG: define internal void @_ZNL1B1fEv
+  static int x;
+  // CHECK-DAG: @_ZNL1B1xE = internal global
+};
+
+int B::x;
+
+void use() {
+  A a;
+  

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[David Majnemer via cfe-commits]

majnemer added a comment.

No diagnostic is issued for the following C test case:

  int x __attribute__((internal_linkage));
  int x __attribute__((common));
  int *f() { return  }



Comment at: include/clang/Basic/Attr.td:2112
@@ +2111,3 @@
+
+def InternalLinkage : InheritableAttr {
+  let Spellings = [GCC<"internal_linkage">];

Why doesn't this use `Subjects` ?


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 38071.
eugenis marked 2 inline comments as done.
eugenis added a comment.

This new version supports __attribute__((internal_linkage)) on classes and even 
namespaces!


Repository:
  rL LLVM

http://reviews.llvm.org/D13925

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/Decl.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/attribute_internal_linkage.cpp
  test/Sema/internal_linkage.c
  test/SemaCXX/internal_linkage.cpp

Index: test/SemaCXX/internal_linkage.cpp
===
--- /dev/null
+++ test/SemaCXX/internal_linkage.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int f() __attribute__((internal_linkage));
+class __attribute__((internal_linkage)) A {
+public:
+  int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute ignored}}
+  static int y __attribute__((internal_linkage));
+  void f1() __attribute__((internal_linkage));
+  void f2() __attribute__((internal_linkage)) {}
+  static void f3() __attribute__((internal_linkage)) {}
+  A() __attribute__((internal_linkage)) {}
+  ~A() __attribute__((internal_linkage)) {}
+  A& operator=(const A&) __attribute__((internal_linkage)) { return *this; }
+  struct {
+int z  __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute ignored}}
+  };
+};
+
+namespace Z __attribute__((internal_linkage)) {
+}
+
+int A::y;
+
+void A::f1() {
+}
Index: test/Sema/internal_linkage.c
===
--- /dev/null
+++ test/Sema/internal_linkage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int var __attribute__((internal_linkage));
+int var2 __attribute__((internal_linkage,common)); // expected-error{{'internal_linkage' and 'common' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+int var3 __attribute__((common,internal_linkage)); // expected-error{{'common' and 'internal_linkage' attributes are not compatible}} \
+   // expected-note{{conflicting attribute is here}}
+
+ __attribute__((internal_linkage)) int f() {}
+struct __attribute__((internal_linkage)) S {
+};
Index: test/CodeGenCXX/attribute_internal_linkage.cpp
===
--- /dev/null
+++ test/CodeGenCXX/attribute_internal_linkage.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((internal_linkage)) static void f() {}
+// CHECK-DAG: define internal void @_ZL1fv
+
+class A {
+public:
+  static int y __attribute__((internal_linkage));
+// CHECK-DAG: @_ZN1A1yE = internal global
+  void f1() __attribute__((internal_linkage));
+// CHECK-DAG: define internal void @_ZN1A2f1Ev
+  void f2() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1A2f2Ev
+  void f3();
+// CHECK-DAG: define internal void @_ZN1A2f3Ev
+  static void f4() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1A2f4Ev
+  A() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1AC1Ev
+// CHECK-DAG: define internal void @_ZN1AC2Ev
+  ~A() __attribute__((internal_linkage)) {}
+// CHECK-DAG: define internal void @_ZN1AD1Ev
+// CHECK-DAG: define internal void @_ZN1AD2Ev
+};
+
+int A::y;
+
+void A::f1() {
+}
+
+__attribute__((internal_linkage)) void A::f3() {
+}
+
+// Internal_linkage on a namespace affects everything within.
+namespace ZZZ __attribute__((internal_linkage)) {
+int x;
+// CHECK-DAG: @_ZNL3ZZZL1xE = internal global
+void f() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1fEv
+class A {
+public:
+  A() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AC1Ev
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AC2Ev
+  ~A() {}
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AD1Ev
+// CHECK-DAG: define internal void @_ZNL3ZZZL1AD2Ev
+  void g() {};
+// CHECK-DAG: define internal void @_ZNL3ZZZL1A1gEv
+};
+}
+
+// Internal_linkage on a class affects all its members.
+class __attribute__((internal_linkage)) B {
+public:
+  B() {}
+  // CHECK-DAG: define internal void @_ZNL1BC1Ev
+  // CHECK-DAG: define internal void @_ZNL1BC2Ev
+  ~B() {}
+  // CHECK-DAG: define internal void @_ZNL1BD1Ev
+  // CHECK-DAG: define internal void @_ZNL1BD2Ev
+  void f() {};
+  // CHECK-DAG: define internal void @_ZNL1B1fEv
+  static int x;
+  // CHECK-DAG: @_ZNL1B1xE = internal global
+};
+
+int B::x;
+
+void use() {
+  A a;
+  a.f1();
+  a.f2();
+  a.f3();
+  A::f4();
+  f();
+  int  = A::y;
+  ZZZ::A za;
+  za.g();
+  ZZZ::f();
+  int  = ZZZ::x;
+  B b;
+  b.f();
+  int  = B::x;
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/Sema/SemaDeclAttr.cpp:1580-1585
@@ -1577,3 +1579,8 @@
+
+  if (InternalLinkageAttr *Internal = D->getAttr()) {
+S.Diag(Attr.getRange().getBegin(), diag::warn_attribute_ignored)
+<< Attr.getName();
+S.Diag(Internal->getLocation(), diag::note_conflicting_attribute);
 return;
   }
 

eugenis wrote:
> majnemer wrote:
> > Why is this here?  You've already got logic for this in 
> > `handleInternalLinkageAttr`
> I think because the attributes may appear in different order. See for example 
> the cross-checks between AlwaysInlineAttr and OptimizeNoneAttr. I did not 
> test this.
Confirmed, we need both. See the test.



Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[David Majnemer via cfe-commits]

majnemer added a subscriber: majnemer.


Comment at: include/clang/Basic/Attr.td:2114
@@ +2113,3 @@
+  let Spellings = [GCC<"internal_linkage">];
+  let Subjects = SubjectList<[Function,Var]>;
+  let Documentation = [InternalLinkageDocs];

Space between `Function` and `Var`.


Comment at: lib/Sema/SemaDeclAttr.cpp:1580-1585
@@ -1577,3 +1579,8 @@
+
+  if (InternalLinkageAttr *Internal = D->getAttr()) {
+S.Diag(Attr.getRange().getBegin(), diag::warn_attribute_ignored)
+<< Attr.getName();
+S.Diag(Internal->getLocation(), diag::note_conflicting_attribute);
 return;
   }
 

Why is this here?  You've already got logic for this in 
`handleInternalLinkageAttr`


Comment at: lib/Sema/SemaDeclAttr.cpp:4563-4564
@@ +4562,4 @@
+  if (CommonAttr *Common = D->getAttr()) {
+S.Diag(Attr.getRange().getBegin(), diag::warn_attribute_ignored)
+<< Attr.getName();
+S.Diag(Common->getLocation(), diag::note_conflicting_attribute);

I'd expect a more serious diagnostic for a mismatch (an error) due to the 
nature of the attribute (namely, it's ABI implications).


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13925: Implement __attribute__((internal_linkage))

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/Sema/SemaDeclAttr.cpp:1580-1585
@@ -1577,3 +1579,8 @@
+
+  if (InternalLinkageAttr *Internal = D->getAttr()) {
+S.Diag(Attr.getRange().getBegin(), diag::warn_attribute_ignored)
+<< Attr.getName();
+S.Diag(Internal->getLocation(), diag::note_conflicting_attribute);
 return;
   }
 

majnemer wrote:
> Why is this here?  You've already got logic for this in 
> `handleInternalLinkageAttr`
I think because the attributes may appear in different order. See for example 
the cross-checks between AlwaysInlineAttr and OptimizeNoneAttr. I did not test 
this.


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits