[clang] [clang][bytecode] Only visit local variables if they have constant init (PR #107576)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

See the comment I added for why this is weird. We might want to have a different mechanism for this in the future.

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


4 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+10-3) 
- (modified) clang/lib/AST/ByteCode/Context.cpp (+10-6) 
- (modified) clang/lib/AST/ByteCode/InterpStack.cpp (+25) 
- (modified) clang/lib/AST/ByteCode/InterpStack.h (+1) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index a831f196abdcb5..2e9807b2d8426b 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -5507,11 +5507,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
         if (isa<DecompositionDecl>(VD))
           return revisit(VD);
 
-        // Visit local const variables like normal.
-        if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
-             VD->isStaticDataMember()) &&
+        if ((VD->hasGlobalStorage() || VD->isStaticDataMember()) &&
             typeShouldBeVisited(VD->getType()))
           return revisit(VD);
+
+        // FIXME: The evaluateValue() check here is a little ridiculous, since
+        // it will ultimately call into Context::evaluateAsInitializer(). In
+        // other words, we're evaluating the initializer, just to know if we can
+        // evaluate the initializer.
+        if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) &&
+            VD->getInit() && !VD->getInit()->isValueDependent() &&
+            VD->evaluateValue())
+          return revisit(VD);
       }
     } else {
       if (const auto *VD = dyn_cast<VarDecl>(D);
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index e682d87b703ad7..594c74cd3796d6 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -44,13 +44,14 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
 bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue());
 
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
     return false;
   }
 
@@ -60,7 +61,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 
@@ -72,12 +73,13 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   auto Res = C.interpretExpr(E);
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
     return false;
   }
 
@@ -87,7 +89,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 
@@ -99,6 +101,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
                                     APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   bool CheckGlobalInitialized =
@@ -107,7 +110,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
   auto Res = C.interpretDecl(VD, CheckGlobalInitialized);
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
+
     return false;
   }
 
@@ -117,7 +121,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 
diff --git a/clang/lib/AST/ByteCode/InterpStack.cpp b/clang/lib/AST/ByteCode/InterpStack.cpp
index b8cdaeee72166c..ae3721e983741d 100644
--- a/clang/lib/AST/ByteCode/InterpStack.cpp
+++ b/clang/lib/AST/ByteCode/InterpStack.cpp
@@ -32,6 +32,16 @@ void InterpStack::clear() {
 #endif
 }
 
+void InterpStack::clearTo(size_t NewSize) {
+  assert(NewSize <= size());
+  size_t ToShrink = size() - NewSize;
+  if (ToShrink == 0)
+    return;
+
+  shrink(ToShrink);
+  assert(size() == NewSize);
+}
+
 void *InterpStack::grow(size_t Size) {
   assert(Size < ChunkSize - sizeof(StackChunk) && "Object too large");
 
@@ -81,6 +91,21 @@ void InterpStack::shrink(size_t Size) {
 
   Chunk->End -= Size;
   StackSize -= Size;
+
+#ifndef NDEBUG
+  size_t TypesSize = 0;
+  for (PrimType T : ItemTypes)
+    TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); });
+
+  size_t StackSize = size();
+  while (TypesSize > StackSize) {
+    TYPE_SWITCH(ItemTypes.back(), {
+      TypesSize -= aligned_size<T>();
+      ItemTypes.pop_back();
+    });
+  }
+  assert(TypesSize == StackSize);
+#endif
 }
 
 void InterpStack::dump() const {
diff --git a/clang/lib/AST/ByteCode/InterpStack.h b/clang/lib/AST/ByteCode/InterpStack.h
index 153d17f0d70e1b..43988bb680d1c6 100644
--- a/clang/lib/AST/ByteCode/InterpStack.h
+++ b/clang/lib/AST/ByteCode/InterpStack.h
@@ -86,6 +86,7 @@ class InterpStack final {
 
   /// Clears the stack without calling any destructors.
   void clear();
+  void clearTo(size_t NewSize);
 
   /// Returns whether the stack is empty.
   bool empty() const { return StackSize == 0; }

``````````

</details>


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