Skip to content

Conversation

@usx95
Copy link
Contributor

@usx95 usx95 commented Jul 23, 2024

Fixes: #81589

There is no way to switch this off without -Wno-dangling.

@usx95 usx95 requested a review from hokein July 23, 2024 20:29
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

Fixes: #81589


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

2 Files Affected:

  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+8)
  • (modified) clang/test/SemaCXX/attr-lifetimebound.cpp (+21)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 5c8ef564f30aa..4d26a2e0f50c6 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "CheckExprLifetime.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "clang/Sema/Initialization.h"
@@ -548,6 +549,13 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
                                        EnableLifetimeWarnings);
   }
 
+  if (auto* M = dyn_cast<MemberExpr>(Init)) {
+    // Lifetime of a field is the lifetime of the base object.
+    if (isa<FieldDecl>(M->getMemberDecl()))
+      visitLocalsRetainedByInitializer(Path, M->getBase(), Visit, true,
+                                       EnableLifetimeWarnings);
+  }
+
   if (isa<CallExpr>(Init)) {
     if (EnableLifetimeWarnings)
       handleGslAnnotatedTypes(Path, Init, Visit);
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 70bc545c07bd9..b43c43bb21699 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -47,6 +47,26 @@ namespace usage_ok {
     q = A(); // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
     r = A(1); // expected-warning {{object backing the pointer r will be destroyed at the end of the full-expression}}
   }
+
+  struct FieldCheck {
+    struct Set {
+      int a;
+    };
+    struct Pair {
+      const int& a;
+      int b;
+      Set c;
+    };
+    Pair p;  
+    FieldCheck(const int a): p(a){}
+    Pair& getPairR() [[clang::lifetimebound]] { return p; }
+    Pair* getPairP() [[clang::lifetimebound]] { return &p; }
+  };
+  void test_field_access() {
+    const int& a = FieldCheck{0}.getPairR().a; // expected-warning {{temporary bound to local reference 'a' will be destroyed at the end of the full-expression}}
+    const int& b = FieldCheck{0}.getPairP()->b; // expected-warning {{temporary bound to local reference 'b' will be destroyed at the end of the full-expression}}
+    const int& c = FieldCheck{0}.getPairP()->c.a; // expected-warning {{temporary bound to local reference 'c' will be destroyed at the end of the full-expression}}
+  }
 }
 
 # 1 "<std>" 1 3
@@ -239,3 +259,4 @@ namespace move_forward_et_al_examples {
   S X;
   S *AddressOfOk = std::addressof(X);
 } // namespace move_forward_et_al_examples
+

@github-actions
Copy link

github-actions bot commented Jul 23, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@usx95 usx95 requested a review from Xazax-hun July 23, 2024 20:33
Copy link
Collaborator

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@hokein hokein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@usx95 usx95 merged commit 1feef92 into llvm:main Jul 24, 2024
@AZero13
Copy link
Contributor

AZero13 commented Jul 24, 2024

Do we want to backport this fix to 19.x?

@usx95
Copy link
Contributor Author

usx95 commented Jul 24, 2024

Makes sense to me. Added cherrypick in #81589

@Xazax-hun
Copy link
Collaborator

Looks like cherry-pick failed and this needs manual back porting.

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Fixes: #81589

There is no way to switch this off without  `-Wno-dangling`.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250725
usx95 added a commit to usx95/llvm-project that referenced this pull request Jul 26, 2024
Fixes: llvm#81589

There is no way to switch this off without  `-Wno-dangling`.
tru pushed a commit to usx95/llvm-project that referenced this pull request Jul 29, 2024
Fixes: llvm#81589

There is no way to switch this off without  `-Wno-dangling`.
pradt2 pushed a commit to pradt2/llvm-project that referenced this pull request Feb 11, 2025
Fixes: llvm#81589

There is no way to switch this off without  `-Wno-dangling`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[[lifetimebound]] basic members tracking

5 participants