[clang] [clang][Interp] Only evaluate the source array initialization of an `ArrayInitLoopExpr` once (PR #68039)

via cfe-commits cfe-commits at lists.llvm.org
Sun Oct 22 06:11:46 PDT 2023


https://github.com/isuckatcs updated https://github.com/llvm/llvm-project/pull/68039

>From 6748bd2171cca24c46ade12d57b57e97c3312501 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 2 Oct 2023 22:29:14 +0200
Subject: [PATCH 1/6] impl

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 21 +++++++++++++++++---
 clang/lib/AST/Interp/ByteCodeExprGen.h   | 25 ++++++++++++++++++++++++
 clang/test/AST/Interp/arrays.cpp         |  6 +-----
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index e9e20b222d5d34f..7d38d82f25331e4 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -810,9 +810,20 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
     const ArrayInitLoopExpr *E) {
   assert(Initializing);
   assert(!DiscardResult);
-  // TODO: This compiles to quite a lot of bytecode if the array is larger.
-  //   Investigate compiling this to a loop, or at least try to use
-  //   the AILE's Common expr.
+
+  const auto *CommonExpr = E->getCommonExpr();
+  std::optional<PrimType> CommonTy = classify(CommonExpr);
+
+  std::optional<unsigned> LocalIndex = this->allocateLocalPrimitive(CommonExpr, *CommonTy, CommonExpr->getType().isConstQualified());
+  if (!LocalIndex)
+    return false;
+  if (!this->visit(CommonExpr))
+    return false;
+  if(!this->emitSetLocal(*CommonTy, *LocalIndex, E))
+    return false;
+    
+  StoredOpaqueValueScope<Emitter> KnownOpaqueScope(this, *LocalIndex);
+
   const Expr *SubExpr = E->getSubExpr();
   size_t Size = E->getArraySize().getZExtValue();
   std::optional<PrimType> ElemT = classify(SubExpr->getType());
@@ -845,8 +856,12 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
+  if(IgnoreOpaqueValue)
+    return this->emitGetLocal(*classify(E), *OpaqueValueIndex, E);
+
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
+  
   return this->visit(E->getSourceExpr());
 }
 
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 2049dab140eaaae..a6c546384580b9a 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -36,6 +36,7 @@ template <class Emitter> class DeclScope;
 template <class Emitter> class OptionScope;
 template <class Emitter> class ArrayIndexScope;
 template <class Emitter> class SourceLocScope;
+template <class Emitter> class StoredOpaqueValueScope;
 
 /// Compilation context for expressions.
 template <class Emitter>
@@ -220,6 +221,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
   friend class OptionScope<Emitter>;
   friend class ArrayIndexScope<Emitter>;
   friend class SourceLocScope<Emitter>;
+  friend class StoredOpaqueValueScope<Emitter>;
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(PrimType T, QualType QT, const Expr *E);
@@ -304,6 +306,10 @@ 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 ignore an OpaqueValueExpr.
+  bool IgnoreOpaqueValue = false;
+  std::optional<uint64_t> OpaqueValueIndex;
 };
 
 extern template class ByteCodeExprGen<ByteCodeEmitter>;
@@ -479,6 +485,25 @@ template <class Emitter> class SourceLocScope final {
   bool Enabled = false;
 };
 
