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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder marked 3 inline comments as done.
tbaeder added inline comments.


================
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);
----------------
aaron.ballman wrote:
> 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.
I did a quick test with `_Atomic` and that seems to work.


================
Comment at: clang/test/AST/Interp/cxx20.cpp:119
+    A a;
+    constexpr C2() {} // expected-note {{subobject of type 'int' is not initialized}}
+  };
----------------
aaron.ballman wrote:
> 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.
I thought this might even be an improvement over the current situation. It's followed by the "subobject declared here" note as well. With the current interpreter, the output is:

```
array.cpp:95:16: error: constexpr variable 'c2' must be initialized by a constant expression
  constexpr C2 c2; // expected-error {{must be initialized by a constant expression}} \
               ^~
array.cpp:95:16: note: subobject of type 'int' is not initialized
array.cpp:88:18: note: subobject declared here
  struct A { int a; };
                 ^
```

And with the new one:
```
array.cpp:95:16: error: constexpr variable 'c2' must be initialized by a constant expression
  constexpr C2 c2; // expected-error {{must be initialized by a constant expression}} \
               ^~
array.cpp:93:15: note: subobject of type 'int' is not initialized
    constexpr C2() {} // expected-note {{subobject of type 'int' is not initialized}}
              ^
array.cpp:95:16: note: in call to 'C2()'
  constexpr C2 c2; // expected-error {{must be initialized by a constant expression}} \
               ^
array.cpp:88:18: note: subobject declared here
  struct A { int a; };
                 ^
```

But I dislike both very much, no idea why there's no "subobject declared here is not initialized" diagnostic and we need two notes instead :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136828