[PATCH] D136828: [clang][Interp] Diagnose uninitialized array record fields

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/Interp.cpp:446-448
+static void DiagnoseUninitializedSubobject(InterpState &S, const SourceInfo &SI,
+                                           QualType SubObjType,
+                                           SourceLocation SL) {
----------------
Some comments describing what `SL` does would be helpful, since it's a bit special.


================
Comment at: clang/lib/AST/Interp/Interp.cpp:464
+
+  if (auto RT = ElemType->getAs<RecordType>()) {
+    const Record *R = BasePtr.getElemRecord();
----------------



================
Comment at: clang/lib/AST/Interp/Interp.cpp:467-472
+      const Pointer ElemPtr = BasePtr.atIndex(I).narrow();
+      Result &= CheckFieldsInitialized(S, OpPC, ElemPtr, R);
+    }
+  } else if (auto *ElemCAT = dyn_cast<ConstantArrayType>(ElemType)) {
+    for (size_t I = 0; I != NumElems; ++I) {
+      const Pointer ElemPtr = BasePtr.atIndex(I).narrow();
----------------



================
Comment at: clang/lib/AST/Interp/Interp.cpp:501
     } else if (FieldType->isArrayType()) {
-      // FIXME: Arrays need to be handled here as well I think.
+      const ArrayType *AT = FieldType->getAsArrayTypeUnsafe();
+      assert(AT);
----------------
Do element qualifiers matter for checking initialization? (I don't think they do, but double-checking to be sure there's not something special for atomics or something like that.)


================
Comment at: clang/lib/AST/Interp/Interp.cpp:501-503
+      const ArrayType *AT = FieldType->getAsArrayTypeUnsafe();
+      assert(AT);
+      const auto *CAT = cast<ConstantArrayType>(AT);
----------------
aaron.ballman wrote:
> Do element qualifiers matter for checking initialization? (I don't think they do, but double-checking to be sure there's not something special for atomics or something like that.)
`cast<>` already asserts if given a null pointer.


================
Comment at: clang/test/AST/Interp/cxx20.cpp:119
+    A a;
+    constexpr C2() {} // expected-note {{subobject of type 'int' is not initialized}}
+  };
----------------
This note is kind of confusing to me. At this location, it's not a subobject of type `int` that matters, it's `A` that's not fully initialized, and within `A` the note points out which field is not initialized.

I think this could get especially confusing in a case like:
```
class C {
public:
  A a;
  int b = 0;

  constexpr C() {}
}
```
because we'll talk about `int` not being initialized and it will look very much like it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136828