+template <class Emitter> class StoredOpaqueValueScope final {
+public:
+  StoredOpaqueValueScope(ByteCodeExprGen<Emitter> *Ctx, uint64_t LocalIndex, bool ignore = true)
+      : Ctx(Ctx), OldIgnoreValue(Ctx->IgnoreOpaqueValue), OldLocalIndex(Ctx->OpaqueValueIndex) {
+    Ctx->IgnoreOpaqueValue = ignore;
+    Ctx->OpaqueValueIndex = LocalIndex;
+  }
+
+  ~StoredOpaqueValueScope() {
+      Ctx->IgnoreOpaqueValue = OldIgnoreValue;
+      Ctx->OpaqueValueIndex = OldLocalIndex;
+  }
+
+private:
+  ByteCodeExprGen<Emitter> *Ctx;
+  bool OldIgnoreValue;
+  std::optional<uint64_t> OldLocalIndex;
+};
+
 } // namespace interp
 } // namespace clang
 
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 281835f828bbd7c..1f8908f2bed0b24 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -352,9 +352,6 @@ namespace ZeroInit {
 }
 
 namespace ArrayInitLoop {
-  /// FIXME: The ArrayInitLoop for the decomposition initializer in g() has
-  /// f(n) as its CommonExpr. We need to evaluate that exactly once and not
-  /// N times as we do right now.
   struct X {
       int arr[3];
   };
@@ -366,8 +363,7 @@ namespace ArrayInitLoop {
       auto [a, b, c] = f(n).arr;
       return a + b + c;
   }
-  static_assert(g() == 6); // expected-error {{failed}} \
-                           // expected-note {{15 == 6}}
+  static_assert(g() == 6);
 }
 
 namespace StringZeroFill {

>From 98ac7bdc5b07fcb23b8852b39179ecb711431f7c Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 2 Oct 2023 22:51:58 +0200
Subject: [PATCH 2/6] cleanup

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 18 +++---------
 clang/lib/AST/Interp/ByteCodeExprGen.h   | 36 ++++++++++++++++--------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 7d38d82f25331e4..7e4bf0f036a11db 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -811,18 +811,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
   assert(Initializing);
   assert(!DiscardResult);
 
-  const auto *CommonExpr = E->getCommonExpr();
-  std::optional<PrimType> CommonTy = classify(CommonExpr);
-
-  std::optional<unsigned> LocalIndex = this->allocateLocalPrimitive(CommonExpr, *CommonTy, CommonExpr->getType().isConstQualified());
-  if (!LocalIndex)
-    return false;
-  if (!this->visit(CommonExpr))
-    return false;
-  if(!this->emitSetLocal(*CommonTy, *LocalIndex, E))
-    return false;
-    
-  StoredOpaqueValueScope<Emitter> KnownOpaqueScope(this, *LocalIndex);
+  StoredOpaqueValueScope<Emitter> StoredOpaqueScope(this);
+  StoredOpaqueScope.VisitAndStoreOpaqueValue(E->getCommonExpr());
 
   const Expr *SubExpr = E->getSubExpr();
   size_t Size = E->getArraySize().getZExtValue();
@@ -856,8 +846,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-  if(IgnoreOpaqueValue)
-    return this->emitGetLocal(*classify(E), *OpaqueValueIndex, E);
+  if(OpaqueExprs.contains(E))
+    return this->emitGetLocal(*classify(E), OpaqueExprs[E], E);
 
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index a6c546384580b9a..6a25457f946ad0f 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -306,10 +306,6 @@ 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 ignore an OpaqueValueExpr.
-  bool IgnoreOpaqueValue = false;
-  std::optional<uint64_t> OpaqueValueIndex;
 };
 
 extern template class ByteCodeExprGen<ByteCodeEmitter>;
