[clang][Interp] Diagnose uninitialized array record fields

[tbaeder]

Summary

As a continuation of https://reviews.llvm.org/D136694.

This required some fixes to the InitMap handling, i.e. https://reviews.llvm.org/D136826.

[aaron.ballman]

clang/lib/AST/Interp/Interp.cpp

410–412

Some comments describing what SL does would be helpful, since it's a bit special.

428

431–436

464–465

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.)

464–466

cast<> already asserts if given a null pointer.

clang/test/AST/Interp/cxx20.cpp

119

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.

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

464–465

I did a quick test with _Atomic and that seems to work.

clang/test/AST/Interp/cxx20.cpp

119

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 :(

[aaron.ballman]

Aside from the request for a comment about SourceLocation SL, this LGTM (though we may want to improve the diagnostic behavior in the future).

clang/test/AST/Interp/cxx20.cpp

119

I think we can live with the behavior for now and improve the diagnostics once we've got more basic functionality in place.

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

410–412

Renamed it to SubObjLoc instead of adding a comment, I think that's enough.

clang/lib/AST/Interp/Interp.cpp

bool CheckPure(InterpState &S, CodePtr OpPC, const CXXMethodDecl *MD) {
   if (!MD->isPure())
     return true;
   const SourceInfo &E = S.Current->getSource(OpPC);
   S.FFDiag(E, diag::note_constexpr_pure_virtual_call, 1) << MD;
   S.Note(MD->getLocation(), diag::note_declared_at);
   return false;
 }
 
+static void DiagnoseUninitializedSubobject(InterpState &S, const SourceInfo &SI,
+                                           QualType SubObjType,
+                                           SourceLocation SubObjLoc) {
+  S.FFDiag(SI, diag::note_constexpr_uninitialized) << true << SubObjType;
+  if (SubObjLoc.isValid())
+    S.Note(SubObjLoc, diag::note_constexpr_subobject_declared_here);
+}
+
+static bool CheckFieldsInitialized(InterpState &S, CodePtr OpPC,
+                                   const Pointer &BasePtr, const Record *R);
+
+static bool CheckArrayInitialized(InterpState &S, CodePtr OpPC,
+                                  const Pointer &BasePtr,
+                                  const ConstantArrayType *CAT) {
+  bool Result = true;
+  size_t NumElems = CAT->getSize().getZExtValue();
+  QualType ElemType = CAT->getElementType();
+
+  if (isa<RecordType>(ElemType.getTypePtr())) {
+    const Record *R = BasePtr.getElemRecord();
+    for (size_t I = 0; I != NumElems; ++I) {
+      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) {
+      Pointer ElemPtr = BasePtr.atIndex(I).narrow();
+      Result &= CheckArrayInitialized(S, OpPC, ElemPtr, ElemCAT);
+    }
+  } else {
+    for (size_t I = 0; I != NumElems; ++I) {
+      if (!BasePtr.atIndex(I).isInitialized()) {
+        DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC), ElemType,
+                                       BasePtr.getFieldDesc()->getLocation());
+        Result = false;
+      }
+    }
+  }
+
+  return Result;
+}
+
 static bool CheckFieldsInitialized(InterpState &S, CodePtr OpPC,
                                    const Pointer &BasePtr, const Record *R) {
   assert(R);
   bool Result = true;
   // Check all fields of this record are initialized.
   for (const Record::Field &F : R->fields()) {
     Pointer FieldPtr = BasePtr.atField(F.Offset);
-    QualType FieldType = FieldPtr.getType();
+    QualType FieldType = F.Decl->getType();
 
     if (FieldType->isRecordType()) {
       Result &= CheckFieldsInitialized(S, OpPC, FieldPtr, FieldPtr.getRecord());
     } else if (FieldType->isArrayType()) {
-      // FIXME: Arrays need to be handled here as well I think.
+      const auto *CAT = cast<ConstantArrayType>(FieldType->getAsArrayTypeUnsafe());
+      Result &= CheckArrayInitialized(S, OpPC, FieldPtr, CAT);
     } else if (!FieldPtr.isInitialized()) {
-      const SourceInfo &SI = S.Current->getSource(OpPC);
-      S.FFDiag(SI, diag::note_constexpr_uninitialized)
-          << true << F.Decl->getType();
-      SourceLocation SubobjectLoc = F.Decl->getLocation();
-      if (SubobjectLoc.isValid())
-        S.Note(SubobjectLoc, diag::note_constexpr_subobject_declared_here);
+      DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC),
+                                     F.Decl->getType(), F.Decl->getLocation());
       Result = false;
     }
   }
   return Result;
 }
 
 bool CheckCtorCall(InterpState &S, CodePtr OpPC, const Pointer &This) {
   assert(!This.isZero());

clang/lib/AST/Interp/Pointer.h

public:
   bool isArrayElement() const { return Base != Offset; }
   /// Pointer points directly to a block.
   bool isRoot() const {
     return (Base == 0 || Base == RootPtrMark) && Offset == 0;
   }
 
   /// Returns the record descriptor of a class.
   Record *getRecord() const { return getFieldDesc()->ElemRecord; }
+  // Returns the element record type, if this is a non-primive array.
+  Record *getElemRecord() const { return getFieldDesc()->ElemDesc->ElemRecord; }
   /// Returns the field information.
   const FieldDecl *getField() const { return getFieldDesc()->asFieldDecl(); }
 
   /// Checks if the object is a union.
   bool isUnion() const;
 
   /// Checks if the storage is extern.
   bool isExtern() const { return Pointee->isExtern(); }

clang/test/AST/Interp/cxx20.cpp

 }
 static_assert(f());
 #endif
 
 
 namespace UninitializedFields {
   class A {
   public:
-    int a; // expected-note {{subobject declared here}} \
-           // ref-note {{subobject declared here}}
+    int a; // expected-note 2{{subobject declared here}} \
+           // ref-note 2{{subobject declared here}}
     constexpr A() {}
   };
   constexpr A a; // expected-error {{must be initialized by a constant expression}} \
                  // expected-note {{subobject of type 'int' is not initialized}} \
                  // ref-error {{must be initialized by a constant expression}} \
                  // ref-note {{subobject of type 'int' is not initialized}}
 
 
namespace UninitializedFields {
   public:
     constexpr Derived() : Base() {} // expected-note {{subobject of type 'int' is not initialized}}
   };
 
 constexpr Derived D; // expected-error {{must be initialized by a constant expression}} \\
                      // expected-note {{in call to 'Derived()'}} \
                      // ref-error {{must be initialized by a constant expression}} \
                      // ref-note {{subobject of type 'int' is not initialized}}
+
+  class C2 {
+  public:
+    A a;
+    constexpr C2() {} // expected-note {{subobject of type 'int' is not initialized}}
+  };
+  constexpr C2 c2; // expected-error {{must be initialized by a constant expression}} \
+                   // expected-note {{in call to 'C2()'}} \
+                   // ref-error {{must be initialized by a constant expression}} \
+                   // ref-note {{subobject of type 'int' is not initialized}}
+
+
+  // FIXME: These two are currently disabled because the array fields
+  //   cannot be initialized.
+#if 0
+  class C3 {
+  public:
+    A a[2];
+    constexpr C3() {}
+  };
+  constexpr C3 c3; // expected-error {{must be initialized by a constant expression}} \
+                   // expected-note {{subobject of type 'int' is not initialized}} \
+                   // ref-error {{must be initialized by a constant expression}} \
+                   // ref-note {{subobject of type 'int' is not initialized}}
+
+  class C4 {
+  public:
+    bool B[2][3]; // expected-note {{subobject declared here}} \
+                  // ref-note {{subobject declared here}}
+    constexpr C4(){}
+  };
+  constexpr C4 c4; // expected-error {{must be initialized by a constant expression}} \
+                   // expected-note {{subobject of type 'bool' is not initialized}} \
+                   // ref-error {{must be initialized by a constant expression}} \
+                   // ref-note {{subobject of type 'bool' is not initialized}}
+#endif
 };
 
 namespace ConstThis {
   class Foo {
     const int T = 12; // expected-note {{declared const here}} \
                       // ref-note {{declared const here}}
     int a;
   public: