[clang] [Sema] Don't drop weak_import from a declaration that follows a declaration directly contained in a linkage-specification (PR #85886)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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