[PATCH] D147349: [C2x] Implement support for empty brace initialization (WG14 N2900 and WG14 N3011)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So not necessary to this patch, but I suspect the non-trivially constructible types could just be solved the same way we do for zero-init of these types in fixed-size arrays, right?  Its a bit more complicated, but the IR is probably just as do-able: https://godbolt.org/z/Gqf65xr8M
There, in the in it of %6 is the only place the length matters, since it looks like we do a 'begin/end' type emit for it.  %2 is kept as the 'current element being processed', and %6 is the 'end', and %5 is the 'begin'.

That said, only a few small comments/questions.



================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1670
+    // in C++, we can safely memset the array memory to zero.
+    CGF.EmitNullInitialization(Dest.getAddress(), ExprToVisit->getType());
+    return;
----------------
I'd probably prefer an assert for 'initializer is empty' here to confirm this, but else this seems fine.


================
Comment at: clang/test/C/C2x/n2900_n3011_2.c:41
+void test_zero_size_array() {
+  int unknown_size[] = {};
+  // CHECK: define {{.*}} void @test_zero_size_array
----------------
This is strange... why is this not an error?  What is this supposed to mean?


================
Comment at: clang/test/C/C2x/n2900_n3011_2.c:55
+  // CHECK-NEXT: store i32 12, ptr %[[I]]
+  // CHECK-NEXT: %[[MEM0:.+]] = load i32, ptr %[[I]]
+  // CHECK-NEXT: %[[MEM1:.+]] = zext i32 %[[MEM0]] to i64
----------------
These MEM# names are horrible to read :/  But the test is doing the right thing it appears.


================
Comment at: clang/test/C/C2x/n2900_n3011_2.c:96
+void test_nested_structs() {
+  struct T t1 = { 1, {} };
+  struct T t2 = { 1, { 2, {} } };
----------------
A VLA of these things still works right?  Is that worth a test?


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

https://reviews.llvm.org/D147349