[PATCH] D154189: [clang][Interp] Implement zero-init of record types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240
+  // Fields
+  for (const Record::Field &Field : R->fields()) {
+    const Descriptor *D = Field.Desc;
----------------
It looks like you're not initializing base classes or padding bits: http://eel.is/c++draft/dcl.init#general-6.2 -- we could use test coverage for both of those cases (don't forget to also test bit-fields). You should also have a test for zero init of unions.


================
Comment at: clang/lib/AST/Interp/Descriptor.cpp:281-289
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  QualType T = getType();
+
+  const auto *CAT = cast<ConstantArrayType>(T);
+  return CAT->getElementType();
+
----------------
This looks pretty dangerous -- is it safe to assume the type is a constant array, or should this be a `dyn_cast` and the spurious `return` is for the `else` branch?


================
Comment at: clang/test/AST/Interp/records.cpp:931
+    constexpr int foo(S x) {
+      return x.a; // expected-note {{read of object outside its lifetime}} \
+                  // ref-note {{read of uninitialized object}}
----------------
Why is `x.a` outside of its lifetime?


================
Comment at: clang/test/AST/Interp/records.cpp:939
+  };
+#endif
+
----------------
I think we should have more test coverage. I mentioned a few above, but other things to test would be inner classes (and anonymous members), like:
```
struct S {
  struct {
    int x, y;
  } v;
  union {
    float a;
    int b;
  };
};
```
and member pointers (because those aren't simple pointers):
```
struct A {
  void func();
};
struct S {
  void (A::*ptr)();
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154189