[clang][Interp] Fix copy constructors of structs with array members

[tbaeder]

Summary

This implements ArrayInitLoopExprs in initializers and fixes a few issues I encountered along the way, with copy constructors generally.

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

641

654–664

In all of these cases we're leaving ArrayIndex set to I instead of None, is that intentional? (Might be worth an RAII object to handle this sort of thing.)

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

654–664

Heh :) Good you notice this as well. Yes, that's something I was worried about. It's not intentional at all. I'll try adding a RAII object.

[aaron.ballman]

LGTM!

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

649

Curious what case requires this check?

clang/lib/AST/Interp/ByteCodeExprGen.h

380

Why an Optional? Your not checking it and I don't see how it won't be set?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

649

I don't think there's a real test case for this, we could as well change the initializer to *classify(SubExpr->getType());. It's just the pattern used everywhere else. We could also use classifyPrim instead.

clang/lib/AST/Interp/ByteCodeExprGen.h

380

The first time we see an ArrayInitLoopExpr, the ByteCodeExprGen<Emitter>::ArrayIndex will be None, so we need to use this here (note: just writing this right now, it's pretty late for me, I didn't test just using a uint64_t)

clang/lib/AST/Interp/ByteCodeExprGen.h

public:
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
   bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E);
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
+  bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
   bool visitDecl(const VarDecl *VD) override;
 
 protected:
   /// Emits scope cleanup instructions.
   void emitCleanup();
protected:
   llvm::DenseMap<const ValueDecl *, Scope::Local> Locals;
 
   /// OpaqueValueExpr to location mapping.
   llvm::DenseMap<const OpaqueValueExpr *, unsigned> OpaqueExprs;
 
   /// Current scope.
   VariableScope<Emitter> *VarScope = nullptr;
 
-  /// Current argument index.
+  /// Current argument index. Needed to emit ArrayInitIndexExpr.
   llvm::Optional<uint64_t> ArrayIndex;
 
   /// Flag indicating if return value is to be discarded.
   bool DiscardResult = false;
 
   /// Expression being initialized.
   llvm::Optional<InitFnRef> InitFn = {};
 };
public:
   void addExtended(const Scope::Local &Local) override {
     this->Parent->addLocal(Local);
   }
 };
 
 } // namespace interp
 } // namespace clang
 
 #endif

clang/lib/AST/Interp/ByteCodeExprGen.cpp

