[clang] [clang][bytecode] Fix diagnosting reads from temporaries (PR #106868)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Fix the DeclID not being set in global temporaries and use the same strategy for deciding if a temporary is readable as the current interpreter.

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


3 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+2-1) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+13-8) 
- (modified) clang/test/AST/ByteCode/references.cpp (+11-2) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index dced9ea3493732..1ddaa5bd41df75 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -3772,7 +3772,6 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD,
 
     auto initGlobal = [&](unsigned GlobalIndex) -> bool {
       assert(Init);
-      DeclScope<Emitter> LocalScope(this, VD);
 
       if (VarT) {
         if (!this->visit(Init))
@@ -3796,6 +3795,8 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD,
       return this->emitPopPtr(Init);
     };
 
+    DeclScope<Emitter> LocalScope(this, VD);
+
     // We've already seen and initialized this global.
     if (std::optional<unsigned> GlobalIndex = P.getGlobal(VD)) {
       if (P.getPtrGlobal(*GlobalIndex).isInitialized())
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 42012767c22332..3d75f2ce7183a6 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -181,16 +181,21 @@ static bool CheckTemporary(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
     if (!Ptr.isStaticTemporary())
       return true;
 
-    if (Ptr.getDeclDesc()->getType().isConstQualified())
+    const auto *MTE = dyn_cast_if_present<MaterializeTemporaryExpr>(
+        Ptr.getDeclDesc()->asExpr());
+    if (!MTE)
       return true;
 
-    if (S.P.getCurrentDecl() == ID)
-      return true;
-
-    const SourceInfo &E = S.Current->getSource(OpPC);
-    S.FFDiag(E, diag::note_constexpr_access_static_temporary, 1) << AK;
-    S.Note(Ptr.getDeclLoc(), diag::note_constexpr_temporary_here);
-    return false;
+    // FIXME(perf): Since we do this check on every Load from a static
+    // temporary, it might make sense to cache the value of the
+    // isUsableInConstantExpressions call.
+    if (!MTE->isUsableInConstantExpressions(S.getASTContext()) &&
+        Ptr.block()->getEvalID() != S.Ctx.getEvalID()) {
+      const SourceInfo &E = S.Current->getSource(OpPC);
+      S.FFDiag(E, diag::note_constexpr_access_static_temporary, 1) << AK;
+      S.Note(Ptr.getDeclLoc(), diag::note_constexpr_temporary_here);
+      return false;
+    }
   }
   return true;
 }
diff --git a/clang/test/AST/ByteCode/references.cpp b/clang/test/AST/ByteCode/references.cpp
index 9a790dc75d7308..7c1dccb1f9e341 100644
--- a/clang/test/AST/ByteCode/references.cpp
+++ b/clang/test/AST/ByteCode/references.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
-// RUN: %clang_cc1 -verify=ref %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
+// RUN: %clang_cc1 -verify=ref,both %s
 
 
 constexpr int a = 10;
@@ -135,3 +135,12 @@ static_assert(nonextended_string_ref[2] == '\0', "");
 /// but taking its address is.
 int &&A = 12;
 int arr[!&A];
+
+namespace Temporaries {
+  struct A { int n; };
+  struct B { const A &a; };
+  const B j = {{1}}; // both-note {{temporary created here}}
+
+  static_assert(j.a.n == 1, "");  // both-error {{not an integral constant expression}} \
+                                  // both-note {{read of temporary is not allowed in a constant expression outside the expression that created the temporary}}
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/106868