[cfe-commits] [PATCH] Check for internal weak decls after merging.

[Rafael Ávila de Espíndola]

This fixes pr14946 and changes our behaviour when instantiating weak decls in
templates with internal type arguments. We now match gcc. In the testcase in
attr-weak.cpp gcc says:

test.cpp: In instantiation of ‘struct Test7<{anonymous}::Internal>’:
test.cpp:8:17:   required from here
test.cpp:5:6: error: weak declaration of ‘void Test7<T>::test7() [with T = {anonymous}::Internal]’ must be public
 void Test7<T>::test7() {
      ^
---
 lib/Sema/SemaDecl.cpp      | 21 +++++++++++++++++----
 lib/Sema/SemaDeclAttr.cpp  |  6 ------
 test/Sema/attr-weak.c      |  6 ++++++
 test/SemaCXX/attr-weak.cpp |  4 ++--
 4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 5d655e2..3357cbd 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4707,6 +4707,15 @@ static bool mayConflictWithNonVisibleExternC(const T *ND) {
   return ND->getDeclContext()->isTranslationUnit();
 }
 
+static void checkAttributesAfterMerging(Sema &S, NamedDecl &ND) {
+  // 'weak' only applies to declarations with external linkage.
+  WeakAttr *WA = ND.getAttr<WeakAttr>();
+  if (WA && ND.getLinkage() != ExternalLinkage) {
+    S.Diag(WA->getLocation(), diag::err_attribute_weak_static);
+    ND.dropAttr<WeakAttr>();
+  }
+}
+
 /// \brief Perform semantic checking on a newly-created variable
 /// declaration.
 ///
@@ -4847,11 +4856,13 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD,
     return false;
   }
 
-  if (!Previous.empty()) {
+  bool IsRedeclaration = !Previous.empty();
+  if (IsRedeclaration)
     MergeVarDecl(NewVD, Previous);
-    return true;
-  }
-  return false;
+
+  checkAttributesAfterMerging(*this, *NewVD);
+
+  return IsRedeclaration;
 }
 
 /// \brief Data used with FindOverriddenMethod
@@ -6284,6 +6295,8 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     }
   }
 
+  checkAttributesAfterMerging(*this, *NewFD);
+
   // Semantic checking for this function declaration (in isolation).
   if (getLangOpts().CPlusPlus) {
     // C++-specific checks.
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 85f48ec..efeafa6 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -2511,12 +2511,6 @@ static void handleWeakAttr(Sema &S, Decl *D, const AttributeList &Attr) {
 
   NamedDecl *nd = cast<NamedDecl>(D);
 
-  // 'weak' only applies to declarations with external linkage.
-  if (hasEffectivelyInternalLinkage(nd)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_weak_static);
-    return;
-  }
-
   nd->addAttr(::new (S.Context) WeakAttr(Attr.getRange(), S.Context));
 }
 
diff --git a/test/Sema/attr-weak.c b/test/Sema/attr-weak.c
index adedf12..df74554 100644
--- a/test/Sema/attr-weak.c
+++ b/test/Sema/attr-weak.c
@@ -16,3 +16,9 @@ static int x __attribute__((weak)); // expected-error {{weak declaration cannot
 // rdar://9538608
 int C; // expected-note {{previous definition is here}}
 extern int C __attribute__((weak_import)); // expected-warning {{an already-declared variable is made a weak_import declaration}}
+
+static int pr14946_x;
+extern int pr14946_x  __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
+
+static void pr14946_f();
+void pr14946_f() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
diff --git a/test/SemaCXX/attr-weak.cpp b/test/SemaCXX/attr-weak.cpp
index b6a9e0a..cabe8bb 100644
--- a/test/SemaCXX/attr-weak.cpp
+++ b/test/SemaCXX/attr-weak.cpp
@@ -22,8 +22,8 @@ namespace {
 }
 
 template <class T> struct Test7 {
-  void test7() __attribute__((weak)) {}
+  void test7() __attribute__((weak)) {} // expected-error {{weak declaration cannot have internal linkage}}
 };
 namespace { class Internal; }
-template struct Test7<Internal>;
+template struct Test7<Internal>; // expected-note {{in instantiation of template class 'Test7<<anonymous>::Internal>' requested here}}
 template struct Test7<int>;
-- 
1.7.11.7