[clang][Interp] Remove GlobalDecl flag

[tbaederr]

When initializing global variables, we still need to create local variables for temporaries created in the process. Otherwise, we will never call their destructors.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

When initializing global variables, we still need to create local variables for temporaries created in the process. Otherwise, we will never call their destructors.

---

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

5 Files Affected:

• (modified) clang/lib/AST/Interp/Compiler.cpp (+1-5)
• (modified) clang/lib/AST/Interp/Compiler.h (-3)
• (modified) clang/lib/AST/Interp/Context.cpp (+3)
• (modified) clang/lib/AST/Interp/Descriptor.cpp (+14-5)
• (modified) clang/test/AST/Interp/codegen.cpp (+10)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index 1295555f4cf22..e5280491dfd73 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -29,9 +29,7 @@ template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
 public:
   DeclScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
       : LocalScope<Emitter>(Ctx, VD), Scope(Ctx->P, VD),
-        OldGlobalDecl(Ctx->GlobalDecl),
         OldInitializingDecl(Ctx->InitializingDecl) {
-    Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
     Ctx->InitializingDecl = VD;
     Ctx->InitStack.push_back(InitLink::Decl(VD));
   }
@@ -41,14 +39,12 @@ template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
   }
 
   ~DeclScope() {
-    this->Ctx->GlobalDecl = OldGlobalDecl;
     this->Ctx->InitializingDecl = OldInitializingDecl;
     this->Ctx->InitStack.pop_back();
   }
 
 private:
   Program::DeclScope Scope;
-  bool OldGlobalDecl;
   const ValueDecl *OldInitializingDecl;
 };
 
@@ -2265,7 +2261,7 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
   // the temporary is explicitly static, create a global variable.
   std::optional<PrimType> SubExprT = classify(SubExpr);
   bool IsStatic = E->getStorageDuration() == SD_Static;
-  if (GlobalDecl || IsStatic) {
+  if (IsStatic) {
     std::optional<unsigned> GlobalIndex = P.createGlobal(E);
     if (!GlobalIndex)
       return false;
diff --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h
index eabde051b4278..244a600d061f4 100644
--- a/clang/lib/AST/Interp/Compiler.h
+++ b/clang/lib/AST/Interp/Compiler.h
@@ -389,9 +389,6 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
   llvm::SmallVector<InitLink> InitStack;
   bool InitStackActive = false;
 
-  /// Flag indicating if we're initializing a global variable.
-  bool GlobalDecl = false;
-
   /// Type of the expression returned by the function.
   std::optional<PrimType> ReturnType;
 
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index b1e06cd4d8a4c..92ac28137fdb4 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -56,6 +56,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 
   if (!Recursing) {
     assert(Stk.empty());
+    C.cleanup();
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
@@ -82,6 +83,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
 
   if (!Recursing) {
     assert(Stk.empty());
+    C.cleanup();
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
@@ -111,6 +113,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
 
   if (!Recursing) {
     assert(Stk.empty());
+    C.cleanup();
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp
index 8becdca6f5c24..23dd08ca48627 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -225,12 +225,21 @@ static void dtorRecord(Block *B, std::byte *Ptr, const Descriptor *D) {
 
 static void moveRecord(Block *B, const std::byte *Src, std::byte *Dst,
                        const Descriptor *D) {
-  for (const auto &F : D->ElemRecord->fields()) {
-    auto FieldOff = F.Offset;
-    auto *FieldDesc = F.Desc;
+  assert(D);
+  assert(D->ElemRecord);
 
-    if (auto Fn = FieldDesc->MoveFn)
-      Fn(B, Src + FieldOff, Dst + FieldOff, FieldDesc);
+  // FIXME: There might be cases where we need to move over the (v)bases as
+  // well.
+  for (const auto &F : D->ElemRecord->fields()) {
+    auto FieldOffset = F.Offset;
+    const auto *SrcDesc =
+        reinterpret_cast<const InlineDescriptor *>(Src + FieldOffset) - 1;
+    auto *DestDesc =
+        reinterpret_cast<InlineDescriptor *>(Dst + FieldOffset) - 1;
+    std::memcpy(DestDesc, SrcDesc, sizeof(InlineDescriptor));
+
+    if (auto Fn = F.Desc->MoveFn)
+      Fn(B, Src + FieldOffset, Dst + FieldOffset, F.Desc);
   }
 }
 
diff --git a/clang/test/AST/Interp/codegen.cpp b/clang/test/AST/Interp/codegen.cpp
index 42d98a079e120..920a5ad7ba3e8 100644
--- a/clang/test/AST/Interp/codegen.cpp
+++ b/clang/test/AST/Interp/codegen.cpp
@@ -54,3 +54,13 @@ namespace reinterpretcast {
   // CHECK: define {{.*}} ptr @_ZN15reinterpretcast2f1Em
   // CHECK: inttoptr
 }
+
+namespace Bitfield {
+  struct S { int a : 5; ~S(); };
+  // CHECK: alloca
+  // CHECK: call {{.*}}memset
+  // CHECK: store i32 {{.*}}, ptr @_ZGRN8Bitfield1rE_
+  // CHECK: call void @_ZN8Bitfield1SD1
+  // CHECK: store ptr @_ZGRN8Bitfield1rE_, ptr @_ZN8Bitfield1rE, align 8
+  int &&r = S().a;
+}