[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-07-17 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak closed 
https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-07-17 Thread John McCall via cfe-commits

https://github.com/rjmccall approved this pull request.


https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-07-17 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak updated 
https://github.com/llvm/llvm-project/pull/85886

>From d39667c7e65c10babb478d8f8d54fecb66d90568 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH 1/5] [Sema] Don't drop weak_import from a declaration that
 follows a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9a..2e45f1191273a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8..a2c5fd4abd35f 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

>From 33e3219c721d0c03f445c9bb977cb9f3809e74ac Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Wed, 8 May 2024 20:40:58 -0700
Subject: [PATCH 2/5] Check whether there's a definition at all

---
 clang/lib/Sema/SemaDecl.cpp | 23 +--
 clang/test/Sema/attr-weak.c |  4 
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2e45f1191273a..0be6906026a85 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,12 +4613,23 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
-  !Old->hasAttr()) {
-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();
+  if (New->hasAttr()) {
+// We know there's no full definition as the attribute on New would have
+// been removed otherwise. Just look for the most recent tentative
+// definition.
+VarDecl *TentativeDef = nullptr;
+for (auto *D = Old; D; D = D->getPreviousDecl())
+  if (D->isThisDeclarationADefinition() == VarDecl::TentativeDefinition) {
+TentativeDef = D;
+break;
+  }
+
+if (TentativeDef) {
+  Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
+  Diag(TentativeDef->getLocation(), diag::note_previous_declaration);
+  // Remove weak_import attribute on new declaration.
+  New->dropAttr();
+}
   }
 
   if (const auto *ILA = New->getAttr())
diff --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c
index b827d1539b997..94e1723e125b6 100644
--- a/clang/test/Sema/attr-weak.c
+++ b/clang/test/Sema/attr-weak.c
@@ -19,6 +19,10 @@ static int x __attribute__((weak)); // expected-error {{weak 
declaration cannot
 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 C2; // expected-note {{previous declaration is here}}
+extern int C2;
+extern int C2 __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}}
 

>From c33375cbc8863be911f7832538228cfde0e1d58d Mon Sep 17 00:00:00 

[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-07-17 Thread John McCall via cfe-commits

https://github.com/rjmccall approved this pull request.

LGTM.  The (existing) diagnostic wording does seem a little awkward, so if you 
want to put effort into cleaning that up, it'd be welcome, but I won't hold up 
this fix.

https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-09 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak updated 
https://github.com/llvm/llvm-project/pull/85886

>From d39667c7e65c10babb478d8f8d54fecb66d90568 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH 1/4] [Sema] Don't drop weak_import from a declaration that
 follows a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9..2e45f1191273 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f..a2c5fd4abd35 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

>From 33e3219c721d0c03f445c9bb977cb9f3809e74ac Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Wed, 8 May 2024 20:40:58 -0700
Subject: [PATCH 2/4] Check whether there's a definition at all

---
 clang/lib/Sema/SemaDecl.cpp | 23 +--
 clang/test/Sema/attr-weak.c |  4 
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2e45f1191273..0be6906026a8 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,12 +4613,23 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
-  !Old->hasAttr()) {
-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();
+  if (New->hasAttr()) {
+// We know there's no full definition as the attribute on New would have
+// been removed otherwise. Just look for the most recent tentative
+// definition.
+VarDecl *TentativeDef = nullptr;
+for (auto *D = Old; D; D = D->getPreviousDecl())
+  if (D->isThisDeclarationADefinition() == VarDecl::TentativeDefinition) {
+TentativeDef = D;
+break;
+  }
+
+if (TentativeDef) {
+  Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
+  Diag(TentativeDef->getLocation(), diag::note_previous_declaration);
+  // Remove weak_import attribute on new declaration.
+  New->dropAttr();
+}
   }
 
   if (const auto *ILA = New->getAttr())
diff --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c
index b827d1539b99..94e1723e125b 100644
--- a/clang/test/Sema/attr-weak.c
+++ b/clang/test/Sema/attr-weak.c
@@ -19,6 +19,10 @@ static int x __attribute__((weak)); // expected-error {{weak 
declaration cannot
 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 C2; // expected-note {{previous declaration is here}}
+extern int C2;
+extern int C2 __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}}
 

