[clang] [clang] Improve the lifetime_capture_by diagnostic on the constructor. (PR #117792)
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 01:34:05 PST 2024
https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/117792
>From edd8e7354c4ff96446d32830f4cd5e6c3c333a84 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 26 Nov 2024 21:42:45 +0100
Subject: [PATCH 1/3] [clang] Improve the lifetime_capture_by diagnostic on the
constructor.
---
clang/lib/Sema/CheckExprLifetime.cpp | 11 +++++++++++
clang/lib/Sema/SemaChecking.cpp | 6 ++++++
.../warn-lifetime-analysis-capture-by.cpp | 19 +++++++++++++++++++
3 files changed, 36 insertions(+)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 6cdd4dc629e50a..c4fa73127410b5 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -535,6 +535,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
bool EnableGSLAnalysis = !Callee->getASTContext().getDiagnostics().isIgnored(
diag::warn_dangling_lifetime_pointer, SourceLocation());
+ bool EnableDanglingCapture =
+ !Callee->getASTContext().getDiagnostics().isIgnored(
+ diag::warn_dangling_reference_captured, SourceLocation());
Expr *ObjectArg = nullptr;
if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) {
ObjectArg = Args[0];
@@ -623,6 +626,14 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
}
if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
+ else if (const auto *CaptureAttr =
+ Callee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
+ EnableDanglingCapture && CaptureAttr &&
+ isa<CXXConstructorDecl>(Callee) &&
+ llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
+ return ArgIdx == LifetimeCaptureByAttr::THIS;
+ }))
+ VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
else if (EnableGSLAnalysis && I == 0) {
// Perform GSL analysis for the first argument
if (shouldTrackFirstArgument(Callee)) {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a49605e4867651..1605523097a6b1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3240,8 +3240,14 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
unsigned ArgIdx) {
if (!Attr)
return;
+
Expr *Captured = const_cast<Expr *>(GetArgAt(ArgIdx));
for (int CapturingParamIdx : Attr->params()) {
+ // lifetime_capture_by(this) case is handled in the lifetimebound expr
+ // initialization codepath.
+ if (CapturingParamIdx == LifetimeCaptureByAttr::THIS &&
+ isa<CXXConstructorDecl>(FD))
+ continue;
Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
CapturingEntity CE{Capturing};
// Ensure that 'Captured' outlives the 'Capturing' entity.
diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
index 4d562bac1e305b..77523210e50203 100644
--- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
@@ -411,3 +411,22 @@ void use() {
}
} // namespace with_span
} // namespace inferred_capture_by
+
+namespace on_constructor {
+struct T {
+ T(const int& t [[clang::lifetime_capture_by(this)]]);
+};
+struct T2 {
+ T2(const int& t [[clang::lifetime_capture_by(x)]], int& x);
+};
+int foo(const T& t);
+
+void test() {
+ auto x = foo(T(1)); // OK. no diagnosic
+ T(1); // OK. no diagnostic
+ T t(1); // expected-warning {{temporary whose address is used}}
+
+ int a;
+ T2(1, a); // expected-warning {{object whose reference is captured by}}
+}
+} // namespace on_constructor
>From 48487a5b1f9b87c295ce1d6df7d0fc60a7db6695 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Thu, 28 Nov 2024 10:12:40 +0100
Subject: [PATCH 2/3] Address comments
---
clang/lib/Sema/CheckExprLifetime.cpp | 19 ++++++++++++++-----
.../warn-lifetime-analysis-capture-by.cpp | 7 +++++++
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index c4fa73127410b5..607b7daf878e17 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -535,9 +535,6 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
bool EnableGSLAnalysis = !Callee->getASTContext().getDiagnostics().isIgnored(
diag::warn_dangling_lifetime_pointer, SourceLocation());
- bool EnableDanglingCapture =
- !Callee->getASTContext().getDiagnostics().isIgnored(
- diag::warn_dangling_reference_captured, SourceLocation());
Expr *ObjectArg = nullptr;
if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) {
ObjectArg = Args[0];
@@ -628,11 +625,23 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
else if (const auto *CaptureAttr =
Callee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
- EnableDanglingCapture && CaptureAttr &&
- isa<CXXConstructorDecl>(Callee) &&
+ CaptureAttr && isa<CXXConstructorDecl>(Callee) &&
llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
return ArgIdx == LifetimeCaptureByAttr::THIS;
}))
+ // `lifetime_capture_by(this)` in a class constructor has the same
+ // semantics as `lifetimebound`:
+ //
+ // struct Foo {
+ // const int& a;
+ // // Equivalent to Foo(const int& t [[clang::lifetimebound]])
+ // Foo(const int& t [[clang::lifetime_capture_by(this)]]) : a(t) {}
+ // };
+ //
+ // In the implementation, `lifetime_capture_by` is treated as an alias for
+ // `lifetimebound` and shares the same code path. This implies the emitted
+ // diagnostics will be emitted under `-Wdangling`, not
+ // `-Wdangling-capture`.
VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
else if (EnableGSLAnalysis && I == 0) {
// Perform GSL analysis for the first argument
diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
index 77523210e50203..3d3e33596a1264 100644
--- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp
@@ -419,12 +419,19 @@ struct T {
struct T2 {
T2(const int& t [[clang::lifetime_capture_by(x)]], int& x);
};
+struct T3 {
+ T3(const T& t [[clang::lifetime_capture_by(this)]]);
+};
+
int foo(const T& t);
+int bar(const T& t[[clang::lifetimebound]]);
void test() {
auto x = foo(T(1)); // OK. no diagnosic
T(1); // OK. no diagnostic
T t(1); // expected-warning {{temporary whose address is used}}
+ auto y = bar(T(1)); // expected-warning {{temporary whose address is used}}
+ T3 t3(T(1)); // expected-warning {{temporary whose address is used}}
int a;
T2(1, a); // expected-warning {{object whose reference is captured by}}
>From d02a905d2ee239c2a143ded4804fc02c26ef48e5 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Thu, 28 Nov 2024 10:32:55 +0100
Subject: [PATCH 3/3] Update the attribute doc.
---
clang/include/clang/Basic/AttrDocs.td | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 6fb2eb3eb3e663..6a31c9a3f31c43 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3976,6 +3976,8 @@ The capturing entity ``X`` can be one of the following:
std::set<std::string_view> s;
};
+ Note: When applied to a constructor parameter, `[[clang::lifetime_capture_by(this)]]` is just an alias of `[[clang::lifetimebound]]`.
+
- `global`, `unknown`.
.. code-block:: c++
More information about the cfe-commits
mailing list