@@ -487,21 +483,37 @@ template <class Emitter> class SourceLocScope final {
 
 template <class Emitter> class StoredOpaqueValueScope final {
 public:
-  StoredOpaqueValueScope(ByteCodeExprGen<Emitter> *Ctx, uint64_t LocalIndex, bool ignore = true)
-      : Ctx(Ctx), OldIgnoreValue(Ctx->IgnoreOpaqueValue), OldLocalIndex(Ctx->OpaqueValueIndex) {
-    Ctx->IgnoreOpaqueValue = ignore;
-    Ctx->OpaqueValueIndex = LocalIndex;
+  StoredOpaqueValueScope(ByteCodeExprGen<Emitter> *Ctx) : Ctx(Ctx) {}
+
+  bool VisitAndStoreOpaqueValue(const OpaqueValueExpr* Ove) {
+    assert(Ove && "OpaqueValueExpr is a nullptr!");
+    assert(!Ctx->OpaqueExprs.contains(Ove) && "OpaqueValueExpr already stored!");
+
+    std::optional<PrimType> CommonTy = Ctx->classify(Ove);
+    std::optional<unsigned> LocalIndex = Ctx->allocateLocalPrimitive(Ove, *CommonTy, Ove->getType().isConstQualified());
+    if (!LocalIndex)
+      return false;
+
+    if (!Ctx->visit(Ove))
+      return false;
+
+    if(!Ctx->emitSetLocal(*CommonTy, *LocalIndex, Ove))
+      return false;
+
+    Ctx->OpaqueExprs.insert({Ove, *LocalIndex});
+    StoredValues.emplace_back(Ove);
+
+    return true;
   }
 
   ~StoredOpaqueValueScope() {
-      Ctx->IgnoreOpaqueValue = OldIgnoreValue;
-      Ctx->OpaqueValueIndex = OldLocalIndex;
+    for(const auto *SV : StoredValues)
+      Ctx->OpaqueExprs.erase(SV);
   }
 
 private:
   ByteCodeExprGen<Emitter> *Ctx;
-  bool OldIgnoreValue;
-  std::optional<uint64_t> OldLocalIndex;
+  std::vector<const OpaqueValueExpr*> StoredValues;
 };
 
 } // namespace interp

>From 530cf3a695db2d12e45092897fa82d11524a5cc6 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 2 Oct 2023 23:12:02 +0200
Subject: [PATCH 3/6] applied clang-format

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp |  3 +--
 clang/lib/AST/Interp/ByteCodeExprGen.h   | 14 ++++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 7e4bf0f036a11db..6569e63a551cf0a 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -846,12 +846,11 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-  if(OpaqueExprs.contains(E))
+  if (OpaqueExprs.contains(E))
     return this->emitGetLocal(*classify(E), OpaqueExprs[E], E);
 
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
-  
   return this->visit(E->getSourceExpr());
 }
 
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 6a25457f946ad0f..6042fe6c0acdf87 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -485,19 +485,21 @@ template <class Emitter> class StoredOpaqueValueScope final {
 public:
   StoredOpaqueValueScope(ByteCodeExprGen<Emitter> *Ctx) : Ctx(Ctx) {}
 
-  bool VisitAndStoreOpaqueValue(const OpaqueValueExpr* Ove) {
+  bool VisitAndStoreOpaqueValue(const OpaqueValueExpr *Ove) {
     assert(Ove && "OpaqueValueExpr is a nullptr!");
-    assert(!Ctx->OpaqueExprs.contains(Ove) && "OpaqueValueExpr already stored!");
+    assert(!Ctx->OpaqueExprs.contains(Ove) &&
+           "OpaqueValueExpr already stored!");
 
     std::optional<PrimType> CommonTy = Ctx->classify(Ove);
-    std::optional<unsigned> LocalIndex = Ctx->allocateLocalPrimitive(Ove, *CommonTy, Ove->getType().isConstQualified());
+    std::optional<unsigned> LocalIndex = Ctx->allocateLocalPrimitive(
+        Ove, *CommonTy, Ove->getType().isConstQualified());
     if (!LocalIndex)
       return false;
 
     if (!Ctx->visit(Ove))
       return false;
 
-    if(!Ctx->emitSetLocal(*CommonTy, *LocalIndex, Ove))
+    if (!Ctx->emitSetLocal(*CommonTy, *LocalIndex, Ove))
       return false;
 
     Ctx->OpaqueExprs.insert({Ove, *LocalIndex});
@@ -507,13 +509,13 @@ template <class Emitter> class StoredOpaqueValueScope final {
   }
 
   ~StoredOpaqueValueScope() {
-    for(const auto *SV : StoredValues)
+    for (const auto *SV : StoredValues)
       Ctx->OpaqueExprs.erase(SV);
   }
 
 private:
   ByteCodeExprGen<Emitter> *Ctx;
-  std::vector<const OpaqueValueExpr*> StoredValues;
+  std::vector<const OpaqueValueExpr *> StoredValues;
 };
 
 } // namespace interp

>From 4a1acf32dc32c501d0f06ce52f7a012d0ae2bb15 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 2 Oct 2023 23:22:36 +0200
Subject: [PATCH 4/6] return if ove evaluation fails

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 6569e63a551cf0a..344dd27933a794e 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -812,7 +812,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
   assert(!DiscardResult);
 
   StoredOpaqueValueScope<Emitter> StoredOpaqueScope(this);
-  StoredOpaqueScope.VisitAndStoreOpaqueValue(E->getCommonExpr());
+  if (!StoredOpaqueScope.VisitAndStoreOpaqueValue(E->getCommonExpr()))
+    return false;
 
   const Expr *SubExpr = E->getSubExpr();
   size_t Size = E->getArraySize().getZExtValue();

>From a2cac4ce48aaf865278e982edf47f7b79a66532d Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 15 Oct 2023 18:35:27 +0200
Subject: [PATCH 5/6] addressed comments

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 35 ++++++++++++++++-----
 clang/lib/AST/Interp/ByteCodeExprGen.h   | 39 ------------------------
 clang/test/AST/Interp/arrays.cpp         |  2 +-
 3 files changed, 28 insertions(+), 48 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 344dd27933a794e..90d59b0f4e0ee0a 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -811,10 +811,6 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
   assert(Initializing);
   assert(!DiscardResult);
 
-  StoredOpaqueValueScope<Emitter> StoredOpaqueScope(this);
-  if (!StoredOpaqueScope.VisitAndStoreOpaqueValue(E->getCommonExpr()))
-    return false;
-
   const Expr *SubExpr = E->getSubExpr();
   size_t Size = E->getArraySize().getZExtValue();
   std::optional<PrimType> ElemT = classify(SubExpr->getType());
@@ -847,12 +843,35 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-  if (OpaqueExprs.contains(E))
-    return this->emitGetLocal(*classify(E), OpaqueExprs[E], E);
-
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
-  return this->visit(E->getSourceExpr());
+
+  PrimType CacheVariableTy = classify(E).value_or(PT_Ptr);
+  if (OpaqueExprs.contains(E))
+    return this->emitGetLocal(CacheVariableTy, OpaqueExprs[E], E);
+
+  if (!this->visit(E->getSourceExpr()))
+    return false;
+
+  // At this point we either have the evaluated source expression or a pointer
+  // to an object on the stack. We want to create a local variable that stores
+  // this value.
+  std::optional<unsigned> LocalIndex =
+      allocateLocalPrimitive(E, CacheVariableTy, true);
+  if (!LocalIndex)
+    return false;
+  if (!this->emitSetLocal(CacheVariableTy, *LocalIndex, 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.
+  if (!this->emitGetLocal(CacheVariableTy, *LocalIndex, E))
+    return false;
+
+  // FIXME: Ideally the cached value should be cleaned up later.
+  OpaqueExprs.insert({E, *LocalIndex});
+
+  return true;
 }
 
 template <class Emitter>
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 6042fe6c0acdf87..2049dab140eaaae 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -36,7 +36,6 @@ template <class Emitter> class DeclScope;
 template <class Emitter> class OptionScope;
 template <class Emitter> class ArrayIndexScope;
 template <class Emitter> class SourceLocScope;
-template <class Emitter> class StoredOpaqueValueScope;
 
 /// Compilation context for expressions.
 template <class Emitter>
@@ -221,7 +220,6 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
   friend class OptionScope<Emitter>;
   friend class ArrayIndexScope<Emitter>;
   friend class SourceLocScope<Emitter>;
-  friend class StoredOpaqueValueScope<Emitter>;
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(PrimType T, QualType QT, const Expr *E);
@@ -481,43 +479,6 @@ template <class Emitter> class SourceLocScope final {
   bool Enabled = false;
 };
 
-template <class Emitter> class StoredOpaqueValueScope final {
-public:
-  StoredOpaqueValueScope(ByteCodeExprGen<Emitter> *Ctx) : Ctx(Ctx) {}
-
-  bool VisitAndStoreOpaqueValue(const OpaqueValueExpr *Ove) {
-    assert(Ove && "OpaqueValueExpr is a nullptr!");
-    assert(!Ctx->OpaqueExprs.contains(Ove) &&
-           "OpaqueValueExpr already stored!");
-
-    std::optional<PrimType> CommonTy = Ctx->classify(Ove);
-    std::optional<unsigned> LocalIndex = Ctx->allocateLocalPrimitive(
-        Ove, *CommonTy, Ove->getType().isConstQualified());
-    if (!LocalIndex)
-      return false;
-
-    if (!Ctx->visit(Ove))
-      return false;
-
-    if (!Ctx->emitSetLocal(*CommonTy, *LocalIndex, Ove))
-      return false;
-
-    Ctx->OpaqueExprs.insert({Ove, *LocalIndex});
-    StoredValues.emplace_back(Ove);
-
-    return true;
-  }
-
-  ~StoredOpaqueValueScope() {
-    for (const auto *SV : StoredValues)
-      Ctx->OpaqueExprs.erase(SV);
-  }
-
-private:
-  ByteCodeExprGen<Emitter> *Ctx;
-  std::vector<const OpaqueValueExpr *> StoredValues;
-};
-
 } // namespace interp
 } // namespace clang
 
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 1f8908f2bed0b24..fcbaa31e8608ec3 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -363,7 +363,7 @@ namespace ArrayInitLoop {
       auto [a, b, c] = f(n).arr;
       return a + b + c;
   }
