[clang] [clang][Interp] Cleaning up `FIXME`s added during `ArrayInitLoopExpr` implementation. (PR #70053)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (isuckatcs)

<details>
<summary>Changes</summary>

This patch removes the two `FIXME`s that were added with patches related to the expression mentioned in the title.

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


2 Files Affected:

- (modified) clang/lib/AST/Interp/ByteCodeExprGen.cpp (+8-12) 
- (modified) clang/lib/AST/Interp/ByteCodeExprGen.h (+20) 


``````````diff
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 1b33c69b93aa4b9..8d18698df60c2c1 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -816,22 +816,18 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
     const ArrayInitLoopExpr *E) {
   assert(Initializing);
   assert(!DiscardResult);
+
+  // We visit the common opaque expression here once so we have its value
+  // cached.
+  if (!this->discard(E->getCommonExpr()))
+    return false;
+
   // TODO: This compiles to quite a lot of bytecode if the array is larger.
   //   Investigate compiling this to a loop.
-
   const Expr *SubExpr = E->getSubExpr();
-  const Expr *CommonExpr = E->getCommonExpr();
   size_t Size = E->getArraySize().getZExtValue();
   std::optional<PrimType> ElemT = classify(SubExpr->getType());
 
-  // If the common expression is an opaque expression, we visit it
-  // here once so we have its value cached.
-  // FIXME: This might be necessary (or useful) for all expressions.
-  if (isa<OpaqueValueExpr>(CommonExpr)) {
-    if (!this->discard(CommonExpr))
-      return false;
-  }
-
   // So, every iteration, we execute an assignment here
   // where the LHS is on the stack (the target array)
   // and the RHS is our SubExpr.
@@ -882,13 +878,13 @@ bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
     return false;
 
   // Here the local variable is created but the value is removed from the stack,
-  // so we put it back, because the caller might need it.
+  // so we put it back if the caller needs it.
   if (!DiscardResult) {
     if (!this->emitGetLocal(SubExprT, *LocalIndex, E))
       return false;
   }
 
-  // FIXME: Ideally the cached value should be cleaned up later.
+  // This is cleaned up when the local variable is destroyed.
   OpaqueExprs.insert({E, *LocalIndex});
 
   return true;
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 83986d3dd579ed6..774d0b25f4ad56d 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -360,6 +360,7 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
     if (!Idx)
       return;
     this->Ctx->emitDestroy(*Idx, SourceInfo{});
+    removeStoredOpaqueValues();
   }
 
   /// Overriden to support explicit destruction.
@@ -368,6 +369,7 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
       return;
     this->emitDestructors();
     this->Ctx->emitDestroy(*Idx, SourceInfo{});
+    removeStoredOpaqueValues();
     this->Idx = std::nullopt;
   }
 
@@ -389,10 +391,28 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
       if (!Local.Desc->isPrimitive() && !Local.Desc->isPrimitiveArray()) {
         this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{});
         this->Ctx->emitRecordDestruction(Local.Desc);
+        removeIfStoredOpaqueValue(Local);
       }
     }
   }
 
+  void removeStoredOpaqueValues() {
+    if (!Idx)
+      return;
+
+    for (Scope::Local &Local : this->Ctx->Descriptors[*Idx]) {
+      removeIfStoredOpaqueValue(Local);
+    }
+  }
+
+  void removeIfStoredOpaqueValue(const Scope::Local &Local) {
+    if (auto *OVE =
+            llvm::dyn_cast_or_null<OpaqueValueExpr>(Local.Desc->asExpr());
+        OVE && this->Ctx->OpaqueExprs.contains(OVE)) {
+      this->Ctx->OpaqueExprs.erase(OVE);
+    };
+  }
+
   /// Index of the scope in the chain.
   std::optional<unsigned> Idx;
 };

``````````

</details>


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