llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-temporal-safety
Author: NeKon69
<details>
<summary>Changes</summary>
The core issue with templates is that Clang can create implicit template
specialization for the primary template. To address this,
`IsImplicitTemplateSpecialization` skips those declarations by checking whether
the declaration has the same source location as the primary template pattern.
Note that this will still warn on the primary template when there is no
separate explicit specialization declaration:
```cpp
template <typename T>
T &foo(T &obj); // suggests here!
template <>
int &foo<int>(int &x [[clang::lifetimebound]]) { return x; }
```
I kept this behavior to avoid complicating the diagnostic wording. (Probably
can come as a future patch?)
Fixes #<!-- -->198632
---
Full diff: https://github.com/llvm/llvm-project/pull/203866.diff
3 Files Affected:
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+50-4)
- (modified)
clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-cross-tu.cpp (+28)
- (modified)
clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-intra-tu.cpp (+98-6)
``````````diff
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index d41d6f43f837b..2ab271c65b85d 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -82,6 +82,39 @@ class LifetimeChecker {
llvm_unreachable("unhandled causing fact in PointerUnion");
}
+ /// For an explicit specialization, returns the source-level specialization
+ /// declaration to target for attribute placement, if one exists. Skips
+ /// implicit specialization redeclarations that are backed by the template
+ /// pattern. In other cases, returns nullptr.
+ static const FunctionDecl *
+ getExplicitSpecializationDeclForAttr(const FunctionDecl *FDef) {
+ if (FDef->getTemplateSpecializationKindForInstantiation() !=
+ TSK_ExplicitSpecialization)
+ return nullptr;
+
+ auto IsImplicitTemplateSpecialization = [](const FunctionDecl *Redecl,
+ const FunctionDecl *Pattern) {
+ return Pattern && Redecl->getBeginLoc() == Pattern->getBeginLoc();
+ };
+
+ auto redecls = llvm::to_vector(FDef->redecls());
+ for (const FunctionDecl *Redecl : llvm::reverse(redecls)) {
+ if (Redecl == FDef)
+ continue;
+ if (auto *MSI = Redecl->getMemberSpecializationInfo();
+ MSI && MSI->isExplicitSpecialization())
+ if (!IsImplicitTemplateSpecialization(
+ Redecl, dyn_cast<FunctionDecl>(MSI->getInstantiatedFrom())))
+ return Redecl;
+ if (auto *FTSI = Redecl->getTemplateSpecializationInfo();
+ FTSI && FTSI->isExplicitSpecialization())
+ if (!IsImplicitTemplateSpecialization(
+ Redecl, FTSI->getTemplate()->getTemplatedDecl()))
+ return Redecl;
+ }
+ return nullptr;
+ }
+
public:
LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation,
const MovedLoansAnalysis &MovedLoans,
@@ -342,11 +375,17 @@ class LifetimeChecker {
};
const FileID DefFile = GetFile(FDef);
- const FunctionDecl *CanonicalDecl = FDef->getCanonicalDecl();
+ const FunctionDecl *TargetDecl = FDef->getCanonicalDecl();
+
+ // For explicit specializations, the canonical decl is the primary
template.
+ // We should target the explicit specialization declaration instead.
+ if (const FunctionDecl *SpecDecl =
+ getExplicitSpecializationDeclForAttr(FDef))
+ TargetDecl = SpecDecl;
+
llvm::SmallVector<std::pair<const FunctionDecl *, WarningScope>, 2>
Targets{
- {CanonicalDecl, GetFile(CanonicalDecl) == DefFile
- ? WarningScope::IntraTU
- : WarningScope::CrossTU}};
+ {TargetDecl, GetFile(TargetDecl) == DefFile ? WarningScope::IntraTU
+ : WarningScope::CrossTU}};
// Find the earliest redeclaration in each file other than the definition
// file.
@@ -354,6 +393,13 @@ class LifetimeChecker {
FileID File = GetFile(FD);
if (File == DefFile)
return;
+ // For explicit specializations, skip redeclarations that do not belong
to
+ // the same explicit-specialization instantiation path.
+ if (FDef->getTemplateSpecializationKindForInstantiation() ==
+ TSK_ExplicitSpecialization &&
+ FD->getTemplateSpecializationKindForInstantiation() !=
+ TSK_ExplicitSpecialization)
+ return;
for (auto [SeenFD, _] : Targets)
if (GetFile(SeenFD) == File)
return;
diff --git
a/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-cross-tu.cpp
b/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-cross-tu.cpp
index cade7c7a81105..c819173a021cd 100644
--- a/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-cross-tu.cpp
+++ b/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-cross-tu.cpp
@@ -23,6 +23,12 @@ struct HeaderS {
// CHECK:
fix-it:"{{.*}}cross.h":{[[#THIS_WARN_LINE]]:{{[0-9]+}}-[[#THIS_WARN_LINE]]:{{[0-9]+}}}:"
{{\[\[}}clang::lifetimebound{{\]\]}}"
};
+template <typename T>
+HeaderObj &spec_cross_tu(HeaderObj &obj);
+
+template <>
+HeaderObj &spec_cross_tu<HeaderObj>(HeaderObj &obj); // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers in
other translation units; add it to the declaration instead}}
+
//--- cross_1.h
struct HeaderObj;
HeaderObj &multi_header_param(HeaderObj & // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers in
other translation units; add it to the declaration instead}}
@@ -30,6 +36,15 @@ HeaderObj &multi_header_param(HeaderObj & //
expected-warning {{'lifetimebound'
obj // CHECK:
fix-it:"{{.*}}cross_1.h":{[[#PARAM_WARN_LINE+2]]:{{[0-9]+}}-[[#PARAM_WARN_LINE+2]]:{{[0-9]+}}}:"
{{\[\[}}clang::lifetimebound{{\]\]}}"
);
+template <typename T>
+struct HeaderMemberSpec {
+ T data;
+ T &get();
+};
+
+template <>
+HeaderObj &HeaderMemberSpec<HeaderObj>::get(); // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers in
other translation units; add it to the declaration instead}}
+
//--- cross_2.h
struct HeaderObj;
HeaderObj &multi_header_param(HeaderObj & // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers in
other translation units; add it to the declaration instead}}
@@ -37,6 +52,9 @@ HeaderObj &multi_header_param(HeaderObj & //
expected-warning {{'lifetimebound'
obj // CHECK:
fix-it:"{{.*}}cross_2.h":{[[#PARAM_WARN_LINE+2]]:{{[0-9]+}}-[[#PARAM_WARN_LINE+2]]:{{[0-9]+}}}:"
{{\[\[}}clang::lifetimebound{{\]\]}}"
);
+template <>
+HeaderObj &HeaderMemberSpec<HeaderObj>::get(); // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers in
other translation units; add it to the declaration instead}}
+
//--- cross.cpp
#include "cross.h"
#include "cross_1.h"
@@ -53,3 +71,13 @@ HeaderObj &HeaderS::header_this() [[clang::lifetimebound]] {
// expected-note {{
HeaderObj &multi_header_param(HeaderObj &obj [[clang::lifetimebound]]) { //
expected-note 2 {{'lifetimebound' attribute appears here on the definition}}
return obj;
}
+
+template <>
+HeaderObj &spec_cross_tu<HeaderObj>(HeaderObj &obj [[clang::lifetimebound]]) {
// expected-note {{'lifetimebound' attribute appears here on the definition}}
+ return obj;
+}
+
+template <>
+HeaderObj &HeaderMemberSpec<HeaderObj>::get() [[clang::lifetimebound]] { //
expected-note 2 {{'lifetimebound' attribute appears here on the definition}}
+ return data;
+}
diff --git
a/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-intra-tu.cpp
b/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-intra-tu.cpp
index 7fa4cae100509..c2157ef0ca2ad 100644
--- a/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-intra-tu.cpp
+++ b/clang/test/Sema/LifetimeSafety/misplaced-lifetimebound-intra-tu.cpp
@@ -133,18 +133,110 @@ View friend_redecl(MyObj &obj) {
}
template <typename T>
-// FIXME: Current analysis suggests adding to the primary template
declaration, which is not ideal, as it will affect all specializations.
-MyObj &spec_func(T & // expected-warning {{'lifetimebound' attribute on this
definition is not visible to callers before the definition; add it to the
declaration instead}}
- obj // CHECK:
fix-it:"{{.*}}":{[[@LINE]]:{{[0-9]+}}-[[@LINE]]:{{[0-9]+}}}:"
{{\[\[clang::lifetimebound\]\]}}"
- );
+MyObj &spec_func(T &obj);
template <>
-// FIXME: Attribute is inhetired, diagnostic's wording is not correct.
-MyObj &spec_func<MyObj>(MyObj &obj [[clang::lifetimebound]]); // expected-note
{{'lifetimebound' attribute appears here on the definition}}
+MyObj &spec_func<MyObj>(MyObj &obj [[clang::lifetimebound]]);
template <>
MyObj &spec_func<MyObj>(MyObj &obj) { return obj; }
+template <typename T>
+MyObj &spec_misplaced(T &obj);
+
+template <>
+MyObj &spec_misplaced<MyObj>(MyObj &obj); // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers before
the definition; add it to the declaration instead}}
+
+template <>
+MyObj &spec_misplaced<MyObj>(MyObj &obj [[clang::lifetimebound]]) { //
expected-note {{'lifetimebound' attribute appears here on the definition}}
+ return obj;
+}
+
+template <class T>
+struct MemberSpec {
+ T data;
+ T &get();
+};
+
+template <>
+MyObj &MemberSpec<MyObj>::get(); // expected-warning {{'lifetimebound'
attribute on this definition is not visible to callers before the definition;
add it to the declaration instead}}
+
+template <>
+MyObj &MemberSpec<MyObj>::get() [[clang::lifetimebound]] { // expected-note
{{'lifetimebound' attribute appears here on the definition}}
+ return data;
+}
+
+template <class T>
+struct MemberSpecOk {
+ T data;
+ T &get();
+};
+
+template <>
+MyObj &MemberSpecOk<MyObj>::get() [[clang::lifetimebound]];
+
+template <>
+MyObj &MemberSpecOk<MyObj>::get() {
+ return data;
+}
+
+template <class T>
+struct NestedMemberSpecBoth {
+ template <class U>
+ struct Inner {
+ U data;
+ template <class V>
+ U &both(U &arg, V);
+ };
+};
+
+template <>
+template <>
+template <>
+MyObj &NestedMemberSpecBoth<int>::Inner<MyObj>::both(
+ MyObj &arg, bool); // expected-warning {{'lifetimebound' attribute on this
definition is not visible to callers before the definition; add it to the
declaration instead}} \
+ // expected-warning {{'lifetimebound' attribute on this
definition is not visible to callers before the definition; add it to the
declaration instead}}
+
+template <>
+template <>
+template <>
+MyObj &NestedMemberSpecBoth<int>::Inner<MyObj>::both(
+ MyObj &arg [[clang::lifetimebound]], bool use_arg) // expected-note
{{'lifetimebound' attribute appears here on the definition}}
+ [[clang::lifetimebound]] { // expected-note
{{'lifetimebound' attribute appears here on the definition}}
+ return use_arg ? arg : data;
+}
+
+template <typename T>
+T &multi_spec(T &obj);
+
+template <>
+int &multi_spec<int>(int &obj);
+
+template <>
+MyObj &multi_spec<MyObj>(MyObj &obj); // expected-warning {{'lifetimebound'
attribute on this definition is not visible to callers before the definition;
add it to the declaration instead}}
+
+template <>
+MyObj &multi_spec<MyObj>(MyObj &obj [[clang::lifetimebound]]) { //
expected-note {{'lifetimebound' attribute appears here on the definition}}
+ return obj;
+}
+
+template <>
+int &multi_spec<int>(int &obj) { return obj; }
+
+template <typename T>
+T &multi_redecl_spec(T &obj);
+
+template <>
+MyObj &multi_redecl_spec<MyObj>(MyObj &obj); // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers before
the definition; add it to the declaration instead}}
+
+template <>
+MyObj &multi_redecl_spec<MyObj>(MyObj &obj);
+
+template <>
+MyObj &multi_redecl_spec<MyObj>(MyObj &obj [[clang::lifetimebound]]) { //
expected-note {{'lifetimebound' attribute appears here on the definition}}
+ return obj;
+}
+
MyObj get_default_obj();
const MyObj &default_arg_param(const MyObj& // expected-warning
{{'lifetimebound' attribute on this definition is not visible to callers before
the definition; add it to the declaration instead}}
obj // CHECK:
fix-it:"{{.*}}":{[[@LINE]]:{{[0-9]+}}-[[@LINE]]:{{[0-9]+}}}:"
{{\[\[clang::lifetimebound\]\]}}"
``````````
</details>
https://github.com/llvm/llvm-project/pull/203866
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
