Author: Timm Bäder
Date: 2024-07-09T10:43:35+02:00
New Revision: dd2bf3b840df260d794e37cc96d4498372aa08f6

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

LOG: [clang][Interp] Redo variable (re)visiting

Depending on the circumstances we visit variables in, we need to
be careful about when to destroy their temporaries and whether to
emit a Ret op at all or not.

Added: 
    

Modified: 
    clang/lib/AST/Interp/ByteCodeEmitter.h
    clang/lib/AST/Interp/Compiler.cpp
    clang/lib/AST/Interp/Compiler.h
    clang/lib/AST/Interp/EvalEmitter.cpp
    clang/lib/AST/Interp/EvalEmitter.h
    clang/test/C/C23/n3017.c
    clang/test/CodeGenCXX/no-const-init-cxx2a.cpp
    clang/test/SemaCXX/warn-unused-variables.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/ByteCodeEmitter.h 
b/clang/lib/AST/Interp/ByteCodeEmitter.h
index 9a329e969f339..a19a25c2f9e8e 100644
--- a/clang/lib/AST/Interp/ByteCodeEmitter.h
+++ b/clang/lib/AST/Interp/ByteCodeEmitter.h
@@ -46,7 +46,7 @@ class ByteCodeEmitter {
   /// Methods implemented by the compiler.
   virtual bool visitFunc(const FunctionDecl *E) = 0;
   virtual bool visitExpr(const Expr *E) = 0;
-  virtual bool visitDecl(const VarDecl *E, bool ConstantContext) = 0;
+  virtual bool visitDeclAndReturn(const VarDecl *E, bool ConstantContext) = 0;
 
   /// Emits jumps.
   bool jumpTrue(const LabelTy &Label);

diff  --git a/clang/lib/AST/Interp/Compiler.cpp 
b/clang/lib/AST/Interp/Compiler.cpp
index 613bf4af137b6..1d39d7bb9e15f 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -25,10 +25,10 @@ namespace clang {
 namespace interp {
 
 /// Scope used to handle temporaries in toplevel variable declarations.
-template <class Emitter> class DeclScope final : public VariableScope<Emitter> 
{
+template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
 public:
   DeclScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
-      : VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
+      : LocalScope<Emitter>(Ctx, VD), Scope(Ctx->P, VD),
         OldGlobalDecl(Ctx->GlobalDecl),
         OldInitializingDecl(Ctx->InitializingDecl) {
     Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
@@ -3462,40 +3462,52 @@ template <class Emitter> bool 
Compiler<Emitter>::visitExpr(const Expr *E) {
   return false;
 }
 
-/// Toplevel visitDecl().
+template <class Emitter>
+VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD) {
+
+  auto R = this->visitVarDecl(VD, /*Toplevel=*/true);
+
+  if (R.notCreated())
+    return R;
+
+  if (R)
+    return true;
+
+  if (!R && Context::shouldBeGloballyIndexed(VD)) {
+    if (auto GlobalIndex = P.getGlobal(VD)) {
+      Block *GlobalBlock = P.getGlobal(*GlobalIndex);
+      GlobalInlineDescriptor &GD =
+          *reinterpret_cast<GlobalInlineDescriptor *>(GlobalBlock->rawData());
+
+      GD.InitState = GlobalInitState::InitializerFailed;
+      GlobalBlock->invokeDtor();
+    }
+  }
+
+  return R;
+}
+
+/// Toplevel visitDeclAndReturn().
 /// We get here from evaluateAsInitializer().
 /// We need to evaluate the initializer and return its value.
 template <class Emitter>
-bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool ConstantContext) {
-  assert(!VD->isInvalidDecl() && "Trying to constant evaluate an invalid 
decl");
-
+bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD,
+                                           bool ConstantContext) {
   std::optional<PrimType> VarT = classify(VD->getType());
 
   // We only create variables if we're evaluating in a constant context.
   // Otherwise, just evaluate the initializer and return it.
   if (!ConstantContext) {
-    DeclScope<Emitter> LocalScope(this, VD);
+    DeclScope<Emitter> LS(this, VD);
     if (!this->visit(VD->getAnyInitializer()))
       return false;
-    return this->emitRet(VarT.value_or(PT_Ptr), VD);
-  }
-
-  // If we've seen the global variable already and the initializer failed,
-  // just return false immediately.
-  if (std::optional<unsigned> Index = P.getGlobal(VD)) {
-    const Pointer &Ptr = P.getPtrGlobal(*Index);
-    const GlobalInlineDescriptor &GD =
-        *reinterpret_cast<const GlobalInlineDescriptor *>(
-            Ptr.block()->rawData());
-    if (GD.InitState == GlobalInitState::InitializerFailed)
-      return false;
+    return this->emitRet(VarT.value_or(PT_Ptr), VD) && LS.destroyLocals();
   }
 
-  // Create and initialize the variable.
+  LocalScope<Emitter> VDScope(this, VD);
   if (!this->visitVarDecl(VD, /*Toplevel=*/true))
     return false;
 
-  // Get a pointer to the variable
   if (Context::shouldBeGloballyIndexed(VD)) {
     auto GlobalIndex = P.getGlobal(VD);
     assert(GlobalIndex); // visitVarDecl() didn't return false.
@@ -3535,7 +3547,7 @@ bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool 
ConstantContext) {
     return false;
   }
 
-  return true;
+  return VDScope.destroyLocals();
 }
 
 template <class Emitter>
@@ -3589,26 +3601,34 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const 
VarDecl *VD, bool Topleve
 
     return !Init || (checkDecl() && initGlobal(*GlobalIndex));
   } else {
-    VariableScope<Emitter> LocalScope(this, VD);
     InitLinkScope<Emitter> ILS(this, InitLink::Decl(VD));
 
     if (VarT) {
       unsigned Offset = this->allocateLocalPrimitive(
           VD, *VarT, VD->getType().isConstQualified());
       if (Init) {
-        // Compile the initializer in its own scope.
-        ExprScope<Emitter> Scope(this);
-        if (!this->visit(Init))
-          return false;
-
-        return this->emitSetLocal(*VarT, Offset, VD);
+        // If this is a toplevel declaration, create a scope for the
+        // initializer.
+        if (Toplevel) {
+          ExprScope<Emitter> Scope(this);
+          if (!this->visit(Init))
+            return false;
+          return this->emitSetLocal(*VarT, Offset, VD);
+        } else {
+          if (!this->visit(Init))
+            return false;
+          return this->emitSetLocal(*VarT, Offset, VD);
+        }
       }
     } else {
-      if (std::optional<unsigned> Offset = this->allocateLocal(VD))
-        return !Init || this->visitLocalInitializer(Init, *Offset);
+      if (std::optional<unsigned> Offset = this->allocateLocal(VD)) {
+        if (!Init)
+          return true;
+
+        return this->visitLocalInitializer(Init, *Offset);
+      }
       return false;
     }
-
     return true;
   }
 
@@ -4074,7 +4094,7 @@ bool Compiler<Emitter>::visitCompoundStmt(const 
CompoundStmt *S) {
   for (const auto *InnerStmt : S->body())
     if (!visitStmt(InnerStmt))
       return false;
-  return true;
+  return Scope.destroyLocals();
 }
 
 template <class Emitter>
@@ -5010,6 +5030,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, 
const Expr *E) {
     }
   }
 
+  // In case we need to re-visit a declaration.
+  auto revisit = [&](const VarDecl *VD) -> bool {
+    auto VarState = this->visitDecl(VD);
+
+    if (VarState.notCreated())
+      return true;
+    if (!VarState)
+      return false;
+    // Retry.
+    return this->visitDeclRef(D, E);
+  };
+
   // Handle lambda captures.
   if (auto It = this->LambdaCaptures.find(D);
       It != this->LambdaCaptures.end()) {
@@ -5020,12 +5052,8 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, 
const Expr *E) {
     return this->emitGetPtrThisField(Offset, E);
   } else if (const auto *DRE = dyn_cast<DeclRefExpr>(E);
              DRE && DRE->refersToEnclosingVariableOrCapture()) {
-    if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture()) {
-      if (!this->visitVarDecl(cast<VarDecl>(D)))
-        return false;
-      // Retry.
-      return this->visitDeclRef(D, E);
-    }
+    if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture())
+      return revisit(VD);
   }
 
   if (D != InitializingDecl) {
@@ -5044,28 +5072,14 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl 
*D, const Expr *E) {
         // Visit local const variables like normal.
         if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
              VD->isStaticDataMember()) &&