>From c33375cbc8863be911f7832538228cfde0e1d58d Mon Sep 17 00:00:00 2001
Fro

[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-09 Thread Akira Hatanaka via cfe-commits

ahatanak wrote:

The updated patch checks that the decl isn't `DeclarationOnly` to make it clear 
that we are looking for all definitions although we know we won't find any full 
definitions.

https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-09 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak updated 
https://github.com/llvm/llvm-project/pull/85886

>From d39667c7e65c10babb478d8f8d54fecb66d90568 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH 1/3] [Sema] Don't drop weak_import from a declaration that
 follows a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9a..2e45f1191273a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8..a2c5fd4abd35f 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

>From 33e3219c721d0c03f445c9bb977cb9f3809e74ac Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Wed, 8 May 2024 20:40:58 -0700
Subject: [PATCH 2/3] Check whether there's a definition at all

---
 clang/lib/Sema/SemaDecl.cpp | 23 +--
 clang/test/Sema/attr-weak.c |  4 
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2e45f1191273a..0be6906026a85 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,12 +4613,23 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
-  !Old->hasAttr()) {
-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();
+  if (New->hasAttr()) {
+// We know there's no full definition as the attribute on New would have
+// been removed otherwise. Just look for the most recent tentative
+// definition.
+VarDecl *TentativeDef = nullptr;
+for (auto *D = Old; D; D = D->getPreviousDecl())
+  if (D->isThisDeclarationADefinition() == VarDecl::TentativeDefinition) {
+TentativeDef = D;
+break;
+  }
+
+if (TentativeDef) {
+  Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
+  Diag(TentativeDef->getLocation(), diag::note_previous_declaration);
+  // Remove weak_import attribute on new declaration.
+  New->dropAttr();
+}
   }
 
   if (const auto *ILA = New->getAttr())
diff --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c
index b827d1539b997..94e1723e125b6 100644
--- a/clang/test/Sema/attr-weak.c
+++ b/clang/test/Sema/attr-weak.c
@@ -19,6 +19,10 @@ static int x __attribute__((weak)); // expected-error {{weak 
declaration cannot
 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 C2; // expected-note {{previous declaration is here}}
+extern int C2;
+extern int C2 __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}}
 

>From c33375cbc8863be911f7832538228cfde0e1d58d Mon Sep 17 00:00:00 

[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-08 Thread Akira Hatanaka via cfe-commits

ahatanak wrote:

> With that said, I think you need to check if a definition exists at all and 
> not just whether the last declaration is that definition.

Note that `checkNewAttributesAfterDef`, which is called right before the check, 
removes attributes on `New` if there was a full definition. The updated patch 
just looks for any previously declared tentative definition.

https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-08 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak updated 
https://github.com/llvm/llvm-project/pull/85886

>From d39667c7e65c10babb478d8f8d54fecb66d90568 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH 1/2] [Sema] Don't drop weak_import from a declaration that
 follows a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9a..2e45f1191273a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8..a2c5fd4abd35f 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

>From 33e3219c721d0c03f445c9bb977cb9f3809e74ac Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Wed, 8 May 2024 20:40:58 -0700
Subject: [PATCH 2/2] Check whether there's a definition at all

