Implement explicit field initialization checks in RecordDecl and make…

… it work in both C and C++

clang/include/clang/AST/CXXRecordDeclDefinitionBits.def

@@ -119,15 +119,6 @@ FIELD(HasInitMethod, 1, NO_MERGE)
 /// within anonymous unions or structs.
 FIELD(HasInClassInitializer, 1, NO_MERGE)
 
-/// Custom attribute that is True if any field is marked as requiring explicit
-/// initialization with [[clang::requires_explicit_initialization]] in a type
-/// without a user-provided default constructor, or if this is the case for any
-/// base classes and/or member variables whose types are aggregates.
-///
-/// In this case, default-construction is diagnosed, as it would not explicitly
-/// initialize the field.
-FIELD(HasUninitializedExplicitInitFields, 1, NO_MERGE)
-
 /// True if any field is of reference type, and does not have an
 /// in-class initializer.
 ///

clang/include/clang/AST/Decl.h

@@ -4269,6 +4269,14 @@ class RecordDecl : public TagDecl {
     RecordDeclBits.HasNonTrivialToPrimitiveCopyCUnion = V;
   }
 
+  bool hasUninitializedExplicitInitFields() const {
+    return RecordDeclBits.HasUninitializedExplicitInitFields;
+  }
+
+  void setHasUninitializedExplicitInitFields(bool V) {
+    RecordDeclBits.HasUninitializedExplicitInitFields = V;
+  }
+
   /// Determine whether this class can be passed in registers. In C++ mode,
   /// it must have at least one trivial, non-deleted copy or move constructor.
   /// FIXME: This should be set as part of completeDefinition.

clang/include/clang/AST/DeclBase.h

@@ -1668,6 +1668,14 @@ class DeclContext {
     LLVM_PREFERRED_TYPE(bool)
     uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
 
+    /// True if any field is marked as requiring explicit initialization with
+    /// [[clang::requires_explicit_initialization]].
+    /// In C++, this is also set for types without a user-provided default
+    /// constructor, and is propagated from any base classes and/or member
+    /// variables whose types are aggregates.
+    LLVM_PREFERRED_TYPE(bool)
+    uint64_t HasUninitializedExplicitInitFields : 1;
+
     /// Indicates whether this struct is destroyed in the callee.
     LLVM_PREFERRED_TYPE(bool)
     uint64_t ParamDestroyedInCallee : 1;
@@ -1682,7 +1690,7 @@ class DeclContext {
 
     /// True if a valid hash is stored in ODRHash. This should shave off some
     /// extra storage and prevent CXXRecordDecl to store unused bits.
-    uint64_t ODRHash : 26;
+    uint64_t ODRHash : 25;
   };
 
   /// Number of inherited and non-inherited bits in RecordDeclBitfields.

clang/include/clang/AST/DeclCXX.h

@@ -1152,11 +1152,6 @@ class CXXRecordDecl : public RecordDecl {
   /// structs).
   bool hasInClassInitializer() const { return data().HasInClassInitializer; }
 
-  bool hasUninitializedExplicitInitFields() const {
-    return !isUnion() && !hasUserProvidedDefaultConstructor() &&
-           data().HasUninitializedExplicitInitFields;
-  }
-
   /// Whether this class or any of its subobjects has any members of
   /// reference type which would make value-initialization ill-formed.
   ///

clang/include/clang/Basic/Attr.td

@@ -1862,10 +1862,9 @@ def Leaf : InheritableAttr {
 }
 
 def ExplicitInit : InheritableAttr {
-  let Spellings = [Clang<"requires_explicit_initialization", 0>];
+  let Spellings = [Clang<"requires_explicit_initialization", 1>];
   let Subjects = SubjectList<[Field], ErrorDiag>;
   let Documentation = [ExplicitInitDocs];
-  let LangOpts = [CPlusPlus];
   let SimpleHandler = 1;
 }
 

clang/include/clang/Basic/AttrDocs.td

@@ -1426,24 +1426,31 @@ The ``clang::requires_explicit_initialization`` attribute indicates that the
 field of an aggregate must be initialized explicitly by users when the class
 is constructed. Its usage is invalid on non-aggregates.
 
-Example usage:
+Note that this attribute is *not* a memory safety feature, and is *not* intended
+to guard against use of uninitialized memory.
+
+Rather, it is intended for use in "parameter-objects", used to simulate the
+passing of named parameters.
+The attribute generates a warning when explicit initializers for such
+"named parameters" are not provided:
 
 .. code-block:: c++
 
-  struct some_aggregate {
-    int x;
-    int y [[clang::requires_explicit_initialization]];
+  struct ArrayIOParams {
+    size_t count [[clang::requires_explicit_initialization]];
+    size_t element_size [[clang::requires_explicit_initialization]];
+    int flags = 0;
   };
 
-  some_aggregate create() {
-    return {.x = 1};  // error: y is not initialized explicitly
-  }
+  size_t ReadArray(FILE *file, void *buffer, ArrayIOParams params);
 
-This attribute is *not* a memory safety feature, and is *not* intended to guard
-against use of uninitialized memory.
-Rather, its intended use is in structs that represent "parameter objects", to
-allow extending them while ensuring that callers do not forget to specify
-values for newly added fields ("parameters").
+  int main() {
+    unsigned int buf[512];
+    ReadArray(stdin, buf, {
+      .count = sizeof(buf) / sizeof(*buf),
+      // warning: field 'element_size' is not explicitly initialized
+    });
+  }
 
   }];
 }

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2336,8 +2336,8 @@ def err_init_reference_member_uninitialized : Error<
 def note_uninit_reference_member : Note<
   "uninitialized reference member is here">;
 def warn_field_requires_explicit_init : Warning<
