[clang] 311f725 - [clang][Interp] Create only globals when initializing a global variable

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-10-19T14:50:17+02:00
New Revision: 311f725d9a6fa29b5672a2dd26a078c6c6dcd01a

URL: https://github.com/llvm/llvm-project/commit/311f725d9a6fa29b5672a2dd26a078c6c6dcd01a
DIFF: https://github.com/llvm/llvm-project/commit/311f725d9a6fa29b5672a2dd26a078c6c6dcd01a.diff

LOG: [clang][Interp] Create only globals when initializing a global variable

For this code:

struct O {
  int &&j;
};

O o1(0);

The generated AST for the initializer of o1 is:

VarDecl 0x62100006ab08 <array.cpp:119:3, col:9> col:5 o1 'O':'O' parenlistinit
`-ExprWithCleanups 0x62100006b250 <col:7, col:9> 'O':'O'
  `-CXXParenListInitExpr 0x62100006b210 <col:7, col:9> 'O':'O'
    `-MaterializeTemporaryExpr 0x62100006b1f0 <col:8> 'int' xvalue
      `-IntegerLiteral 0x62100006abd0 <col:8> 'int' 0

Before this patch, we create a local temporary variable for the
MaterializeTemporaryExpr and destroy it again when destroying the
EvalEmitter we create to interpret the initializer. However, since
O::j is a reference, this reference now points to a local variable
that doesn't exist anymore.

Differential Revision: https://reviews.llvm.org/D156453

Added: 
    

Modified: 
    clang/lib/AST/Interp/ByteCodeExprGen.cpp
    clang/lib/AST/Interp/ByteCodeExprGen.h
    clang/test/AST/Interp/records.cpp
    clang/test/SemaCXX/paren-list-agg-init.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index d9389e7b0033191..ed971fe0f650f22 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -29,14 +29,20 @@ namespace interp {
 template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
 public:
   DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
-      : VariableScope<Emitter>(Ctx), Scope(Ctx->P, VD) {}
+      : VariableScope<Emitter>(Ctx), Scope(Ctx->P, VD),
+        OldGlobalDecl(Ctx->GlobalDecl) {
+    Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
+  }
 
   void addExtended(const Scope::Local &Local) override {
     return this->addLocal(Local);
   }
 
+  ~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
+
 private:
   Program::DeclScope Scope;
+  bool OldGlobalDecl;
 };
 
 /// Scope used to handle initialization methods.
@@ -1198,21 +1204,30 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
   if (DiscardResult)
     return this->discard(SubExpr);
 
+  // When we're initializing a global variable *or* the storage duration of
+  // the temporary is explicitly static, create a global variable.
   std::optional<PrimType> SubExprT = classify(SubExpr);
-  if (E->getStorageDuration() == SD_Static) {
+  bool IsStatic = E->getStorageDuration() == SD_Static;
+  if (GlobalDecl || IsStatic) {
     std::optional<unsigned> GlobalIndex = P.createGlobal(E);
     if (!GlobalIndex)
       return false;
 
     const LifetimeExtendedTemporaryDecl *TempDecl =
         E->getLifetimeExtendedTemporaryDecl();
-    assert(TempDecl);
+    if (IsStatic)
+      assert(TempDecl);
 
     if (SubExprT) {
       if (!this->visit(SubExpr))
         return false;
-      if (!this->emitInitGlobalTemp(*SubExprT, *GlobalIndex, TempDecl, E))
-        return false;
+      if (IsStatic) {
+        if (!this->emitInitGlobalTemp(*SubExprT, *GlobalIndex, TempDecl, E))
+          return false;
+      } else {
+        if (!this->emitInitGlobal(*SubExprT, *GlobalIndex, E))
+          return false;
+      }
       return this->emitGetPtrGlobal(*GlobalIndex, E);
     }
 
@@ -1221,7 +1236,9 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
       return false;
     if (!this->visitInitializer(SubExpr))
       return false;
-    return this->emitInitGlobalTempComp(TempDecl, E);
+    if (IsStatic)
+      return this->emitInitGlobalTempComp(TempDecl, E);
+    return true;
   }
 
   // For everyhing else, use local variables.

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 2049dab140eaaae..83986d3dd579ed6 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -304,6 +304,9 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
   /// Flag inidicating if we're initializing an already created
   /// variable. This is set in visitInitializer().
   bool Initializing = false;
+
+  /// Flag indicating if we're initializing a global variable.
+  bool GlobalDecl = false;
 };
 
 extern template class ByteCodeExprGen<ByteCodeEmitter>;

diff  --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 3c866825d1f077c..a2e878f6132d0ac 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1064,6 +1064,22 @@ namespace ParenInit {
   };
 
   constexpr B b(A(1),2);
+
+
+  struct O {
+    int &&j;
+  };
+
+  /// Not constexpr!
+  O o1(0);
+  constinit O o2(0); // ref-error {{variable does not have a constant initializer}} \
+                     // ref-note {{required by 'constinit' specifier}} \
+                     // ref-note {{reference to temporary is not a constant expression}} \
+                     // ref-note {{temporary created here}} \
+                     // expected-error {{variable does not have a constant initializer}} \
+                     // expected-note {{required by 'constinit' specifier}} \
+                     // expected-note {{reference to temporary is not a constant expression}} \
+                     // expected-note {{temporary created here}}
 }
 #endif
 

diff  --git a/clang/test/SemaCXX/paren-list-agg-init.cpp b/clang/test/SemaCXX/paren-list-agg-init.cpp
index 944ea76b81d24d3..ef6e67f263caeda 100644
--- a/clang/test/SemaCXX/paren-list-agg-init.cpp
+++ b/clang/test/SemaCXX/paren-list-agg-init.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only
+// RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter
 // RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only
 
 struct A { // expected-note 4{{candidate constructor}}