---
 clang/lib/Sema/SemaDecl.cpp | 23 +--
 clang/test/Sema/attr-weak.c |  4 
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2e45f1191273a..0be6906026a85 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,12 +4613,23 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
-  !Old->hasAttr()) {
-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();
+  if (New->hasAttr()) {
+// We know there's no full definition as the attribute on New would have
+// been removed otherwise. Just look for the most recent tentative
+// definition.
+VarDecl *TentativeDef = nullptr;
+for (auto *D = Old; D; D = D->getPreviousDecl())
+  if (D->isThisDeclarationADefinition() == VarDecl::TentativeDefinition) {
+TentativeDef = D;
+break;
+  }
+
+if (TentativeDef) {
+  Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
+  Diag(TentativeDef->getLocation(), diag::note_previous_declaration);
+  // Remove weak_import attribute on new declaration.
+  New->dropAttr();
+}
   }
 
   if (const auto *ILA = New->getAttr())
diff --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c
index b827d1539b997..94e1723e125b6 100644
--- a/clang/test/Sema/attr-weak.c
+++ b/clang/test/Sema/attr-weak.c
@@ -19,6 +19,10 @@ static int x __attribute__((weak)); // expected-error {{weak 
declaration cannot
 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 C2; // expected-note {{previous declaration is here}}
+extern int C2;
+extern int C2 __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}}
 

___
cfe-commits mailing

[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-08 Thread John McCall via cfe-commits

https://github.com/rjmccall edited 
https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-05-08 Thread John McCall via cfe-commits

https://github.com/rjmccall commented:

I think you're right about the intended logic being to check for a definition, 
especially given the wording of the warning.  IIRC, we didn't have some of 
these high-level checks at the time.

With that said, I think you need to check if a definition exists at all and not 
just whether the last declaration is that definition.

https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-03-21 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak updated 
https://github.com/llvm/llvm-project/pull/85886

>From d39667c7e65c10babb478d8f8d54fecb66d90568 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH] [Sema] Don't drop weak_import from a declaration that follows
 a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9aa..2e45f1191273a4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8a..a2c5fd4abd35f6 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

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


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-03-19 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Akira Hatanaka (ahatanak)


Changes

Only drop it if the declaration follows a definition. I believe this is what 
33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848

---
Full diff: https://github.com/llvm/llvm-project/pull/85886.diff


2 Files Affected:

- (modified) clang/lib/Sema/SemaDecl.cpp (+1-2) 
- (modified) clang/test/SemaCXX/attr-weak.cpp (+7) 


``diff
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9aa..2e45f1191273a4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8a..a2c5fd4abd35f6 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

``




https://github.com/llvm/llvm-project/pull/85886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)

2024-03-19 Thread Akira Hatanaka via cfe-commits

https://github.com/ahatanak created 
https://github.com/llvm/llvm-project/pull/85886

Only drop it if the declaration follows a definition. I believe this is what 
33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848

>From 1ccbc2e2a3bdf127f5baeb22123c67428da4e656 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka 
Date: Tue, 19 Mar 2024 15:50:00 -0700
Subject: [PATCH] [Sema] Don't drop weak_import from a declaration that follows
 a declaration directly contained in a linkage-specification

Only drop it if the declaration follows a definition. I believe this is
what 33e022650adee965c65f9aea086ee74f3fd1bad5 was trying to do.

rdar://61865848
---
 clang/lib/Sema/SemaDecl.cpp  | 3 +--
 clang/test/SemaCXX/attr-weak.cpp | 7 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5850cd0ab6b9aa..2e45f1191273a4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4613,8 +4613,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult 
&Previous) {
   mergeDeclAttributes(New, Old);
   // Warn if an already-declared variable is made a weak_import in a subsequent
   // declaration
-  if (New->hasAttr() &&
-  Old->getStorageClass() == SC_None &&
+  if (New->hasAttr() && Old->isThisDeclarationADefinition() &&
   !Old->hasAttr()) {
 Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
 Diag(Old->getLocation(), diag::note_previous_declaration);
diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp
index f065bfd9483f8a..a2c5fd4abd35f6 100644
--- a/clang/test/SemaCXX/attr-weak.cpp
+++ b/clang/test/SemaCXX/attr-weak.cpp
@@ -55,3 +55,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@-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}}

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