[clang][Interp] Array initialization via ImplicitValueInitExpr

[tbaeder]

Summary

Unfortunately, I have no idea how to test this, the one and only reproducer I have so far is:

#include <array>
constexpr std::array<int, 10> IntArray = {};

which results in a InitListExpr containing a ImplicitValueInitExpr with an array type.

[tbaeder]

Ping

[shafik]

The following also generates an ImplicitValueInitExpr:

template <typename T, unsigned N>
struct B {
  T a[N];
};

int main() {
   constexpr B<int,10> arr = {};
   constexpr int x = arr.a[0];
}

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

748

I believe you should check ArrayType before you use it.

759

I guess this covers both arrays and class types?

[tbaeder]

Thanks for the reproducer!

clang/lib/AST/Interp/ByteCodeExprGen.cpp

759

Yes. But now that I look at it, the check should probably be outside of the loop.

[shafik]

I found a way to get ImplictValueInitExpr for a record: https://godbolt.org/z/Eqf9Gz1G4

I am not sure if we can achieve the same in C++ b/c value init of a class break down to default init or zero init.

[tbaeder]

In D135013#3860317, @shafik wrote:

I found a way to get ImplictValueInitExpr for a record: https://godbolt.org/z/Eqf9Gz1G4

I am not sure if we can achieve the same in C++ b/c value init of a class break down to default init or zero init.

Does clang generate a default constructor in this case anyway, which the interpreter could use in this case? Or is it support to just memset(0) the entire struct instance?

[tbaeder]

Ah it's the array filler which we don't handle at all right now :(

[tbaeder]

@aaron.ballman Do you maybe have a different reproducer or know more about the expected behavior of a ImplicitValueInitExpr for array and record types?

[aaron.ballman]

In D135013#3870894, @tbaeder wrote:

@aaron.ballman Do you maybe have a different reproducer or know more about the expected behavior of a ImplicitValueInitExpr for array and record types?

So here's another reproducer with a record type: https://godbolt.org/z/EhT4oqT3s and here's one with an array type: https://godbolt.org/z/Pbncnq418

My understanding of ImplicitValueInitExpr is that it's used to represent some of the "holes" in the middle of an array/record that need to implicit get some values.

[tbaeder]

In D135013#3871209, @aaron.ballman wrote:

In D135013#3870894, @tbaeder wrote:

@aaron.ballman Do you maybe have a different reproducer or know more about the expected behavior of a ImplicitValueInitExpr for array and record types?

So here's another reproducer with a record type: https://godbolt.org/z/EhT4oqT3s and here's one with an array type: https://godbolt.org/z/Pbncnq418

My understanding of ImplicitValueInitExpr is that it's used to represent some of the "holes" in the middle of an array/record that need to implicit get some values.

They are inside a struct or array, but not of array or struct type, if I understand correctly. But I see what you're getting at, I guess this should basically go through visitZeroInitializer() (which is currently unused), which should be extended to handle structs and arrays.

[aaron.ballman]

In D135013#3871250, @tbaeder wrote:

In D135013#3871209, @aaron.ballman wrote:

In D135013#3870894, @tbaeder wrote:

@aaron.ballman Do you maybe have a different reproducer or know more about the expected behavior of a ImplicitValueInitExpr for array and record types?

So here's another reproducer with a record type: https://godbolt.org/z/EhT4oqT3s and here's one with an array type: https://godbolt.org/z/Pbncnq418

My understanding of ImplicitValueInitExpr is that it's used to represent some of the "holes" in the middle of an array/record that need to implicit get some values.

They are inside a struct or array, but not of array or struct type, if I understand correctly. But I see what you're getting at, I guess this should basically go through visitZeroInitializer() (which is currently unused), which should be extended to handle structs and arrays.

Ah, yes! That's true, they are inside the struct or array. I'm not certain you can have one *of* struct type in C++ because it's either going to be zeroed or left uninitialized (due to constructors).

[tbaeder]

Thinking about it some more, doesn't make much sense to add all that right now when we don't have a reproducer to test it anyway. I think the patch as it is makes sense and it it's not enough in the future, there's a proper assert in place.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

726

Please spell out the type.

728

Should this be using cast<> as the code below is assuming this pointer will never be null?

729

[aaron.ballman]

Aside from a naming quirk, LGTM

clang/lib/AST/Interp/ByteCodeExprGen.cpp

726

(Sorry, I hadn't noticed you were using a type name as a variable name before! That sometimes confuses IDEs, so suggesting a different name.)

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

726

I can't say I saw that when making the change, but now that I see it, yes, that might've been the reason I used auto here. Anyway, I changed that, thanks.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

for (size_t I = 0; I != Size; ++I) {
         if (!visitInitializer(SubExpr))
           return false;
       }
 
       if (!this->emitPopPtr(Initializer))
         return false;
     }
     return true;
+  } else if (const auto *IVIE = dyn_cast<ImplicitValueInitExpr>(Initializer)) {
+    auto ArrayType = IVIE->getType()->getAsArrayTypeUnsafe();
+    assert(ArrayType);
+    const auto *CAT = dyn_cast<ConstantArrayType>(ArrayType);
+    const size_t NumElems = CAT->getSize().getZExtValue();
+
+    if (Optional<PrimType> ElemT = classify(CAT->getElementType())) {
+      // TODO(perf): For int and bool types, we can probably just skip this
+      //   since we memset our Block*s to 0 and so we have the desired value
+      //   without this.
+      for (size_t I = 0; I != NumElems; ++I) {
+        if (!this->emitZero(*ElemT, Initializer))
+          return false;
+        if (!this->emitInitElem(*ElemT, I, Initializer))
+          return false;
+      }
+    } else {
+      assert(false && "default initializer for non-primitive type");
+    }
+
+    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 || !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;
 

clang/test/AST/Interp/arrays.cpp

constexpr int secondValue = second[10];// ref-error {{must be initialized by a constant expression}} \
                                          // expected-error {{must be initialized by a constant expression}} \
                                          // expected-note {{read of dereferenced one-past-the-end pointer}}
 
   constexpr int negative = second[-2]; // ref-error {{must be initialized by a constant expression}} \
                                        // ref-note {{cannot refer to element -2 of array of 10}} \
                                        // expected-error {{must be initialized by a constant expression}} \
                                        // expected-note {{cannot refer to element -2 of array of 10}}
 };
+
+namespace DefaultInit {
+  template <typename T, unsigned N>
+  struct B {
+    T a[N];
+  };
+
+  int f() {
+     constexpr B<int,10> arr = {};
+     constexpr int x = arr.a[0];
+  }
+};