[clang] 884772f - [Sema] Don't drop weak_import from a declaration if its definition isn't seen (#85886)

[via cfe-commits]

Author: Akira Hatanaka
Date: 2024-07-17T16:19:21-07:00
New Revision: 884772fdd6213c1bc16316b1e57fe08d85bdbc2d

URL: https://github.com/llvm/llvm-project/commit/884772fdd6213c1bc16316b1e57fe08d85bdbc2d
DIFF: https://github.com/llvm/llvm-project/commit/884772fdd6213c1bc16316b1e57fe08d85bdbc2d.diff

LOG: [Sema] Don't drop weak_import from a declaration if its definition isn't seen (#85886)

I believe this is what the original commit (33e022650adee965c65f9aea086ee74f3fd1bad5) was trying to do.

This fixes a bug where clang removes the attribute from a declaration that follows a declaration directly contained in a linkage-specification.

rdar://61865848

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaDecl.cpp
    clang/test/Sema/attr-weak.c
    clang/test/SemaCXX/attr-weak.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index de3d94155a9a0..b8a43b0a9fe8e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6075,7 +6075,7 @@ def note_extern_c_global_conflict : Note<
 def note_extern_c_begins_here : Note<
   "extern \"C\" language linkage specification begins here">;
 def warn_weak_import : Warning <
-  "an already-declared variable is made a weak_import declaration %0">;
+  "%0 cannot be declared 'weak_import' because its definition has been provided">;
 def ext_static_non_static : Extension<
   "redeclaring non-static %0 as static is a Microsoft extension">,
   InGroup<MicrosoftRedeclareStatic>;

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a3dd5ede9116a..6c3589bf87433 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4518,16 +4518,18 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
   }
 
   mergeDeclAttributes(New, Old);
-  // Warn if an already-declared variable is made a weak_import in a subsequent
+  // Warn if an already-defined variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr<WeakImportAttr>() &&
-      Old->getStorageClass() == SC_None &&
-      !Old->hasAttr<WeakImportAttr>()) {
-    Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
-    Diag(Old->getLocation(), diag::note_previous_declaration);
-    // Remove weak_import attribute on new declaration.
-    New->dropAttr<WeakImportAttr>();
-  }
+  if (New->hasAttr<WeakImportAttr>())
+    for (auto *D = Old; D; D = D->getPreviousDecl()) {
+      if (D->isThisDeclarationADefinition() != VarDecl::DeclarationOnly) {
+        Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
+        Diag(D->getLocation(), diag::note_previous_definition);
+        // Remove weak_import attribute on new declaration.
+        New->dropAttr<WeakImportAttr>();
+        break;
+      }
+    }
 
   if (const auto *ILA = New->getAttr<InternalLinkageAttr>())
     if (!Old->hasAttr<InternalLinkageAttr>()) {

diff  --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c
index b827d1539b997..f6482109bc9f6 100644
--- a/clang/test/Sema/attr-weak.c
+++ b/clang/test/Sema/attr-weak.c
@@ -16,8 +16,12 @@ struct __attribute__((weak_import)) s1 {}; // expected-warning {{'weak_import' a
 static int f(void) __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
 static int x __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
 
-int C; // expected-note {{previous declaration is here}}
-extern int C __attribute__((weak_import)); // expected-warning {{an already-declared variable is made a weak_import declaration}}
+int C; // expected-note {{previous definition is here}}
+extern int C __attribute__((weak_import)); // expected-warning {{'C' cannot be declared 'weak_import'}}
+
+int C2; // expected-note {{previous definition is here}}
+extern int C2;
+extern int C2 __attribute__((weak_import)); // expected-warning {{'C2' cannot be declared 'weak_import'}}
 
 static int pr14946_x;
 extern int pr14946_x  __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}

diff  --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index 0f9a2975e5f68..c6272ef5786ef 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -56,3 +56,10 @@ constexpr bool weak_method_is_non_null = &WithWeakMember::weak_method != nullptr
 // virtual member function is present.
 constexpr bool virtual_weak_method_is_non_null = &WithWeakMember::virtual_weak_method != nullptr; // expected-error {{must be initialized by a constant expression}}
 // expected-note at -1 {{comparison against pointer to weak member 'WithWeakMember::virtual_weak_method' can only be performed at runtime}}
+
+// Check that no warnings are emitted.
+extern "C" int g0;
+extern int g0 __attribute__((weak_import));
+
+extern "C" int g1 = 0; // expected-note {{previous definition is here}}
+extern int g1 __attribute__((weak_import)); // expected-warning {{attribute declaration must precede definition}}