-            typeShouldBeVisited(VD->getType())) {
-          auto VarState = this->visitVarDecl(VD, true);
-          if (VarState.notCreated())
-            return true;
-          if (!VarState)
-            return false;
-          // Retry.
-          return this->visitDeclRef(VD, E);
-        }
+            typeShouldBeVisited(VD->getType()))
+          return revisit(VD);
       }
     } else {
       if (const auto *VD = dyn_cast<VarDecl>(D);
           VD && VD->getAnyInitializer() &&
-          VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
-        auto VarState = this->visitVarDecl(VD, true);
-        if (VarState.notCreated())
-          return true;
-        if (!VarState)
-          return false;
-        // Retry.
-        return this->visitDeclRef(VD, E);
-      }
+          VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak())
+        return revisit(VD);
     }
   }
 

diff  --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h
index 1405abfc1a1e2..fbeda8a0b7a4f 100644
--- a/clang/lib/AST/Interp/Compiler.h
+++ b/clang/lib/AST/Interp/Compiler.h
@@ -212,9 +212,10 @@ class Compiler : public 
ConstStmtVisitor<Compiler<Emitter>, bool>,
 protected:
   bool visitStmt(const Stmt *S);
   bool visitExpr(const Expr *E) override;
-  bool visitDecl(const VarDecl *VD, bool ConstantContext) override;
   bool visitFunc(const FunctionDecl *F) override;
 
