[clang][bytecode] Diagnose destroying already dead locals

[tbaederr]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

---

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

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/Interp.h (+15)
• (modified) clang/test/AST/ByteCode/lifetimes.cpp (+16-2)

diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 0cfeed86f0b51..9b3d312b547de 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2197,6 +2197,21 @@ inline bool SubPtr(InterpState &S, CodePtr OpPC) {
 //===----------------------------------------------------------------------===//
 
 inline bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) {
+  assert(S.Current->getFunction());
+
+  // FIXME: We iterate the scope once here and then again in the destroy() call
+  // below.
+  for (auto &Local : S.Current->getFunction()->getScope(I).locals_reverse()) {
+    const Pointer &Ptr = S.Current->getLocalPointer(Local.Offset);
+
+    if (Ptr.getLifetime() == Lifetime::Ended) {
+      auto *D = cast<NamedDecl>(Ptr.getFieldDesc()->asDecl());
+      S.FFDiag(D->getLocation(), diag::note_constexpr_destroy_out_of_lifetime)
+          << D->getNameAsString();
+      return false;
+    }
+  }
+
   S.Current->destroy(I);
   return true;
 }
diff --git a/clang/test/AST/ByteCode/lifetimes.cpp b/clang/test/AST/ByteCode/lifetimes.cpp
index 43039d0c766e9..17f8d4e24bd85 100644
--- a/clang/test/AST/ByteCode/lifetimes.cpp
+++ b/clang/test/AST/ByteCode/lifetimes.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -verify=ref,both %s
+// RUN: %clang_cc1 -verify=expected,both -std=c++20 %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -verify=ref,both      -std=c++20 %s
 
 /// FIXME: Slight difference in diagnostic output here.
 
@@ -68,3 +68,17 @@ namespace PrimitiveMoveFn {
     const float &x = y;
   }
 }
+
+/// FIXME:
+///  1) This doesn't work for parameters
+///  2) We need to do this for all fields in composite scenarios
+namespace PseudoDtor {
+  typedef int I;
+  constexpr bool foo() { // both-error {{never produces a constant expression}}
+    {
+      int a; // both-note {{destroying object 'a' whose lifetime has already ended}}
+      a.~I();
+    }
+    return true;
+  }
+}