if (const auto *FD = dyn_cast<FieldDecl>(Member)) {
     const Record::Field *F = R->getField(FD);
     // Leave a pointer to the field on the stack.
     return this->emitGetPtrField(F->Offset, E);
   }
 
   return false;
 }
 
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitArrayInitIndexExpr(
+    const ArrayInitIndexExpr *E) {
+  assert(ArrayIndex);
+  return this->emitConstUint64(*ArrayIndex, E);
+}
+
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
+  return this->visit(E->getSourceExpr());
+}
+
 template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visit(const Expr *E) {
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/false);
for (const Expr *Init : InitList->inits()) {
         assert(false && "Unhandled type in array initializer initlist");
       }
 
       ++ElementIndex;
     }
     return true;
   } else if (const auto *DIE = dyn_cast<CXXDefaultInitExpr>(Initializer)) {
     return this->visitInitializer(DIE->getExpr());
+  } else if (const auto AILE = dyn_cast<ArrayInitLoopExpr>(Initializer)) {
+    // 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 Expr *SubExpr = AILE->getSubExpr();
+    size_t Size = AILE->getArraySize().getZExtValue();
+    Optional<PrimType> ElemT = classify(SubExpr->getType());
+
+    if (!ElemT)
+      return false;
+
+    for (size_t I = 0; I != Size; ++I) {
+      ArrayIndex = I;
+      if (!this->emitDupPtr(SubExpr))
+        return false;
+
+      if (!this->visit(SubExpr))
+        return false;
+
+      if (!this->emitInitElem(*ElemT, I, Initializer))
+        return false;
+
+      if (!this->emitPopPtr(Initializer))
+        return false;
+    }
+    ArrayIndex = None;
+    return true;
   }
 
   assert(false && "Unknown expression for array initialization");
   return false;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitRecordInitializer(const Expr *Initializer) {
   Initializer = Initializer->IgnoreParenImpCasts();
   assert(Initializer->getType()->isRecordType());
 
   if (const auto CtorExpr = dyn_cast<CXXConstructExpr>(Initializer)) {
     const Function *Func = getFunction(CtorExpr->getConstructor());
 
+    if (!Func->isConstexpr())
+      return false;
+
     // The This pointer is already on the stack because this is an initializer,
     // but we need to dup() so the call() below has its own copy.
     if (!this->emitDupPtr(Initializer))
       return false;
+
+    // Constructor arguments.
+    for (const auto *Arg : CtorExpr->arguments()) {
+      if (!this->visit(Arg))
+        return false;
+    }
+
     return this->emitCallVoid(Func, Initializer);
   } else if (const auto *InitList = dyn_cast<InitListExpr>(Initializer)) {
     const Record *R = getRecord(InitList->getType());
 
     unsigned InitIndex = 0;
     for (const Expr *Init : InitList->inits()) {
       const Record::Field *FieldToInit = R->getField(InitIndex);
 
-      if (Optional<PrimType> T = classify(Init->getType())) {
-        if (!this->emitDupPtr(Initializer))
-          return false;
 
+      if (Optional<PrimType> T = classify(Init->getType())) {
         if (!this->visit(Init))
           return false;
 
         if (!this->emitInitField(*T, FieldToInit->Offset, Initializer))
           return false;
+      } else {
+        // Non-primitive case. Get a pointer to the field-to-initialize
+        // on the stack and recurse into visitInitializer().
+        if (!this->emitGetPtrField(FieldToInit->Offset, Init))
+          return false;
+
+        if (!this->visitInitializer(Init))
+          return false;
+
+        if (!this->emitPopPtr(Initializer))
+          return false;
       }
       ++InitIndex;
     }
 
     return true;
   } else if (const CallExpr *CE = dyn_cast<CallExpr>(Initializer)) {
     const Decl *Callee = CE->getCalleeDecl();
     const Function *Func = getFunction(dyn_cast<FunctionDecl>(Callee));

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

if (const auto Ctor = dyn_cast<CXXConstructorDecl>(F)) {
     const Record *R = this->getRecord(RD);
 
     for (const auto *Init : Ctor->inits()) {
       const FieldDecl *Member = Init->getMember();
       const Expr *InitExpr = Init->getInit();
       const Record::Field *F = R->getField(Member);
 
       if (Optional<PrimType> T = this->classify(InitExpr->getType())) {
-        if (!this->emitDupPtr(InitExpr))
+        if (!this->emitThis(InitExpr))
           return false;
 
         if (!this->visit(InitExpr))
           return false;
 
         if (!this->emitInitField(*T, F->Offset, InitExpr))
           return false;
       } else {
         // Non-primitive case. Get a pointer to the field-to-initialize
         // on the stack and call visitInitialzer() for it.
-        if (!this->emitDupPtr(InitExpr))
+        if (!this->emitThis(InitExpr))
           return false;
 
         if (!this->emitGetPtrField(F->Offset, InitExpr))
           return false;
 
         if (!this->visitInitializer(InitExpr))
           return false;
 

clang/test/AST/Interp/records.cpp

 
 static_assert(Ints::getFive() == 5, "");
 
 constexpr Ints ints;
 static_assert(ints.a == 20, "");
 static_assert(ints.b == 30, "");
 static_assert(ints.c, "");
 static_assert(ints.getTen() == 10, "");
+static_assert(ints.numbers[0] == 1, "");
+static_assert(ints.numbers[1] == 2, "");
+static_assert(ints.numbers[2] == 3, "");
 
 constexpr const BoolPair &BP = ints.bp;
 static_assert(BP.first, "");
 static_assert(!BP.second, "");
 static_assert(ints.bp.first, "");
 static_assert(!ints.bp.second, "");
 
 
constexpr Ints ints4 = {
   .a = 40 * 50,
   .b = 0,
   .c = (ints.a > 0),
 
 };
 static_assert(ints4.a == (40 * 50), "");
 static_assert(ints4.b == 0, "");
 static_assert(ints4.c, "");
-
-
-// FIXME: Implement initialization by DeclRefExpr.
-//constexpr Ints ints4 = ints3;  TODO
-
+static_assert(ints4.numbers[0] == 1, "");
+static_assert(ints4.numbers[1] == 2, "");
+static_assert(ints4.numbers[2] == 3, "");
+
+constexpr Ints ints5 = ints4;
+static_assert(ints5.a == (40 * 50), "");
+static_assert(ints5.b == 0, "");
+static_assert(ints5.c, "");
+static_assert(ints5.numbers[0] == 1, "");
+static_assert(ints5.numbers[1] == 2, "");
+static_assert(ints5.numbers[2] == 3, "");
 
 
 struct Ints2 {
   int a = 10;
   int b;
 };
 // FIXME: Broken in the new constant interpreter.
 //   Should be rejected, but without asan errors.