+  bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) override;
+
 protected:
   /// Emits scope cleanup instructions.
   void emitCleanup();
@@ -267,6 +268,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, 
bool>,
   bool delegate(const Expr *E);
   /// Creates and initializes a variable from the given decl.
   VarCreationState visitVarDecl(const VarDecl *VD, bool Toplevel = false);
+  VarCreationState visitDecl(const VarDecl *VD);
   /// Visit an APValue.
   bool visitAPValue(const APValue &Val, PrimType ValType, const Expr *E);
   bool visitAPValueInitializer(const APValue &Val, const Expr *E);
@@ -496,6 +498,8 @@ template <class Emitter> class VariableScope {
 template <class Emitter> class LocalScope : public VariableScope<Emitter> {
 public:
   LocalScope(Compiler<Emitter> *Ctx) : VariableScope<Emitter>(Ctx, nullptr) {}
+  LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
+      : VariableScope<Emitter>(Ctx, VD) {}
 
   /// Emit a Destroy op for this scope.
   ~LocalScope() override {

diff  --git a/clang/lib/AST/Interp/EvalEmitter.cpp 
b/clang/lib/AST/Interp/EvalEmitter.cpp
index acd1f2c673ce0..9701796fb9303 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -68,7 +68,7 @@ EvaluationResult EvalEmitter::interpretDecl(const VarDecl *VD,
 
   EvalResult.setSource(VD);
 
-  if (!this->visitDecl(VD, S.inConstantContext()) && EvalResult.empty())
+  if (!this->visitDeclAndReturn(VD, S.inConstantContext()))
     EvalResult.setInvalid();
 
   S.EvaluatingDecl = nullptr;

diff  --git a/clang/lib/AST/Interp/EvalEmitter.h 
b/clang/lib/AST/Interp/EvalEmitter.h
index d1e125cae9594..338786d3dea91 100644
--- a/clang/lib/AST/Interp/EvalEmitter.h
+++ b/clang/lib/AST/Interp/EvalEmitter.h
@@ -55,7 +55,7 @@ class EvalEmitter : public SourceMapper {
 
   /// Methods implemented by the compiler.
   virtual bool visitExpr(const Expr *E) = 0;
-  virtual bool visitDecl(const VarDecl *VD, bool ConstantContext) = 0;
+  virtual bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) = 0;
   virtual bool visitFunc(const FunctionDecl *F) = 0;
 
   /// Emits jumps.

diff  --git a/clang/test/C/C23/n3017.c b/clang/test/C/C23/n3017.c
index 0d22d31baa4b7..05337351dd869 100644
--- a/clang/test/C/C23/n3017.c
+++ b/clang/test/C/C23/n3017.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s 
-Wno-constant-logical-operand
+// RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s 
-Wno-constant-logical-operand -fexperimental-new-constant-interpreter
 
 /* WG14 N3017: full
  * #embed - a scannable, tooling-friendly binary resource inclusion mechanism

diff  --git a/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp 
b/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp
index a945c066e3bb2..2933d429c62f8 100644
--- a/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp
+++ b/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a 
-fexperimental-new-constant-interpreter | FileCheck %s
 
 // CHECK-DAG: @p = {{.*}} null
 // CHECK-DAG: @_ZGR1p_ = {{.*}} null

diff  --git a/clang/test/SemaCXX/warn-unused-variables.cpp 
b/clang/test/SemaCXX/warn-unused-variables.cpp
index 29e8d08d37d8c..216873aa4074b 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -2,6 +2,12 @@
 // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s
 // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s
 // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s
+
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s 
-fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s 
-fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label 
-Wno-c++1y-extensions -verify -std=gnu++17 %s 
-fexperimental-new-constant-interpreter
+
 template<typename T> void f() {
   T t;
   t = 17;


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to