-  static_assert(g() == 6);
+  static_assert(g() == 6, "");
 }
 
 namespace StringZeroFill {

>From c00fe79daf07b1136ad7d1ed4428be643ec73ad7 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 22 Oct 2023 15:11:14 +0200
Subject: [PATCH 6/6] addressed comments

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 14 ++++++++------
 clang/test/AST/Interp/cxx20.cpp          | 12 ++++++++++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 90d59b0f4e0ee0a..bb639f714b60c5e 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -810,6 +810,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
     const ArrayInitLoopExpr *E) {
   assert(Initializing);
   assert(!DiscardResult);
+  // 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();
   size_t Size = E->getArraySize().getZExtValue();
@@ -846,9 +848,9 @@ bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
 
-  PrimType CacheVariableTy = classify(E).value_or(PT_Ptr);
-  if (OpaqueExprs.contains(E))
-    return this->emitGetLocal(CacheVariableTy, OpaqueExprs[E], E);
+  PrimType SubExprT = classify(E->getSourceExpr()).value_or(PT_Ptr);
+  if (auto It = OpaqueExprs.find(E); It != OpaqueExprs.end())
+    return this->emitGetLocal(SubExprT, It->getSecond(), E);
 
   if (!this->visit(E->getSourceExpr()))
     return false;
@@ -857,15 +859,15 @@ bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
   // to an object on the stack. We want to create a local variable that stores
   // this value.
   std::optional<unsigned> LocalIndex =
-      allocateLocalPrimitive(E, CacheVariableTy, true);
+      allocateLocalPrimitive(E, SubExprT, /*IsConst=*/true);
   if (!LocalIndex)
     return false;
-  if (!this->emitSetLocal(CacheVariableTy, *LocalIndex, E))
+  if (!this->emitSetLocal(SubExprT, *LocalIndex, 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.
-  if (!this->emitGetLocal(CacheVariableTy, *LocalIndex, E))
+  if (!this->emitGetLocal(SubExprT, *LocalIndex, E))
     return false;
 
   // FIXME: Ideally the cached value should be cleaned up later.
diff --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp
index 197090b0a37d9df..553bc6eb4d5244f 100644
--- a/clang/test/AST/Interp/cxx20.cpp
+++ b/clang/test/AST/Interp/cxx20.cpp
@@ -733,3 +733,15 @@ namespace ConstexprArrayInitLoopExprDestructors
       return f();
   }
 }
+
+namespace NonPrimitiveOpaqueValue
+{
+  struct X {
+    int x;
+    constexpr operator bool() const { return x != 0; }
+  };
+
+  constexpr int ternary() { return X(0) ?: X(0); }
+
+  static_assert(!ternary(), "");
+}



More information about the cfe-commits mailing list