[clang] 6e83209 - [clang][Interp] Fix copy constructors with record array members

Timm Bäder via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 14 03:00:21 PDT 2022


Author: Timm Bäder
Date: 2022-10-14T11:57:26+02:00
New Revision: 6e83209f623e00b16f5858efbfa0fd6409abf1fe

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

LOG: [clang][Interp] Fix copy constructors with record array members

Differential Revision: https://reviews.llvm.org/D134523

Added: 
    

Modified: 
    clang/lib/AST/Interp/ByteCodeExprGen.cpp
    clang/lib/AST/Interp/Interp.h
    clang/lib/AST/Interp/Pointer.cpp
    clang/test/AST/Interp/records.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 5af97ebbcf27..904a99b4f872 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -329,7 +329,10 @@ bool ByteCodeExprGen<Emitter>::VisitMemberExpr(const MemberExpr *E) {
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitArrayInitIndexExpr(
     const ArrayInitIndexExpr *E) {
-  assert(ArrayIndex);
+  // ArrayIndex might not be set if a ArrayInitIndexExpr is being evaluated
+  // stand-alone, e.g. via EvaluateAsInt().
+  if (!ArrayIndex)
+    return false;
   QualType IndexType = E->getType();
   APInt Value(getIntWidth(IndexType), *ArrayIndex);
   return this->emitConst(classifyPrim(IndexType), 0, Value, E);
@@ -608,9 +611,15 @@ bool ByteCodeExprGen<Emitter>::visitArrayInitializer(const Expr *Initializer) {
   if (const auto *InitList = dyn_cast<InitListExpr>(Initializer)) {
     unsigned ElementIndex = 0;
     for (const Expr *Init : InitList->inits()) {
-      QualType InitType = Init->getType();
-
-      if (InitType->isArrayType()) {
+      if (Optional<PrimType> T = classify(Init->getType())) {
+        // Visit the primitive element like normal.
+        if (!this->emitDupPtr(Init))
+          return false;
+        if (!this->visit(Init))
+          return false;
+        if (!this->emitInitElem(*T, ElementIndex, Init))
+          return false;
+      } else {
         // Advance the pointer currently on the stack to the given
         // dimension and narrow().
         if (!this->emitDupPtr(Init))
@@ -621,21 +630,12 @@ bool ByteCodeExprGen<Emitter>::visitArrayInitializer(const Expr *Initializer) {
           return false;
         if (!this->emitNarrowPtr(Init))
           return false;
-        if (!visitArrayInitializer(Init))
+
+        if (!visitInitializer(Init))
           return false;
+      }
         if (!this->emitPopPtr(Init))
           return false;
-      } else if (Optional<PrimType> T = classify(InitType)) {
-        // Visit the primitive element like normal.
-        if (!this->emitDupPtr(Init))
-          return false;
-        if (!this->visit(Init))
-          return false;
-        if (!this->emitInitElemPop(*T, ElementIndex, Init))
-          return false;
-      } else {
-        assert(false && "Unhandled type in array initializer initlist");
-      }
 
       ++ElementIndex;
     }
@@ -650,19 +650,34 @@ bool ByteCodeExprGen<Emitter>::visitArrayInitializer(const Expr *Initializer) {
     size_t Size = AILE->getArraySize().getZExtValue();
     Optional<PrimType> ElemT = classify(SubExpr->getType());
 
-    if (!ElemT)
-      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.
     for (size_t I = 0; I != Size; ++I) {
       ArrayIndexScope<Emitter> IndexScope(this, I);
-      if (!this->emitDupPtr(SubExpr))
-        return false;
 
-      if (!this->visit(SubExpr))
+      if (!this->emitDupPtr(SubExpr)) // LHS
         return false;
 
-      if (!this->emitInitElem(*ElemT, I, Initializer))
-        return false;
+      if (ElemT) {
+        if (!this->visit(SubExpr))
+          return false;
+        if (!this->emitInitElem(*ElemT, I, Initializer))
+          return false;
+      } else {
+        // Narrow to our array element and recurse into visitInitializer()
+        if (!this->emitConstUint64(I, SubExpr))
+          return false;
+
+        if (!this->emitAddOffsetUint64(SubExpr))
+          return false;
+
+        if (!this->emitNarrowPtr(SubExpr))
+          return false;
+
+        if (!visitInitializer(SubExpr))
+          return false;
+      }
 
       if (!this->emitPopPtr(Initializer))
         return false;

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 9356c572ddd9..5d5164a95f00 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -494,7 +494,7 @@ bool InitThisFieldActive(InterpState &S, CodePtr OpPC, uint32_t I) {
 
 /// 1) Pops the value from the stack
 /// 2) Pops a pointer from the stack
-/// 3) Writes the value to field I of the pointer
+/// 3) Pushes the value to field I of the pointer on the stack
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitField(InterpState &S, CodePtr OpPC, uint32_t I) {
   const T &Value = S.Stk.pop<T>();
@@ -548,6 +548,8 @@ inline bool GetPtrGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
   return true;
 }
 
+/// 1) Pops a Pointer from the stack
+/// 2) Pushes Pointer.atField(Off) on the stack
 inline bool GetPtrField(InterpState &S, CodePtr OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.pop<Pointer>();
   if (!CheckNull(S, OpPC, Ptr, CSK_Field))

diff  --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp
index 1dfb8f4b18eb..ec00afc37124 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -135,6 +135,7 @@ APValue Pointer::toAPValue() const {
 bool Pointer::isInitialized() const {
   assert(Pointee && "Cannot check if null pointer was initialized");
   Descriptor *Desc = getFieldDesc();
+  assert(Desc);
   if (Desc->isPrimitiveArray()) {
     if (Pointee->IsStatic)
       return true;
@@ -155,6 +156,7 @@ void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
 
+  assert(Desc);
   if (Desc->isArray()) {
     if (Desc->isPrimitiveArray() && !Pointee->IsStatic) {
       // Primitive array initializer.

diff  --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 002ea1985923..8839c597530c 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -164,3 +164,24 @@ namespace thisPointer {
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
                                   // ref-note {{in call to 'foo()'}}
 };
+
+struct FourBoolPairs {
+  BoolPair v[4] = {
+    {false, false},
+    {false,  true},
+    {true,  false},
+    {true,  true },
+  };
+};
+// Init
+constexpr FourBoolPairs LT;
+// Copy ctor
+constexpr FourBoolPairs LT2 = LT;
+// FIXME: The copy constructor call above
+//   works, but APValue we generate for it is
+//   not sufficiently correct, so the lvalue-to-rvalue
+//   conversion in ExprConstant.c runs into an assertion.
+//static_assert(LT2.v[0].first == false, "");
+//static_assert(LT2.v[0].second == false, "");
+//static_assert(LT2.v[2].first == true, "");
+//static_assert(LT2.v[2].second == false, "");


        


More information about the cfe-commits mailing list