-  "field %0 is not explicitly initialized, but was marked as requiring "
-  "explicit initialization">, InGroup<UninitializedExplicitInit>;
+  "field %select{%1|in %1}0 is not explicitly initialized, but was marked as "
+  "requiring explicit initialization">, InGroup<UninitializedExplicitInit>;
 def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">,
   InGroup<Uninitialized>;
 def warn_base_class_is_uninit : Warning<
@@ -3148,6 +3148,10 @@ def warn_attribute_ignored_no_calls_in_stmt: Warning<
   "statement">,
   InGroup<IgnoredAttributes>;
 
+def warn_attribute_needs_aggregate : Warning<
+  "%0 attribute is ignored in non-aggregate type %1">,
+  InGroup<IgnoredAttributes>;
+
 def warn_attribute_ignored_non_function_pointer: Warning<
   "%0 attribute is ignored because %1 is not a function pointer">,
   InGroup<IgnoredAttributes>;

clang/lib/AST/Decl.cpp

@@ -5014,6 +5014,7 @@ RecordDecl::RecordDecl(Kind DK, TagKind TK, const ASTContext &C,
   setHasNonTrivialToPrimitiveDefaultInitializeCUnion(false);
   setHasNonTrivialToPrimitiveDestructCUnion(false);
   setHasNonTrivialToPrimitiveCopyCUnion(false);
+  setHasUninitializedExplicitInitFields(false);
   setParamDestroyedInCallee(false);
   setArgPassingRestrictions(RecordArgPassingKind::CanPassInRegs);
   setIsRandomized(false);

clang/lib/AST/DeclCXX.cpp

@@ -82,7 +82,6 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       HasPrivateFields(false), HasProtectedFields(false),
       HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false),
       HasOnlyCMembers(true), HasInitMethod(false), HasInClassInitializer(false),
-      HasUninitializedExplicitInitFields(false),
       HasUninitializedReferenceMember(false), HasUninitializedFields(false),
       HasInheritedConstructor(false), HasInheritedDefaultConstructor(false),
       HasInheritedAssignment(false),
@@ -460,6 +459,10 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
     if (BaseClassDecl->hasMutableFields())
       data().HasMutableFields = true;
 
+    if (BaseClassDecl->hasUninitializedExplicitInitFields() &&
+        BaseClassDecl->isAggregate())
+      setHasUninitializedExplicitInitFields(true);
+
     if (BaseClassDecl->hasUninitializedReferenceMember())
       data().HasUninitializedReferenceMember = true;
 
