[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:277
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitConstantExpr (const ConstantExpr *E) {
+  // TODO: Check if the ConstantExpr already has a value set and if so,
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:246
 
-    auto Off = this->allocateLocalPrimitive(VD, *T, DT.isConstQualified());
+    auto Offset =
+        this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:256
+    return this->emitSetLocal(*T, Offset, VD);
   } else {
     // Composite types - allocate storage and initialize it.
----------------
You can drop this `else` because of the preceding unconditional `return`.


================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:258
     // Composite types - allocate storage and initialize it.
-    if (auto Off = this->allocateLocal(VD)) {
-      return this->visitLocalInitializer(VD->getInit(), *Off);
-    } else {
+    if (auto Offset = this->allocateLocal(VD))
+      return this->visitLocalInitializer(VD->getInit(), *Offset);
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:260
+      return this->visitLocalInitializer(VD->getInit(), *Offset);
+    else
       return this->bail(VD);
----------------
You should drop this `else` as well.


================
Comment at: clang/lib/AST/Interp/Pointer.cpp:158
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
     if (!Pointee->IsStatic) {
----------------
`isArray()` already covers `isPrimitiveArray()` so this change looks unnecessary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132727/new/

https://reviews.llvm.org/D132727