@@ -1114,7 +1117,7 @@ void CXXRecordDecl::addedMember(Decl *D) {
       data().PlainOldData = false;
 
     if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) {
-      data().HasUninitializedExplicitInitFields = true;
+      setHasUninitializedExplicitInitFields(true);
     }
 
     if (T->isReferenceType()) {
@@ -1370,7 +1373,7 @@ void CXXRecordDecl::addedMember(Decl *D) {
 
         if (FieldRec->hasUninitializedExplicitInitFields() &&
             FieldRec->isAggregate() && !Field->hasInClassInitializer())
-          data().HasUninitializedExplicitInitFields = true;
+          setHasUninitializedExplicitInitFields(true);
 
         if (FieldRec->hasUninitializedReferenceMember() &&
             !Field->hasInClassInitializer())
@@ -2160,17 +2163,32 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
     I.setAccess((*I)->getAccess());
 
   ASTContext &Context = getASTContext();
-  if (!Context.getLangOpts().CPlusPlus20 && isAggregate() &&
-      hasUserDeclaredConstructor()) {
+
+  if (isAggregate() && hasUserDeclaredConstructor() &&
+      !Context.getLangOpts().CPlusPlus20) {
     // Diagnose any aggregate behavior changes in C++20
     for (field_iterator I = field_begin(), E = field_end(); I != E; ++I) {
       if (const auto *attr = I->getAttr<ExplicitInitAttr>()) {
         Context.getDiagnostics().Report(
-            getLocation(),
+            attr->getLocation(),
             diag::warn_cxx20_compat_requires_explicit_init_non_aggregate)
-            << attr->getRange() << Context.getRecordType(this);
+            << attr << Context.getRecordType(this);
+      }
+    }
+  }
+
+  if (!isAggregate() && hasUninitializedExplicitInitFields()) {
+    // Diagnose any fields that required explicit initialization in a
+    // non-aggregate type. (Note that the fields may not be directly in this
+    // type, but in a subobject. In such cases we don't emit diagnoses here.)
+    for (field_iterator I = field_begin(), E = field_end(); I != E; ++I) {
+      if (const auto *attr = I->getAttr<ExplicitInitAttr>()) {
+        Context.getDiagnostics().Report(attr->getLocation(),
+                                        diag::warn_attribute_needs_aggregate)
+            << attr << Context.getRecordType(this);
       }
     }
+    setHasUninitializedExplicitInitFields(false);
   }
 }
 

clang/lib/Sema/SemaDecl.cpp

@@ -19086,6 +19086,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
         if (FT.hasNonTrivialToPrimitiveCopyCUnion() || Record->isUnion())
           Record->setHasNonTrivialToPrimitiveCopyCUnion(true);
       }
+      if (FD->hasAttr<ExplicitInitAttr>()) {
+        Record->setHasUninitializedExplicitInitFields(true);
+      }
       if (FT.isDestructedType()) {
         Record->setNonTrivialToPrimitiveDestroy(true);
         Record->setParamDestroyedInCallee(true);

clang/lib/Sema/SemaInit.cpp

@@ -746,7 +746,7 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
 
     if (!VerifyOnly && Field->hasAttr<ExplicitInitAttr>()) {
       SemaRef.Diag(ILE->getExprLoc(), diag::warn_field_requires_explicit_init)
-          << Field;
+          << /* Var-in-Record */ 0 << Field;
       SemaRef.Diag(Field->getLocation(), diag::note_entity_declared_at)
           << Field;
     }
@@ -4573,7 +4573,7 @@ static void TryConstructorInitialization(Sema &S,
         DestRecordDecl != nullptr && DestRecordDecl->isAggregate() &&
         DestRecordDecl->hasUninitializedExplicitInitFields()) {
       S.Diag(Kind.getLocation(), diag::warn_field_requires_explicit_init)
-          << "in class";
+          << /* Var-in-Record */ 1 << DestRecordDecl;
     }
 
     // C++11 [dcl.init]p6:
@@ -6472,6 +6472,18 @@ void InitializationSequence::InitializeFrom(Sema &S,
     }
   }
 
+  if (!S.getLangOpts().CPlusPlus &&
+      Kind.getKind() == InitializationKind::IK_Default) {
+    RecordDecl *Rec = DestType->getAsRecordDecl();
+    if (Rec && Rec->hasUninitializedExplicitInitFields()) {
+      VarDecl *Var = dyn_cast_or_null<VarDecl>(Entity.getDecl());
+      if (Var && !Initializer) {
+        S.Diag(Var->getLocation(), diag::warn_field_requires_explicit_init)
+            << /* Var-in-Record */ 1 << Rec;
+      }
+    }
+  }
+
   //     - If the destination type is a reference type, see 8.5.3.
   if (DestType->isReferenceType()) {
     // C++0x [dcl.init.ref]p1:

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -859,6 +859,7 @@ RedeclarableResult ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
       RecordDeclBits.getNextBit());
   RD->setHasNonTrivialToPrimitiveDestructCUnion(RecordDeclBits.getNextBit());
   RD->setHasNonTrivialToPrimitiveCopyCUnion(RecordDeclBits.getNextBit());
+  RD->setHasUninitializedExplicitInitFields(RecordDeclBits.getNextBit());
   RD->setParamDestroyedInCallee(RecordDeclBits.getNextBit());
   RD->setArgPassingRestrictions(
       (RecordArgPassingKind)RecordDeclBits.getNextBits(/*Width=*/2));

clang/test/Misc/pragma-attribute-supported-attributes-list.test

@@ -77,7 +77,6 @@
 // CHECK-NEXT: EnumExtensibility (SubjectMatchRule_enum)
 // CHECK-NEXT: Error (SubjectMatchRule_function)
 // CHECK-NEXT: ExcludeFromExplicitInstantiation (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
-// CHECK-NEXT: ExplicitInit (SubjectMatchRule_field)
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)

clang/test/Sema/uninit-variables.c

@@ -551,3 +551,14 @@ struct full_of_empty empty_test_2(void) {
   struct full_of_empty e;
   return e; // no-warning
 }
+
+struct with_explicit_field {
+  int x;
+  int y [[clang::requires_explicit_initialization]]; // expected-note {{declared}}
+};
+
+void aggregate() {
+  struct with_explicit_field a; // expected-warning {{is not explicitly initialized}}
+  struct with_explicit_field b = {1}; // expected-warning {{is not explicitly initialized}}
+  (void)(&a != &b);
+}

clang/test/SemaCXX/uninitialized.cpp

@@ -1474,17 +1474,24 @@ template<typename T> struct Outer {
 Outer<int>::Inner outerinner;
 
 void aggregate() {
+  struct NonAgg {
+    NonAgg() { }
+    [[clang::requires_explicit_initialization]] int f;  // expected-warning {{attribute is ignored}}
+  };
+  NonAgg nonagg;
+  (void)nonagg;
+
   struct S {
-    [[clang::requires_explicit_initialization]] int x; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}}
+    [[clang::requires_explicit_initialization]] int x; // expected-note 4{{declared here}}
     int y;
     int z = 12;
-    [[clang::requires_explicit_initialization]] int q = 100; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}}
+    [[clang::requires_explicit_initialization]] int q = 100; // expected-note 4{{declared}}
     static void foo(S) { }
   };
 
-  struct D : S { // expected-warning {{not explicitly initialized}}
+  struct D : S { // expected-warning {{field in 'S' is not explicitly initialized, but was marked as requiring explicit initialization}}
     int f1;
-    int f2 [[clang::requires_explicit_initialization]]; // expected-note {{declared}} // expected-note {{declared}}
+    int f2 [[clang::requires_explicit_initialization]]; // expected-note 2{{declared}}
   };
 
   struct C {
@@ -1494,27 +1501,27 @@ void aggregate() {
 
   S::foo(S{1, 2, 3, 4});
   S::foo(S{.x = 100, .q = 100});
-  S::foo(S{.x = 100}); // expected-warning {{'q' is not explicitly initialized}}
+  S::foo(S{.x = 100}); // expected-warning {{field 'q' is not explicitly initialized, but was marked as requiring explicit initialization}}
   S s{.x = 100, .q = 100};
   (void)s;
-  S t{.q = 100}; // expected-warning {{'x' is not explicitly initialized}}
+  S t{.q = 100}; // expected-warning {{field 'x' is not explicitly initialized, but was marked as requiring explicit initialization}}
   (void)t;
-  S *ptr1 = new S; // expected-warning {{not explicitly initialized}}
+  S *ptr1 = new S; // expected-warning {{field in 'S' is not explicitly initialized, but was marked as requiring explicit initialization}}
   delete ptr1;
   S *ptr2 = new S{.x = 100, .q = 100};
   delete ptr2;
 #if __cplusplus >= 202002L
-  D a({}, 0); // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'f2' is not explicitly initialized}}
+  D a({}, 0); // expected-warning {{field 'x' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'f2' is not explicitly initialized, but was marked as requiring explicit initialization}}
   (void)a;
 #else
-  C a; // expected-warning {{not explicitly initialized}}
+  C a; // expected-warning {{field in 'C' is not explicitly initialized, but was marked as requiring explicit initialization}}
   (void)a;
 #endif
-  D b{.f2 = 1}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'q' is not explicitly initialized}}
+  D b{.f2 = 1}; // expected-warning {{field 'x' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'q' is not explicitly initialized, but was marked as requiring explicit initialization}}
   (void)b;
-  D c{.f1 = 5}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'q' is not explicitly initialized}} expected-warning {{'f2' is not explicitly initialized}}
-  c = {{}, 0}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'q' is not explicitly initialized}} expected-warning {{'f2' is not explicitly initialized}}
+  D c{.f1 = 5}; // expected-warning {{field 'x' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'q' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'f2' is not explicitly initialized, but was marked as requiring explicit initialization}}
+  c = {{}, 0}; // expected-warning {{field 'x' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'q' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-warning {{field 'f2' is not explicitly initialized, but was marked as requiring explicit initialization}}
   (void)c;
-  D d; // expected-warning {{not explicitly initialized}}  // expected-note {{constructor}}
+  D d; // expected-warning {{field in 'D' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note {{in implicit default constructor for 'D' first required here}}
   (void)d;
 }