[clang] Add Clang attribute to ensure that fields are initialized explicitly (PR #102040)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 9 14:45:49 PDT 2024


https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/102040

>From 9335d802927dc6747ca12462744e1a9ca0b43949 Mon Sep 17 00:00:00 2001
From: higher-performance <higher.performance.github at gmail.com>
Date: Mon, 5 Aug 2024 15:04:19 -0400
Subject: [PATCH] Add Clang attribute to ensure that fields are initialized
 explicitly

---
 .../clang/AST/CXXRecordDeclDefinitionBits.def |  8 ++++
 clang/include/clang/AST/DeclCXX.h             |  5 ++
 clang/include/clang/Basic/Attr.td             |  8 ++++
 clang/include/clang/Basic/AttrDocs.td         | 29 ++++++++++++
 clang/include/clang/Basic/DiagnosticGroups.td |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td        |  7 +++
 clang/lib/AST/DeclCXX.cpp                     | 23 ++++++++++
 clang/lib/Sema/SemaDeclAttr.cpp               |  7 +++
 clang/lib/Sema/SemaInit.cpp                   | 13 ++++++
 ...a-attribute-supported-attributes-list.test |  1 +
 clang/test/SemaCXX/uninitialized.cpp          | 46 +++++++++++++++++++
 11 files changed, 148 insertions(+)

diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
index 6620840df0ced2..a6bb3c3458b850 100644
--- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -119,6 +119,14 @@ 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 explicit 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.
 ///
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 252e6e92564142..27994da80243d6 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1152,6 +1152,11 @@ 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.
   ///
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 9a7b163b2c6da8..4f730a31a0f615 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1861,6 +1861,14 @@ def Leaf : InheritableAttr {
   let SimpleHandler = 1;
 }
 
+def ExplicitInit : InheritableAttr {
+  let Spellings = [Clang<"requires_explicit_initialization", 0>];
+  let Subjects = SubjectList<[Field], ErrorDiag>;
+  let Documentation = [ExplicitInitDocs];
+  let LangOpts = [CPlusPlus];
+  let SimpleHandler = 1;
+}
+
 def LifetimeBound : DeclOrTypeAttr {
   let Spellings = [Clang<"lifetimebound", 0>];
   let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 546e5100b79dd9..625dff5b4b7aa9 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1419,6 +1419,35 @@ is not specified.
   }];
 }
 
+def ExplicitInitDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+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:
+
+.. code-block:: c++
+
+  struct some_aggregate {
+    int x;
+    int y [[clang::requires_explicit_initialization]];
+  };
+
+  some_aggregate create() {
+    return {.x = 1};  // error: y is not initialized explicitly
+  }
+
+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").
+
+  }];
+}
+
 def NoUniqueAddressDocs : Documentation {
   let Category = DocCatField;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 116ce7a04f66f7..3a8b13898e2cd0 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -787,6 +787,7 @@ def Trigraphs      : DiagGroup<"trigraphs">;
 def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">;
 def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">;
 def Unicode  : DiagGroup<"unicode">;
+def UninitializedExplicitInit : DiagGroup<"uninitialized-explicit-init">;
 def UninitializedMaybe : DiagGroup<"conditional-uninitialized">;
 def UninitializedSometimes : DiagGroup<"sometimes-uninitialized">;
 def UninitializedStaticSelfInit : DiagGroup<"static-self-init">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 58819a64813fce..6d38c546811bc7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2303,6 +2303,10 @@ def err_init_list_bad_dest_type : Error<
 def warn_cxx20_compat_aggregate_init_with_ctors : Warning<
   "aggregate initialization of type %0 with user-declared constructors "
   "is incompatible with C++20">, DefaultIgnore, InGroup<CXX20Compat>;
+def warn_cxx20_compat_requires_explicit_init_non_aggregate : Warning<
+  "explicit initialization of field %0 may not be enforced in C++20 as type %1 "
+  "will become a non-aggregate due to the presence of user-declared "
+  "constructors">, DefaultIgnore, InGroup<CXX20Compat>;
 def warn_cxx17_compat_aggregate_init_paren_list : Warning<
   "aggregate initialization of type %0 from a parenthesized list of values "
   "is a C++20 extension">, DefaultIgnore, InGroup<CXX20>;
@@ -2332,6 +2336,9 @@ def err_init_reference_member_uninitialized : Error<
   "reference member of type %0 uninitialized">;
 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>;
 def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">,
   InGroup<Uninitialized>;
 def warn_base_class_is_uninit : Warning<
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 01143391edab40..5f8e6e1fb0af9a 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -29,6 +29,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
@@ -81,6 +82,7 @@ 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),
@@ -1111,6 +1113,10 @@ void CXXRecordDecl::addedMember(Decl *D) {
     } else if (!T.isCXX98PODType(Context))
       data().PlainOldData = false;
 
+    if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) {
+      data().HasUninitializedExplicitInitFields = true;
+    }
+
     if (T->isReferenceType()) {
       if (!Field->hasInClassInitializer())
         data().HasUninitializedReferenceMember = true;
@@ -1362,6 +1368,10 @@ void CXXRecordDecl::addedMember(Decl *D) {
         if (!FieldRec->hasCopyAssignmentWithConstParam())
           data().ImplicitCopyAssignmentHasConstParam = false;
 
+        if (FieldRec->hasUninitializedExplicitInitFields() &&
+            FieldRec->isAggregate() && !Field->hasInClassInitializer())
+          data().HasUninitializedExplicitInitFields = true;
+
         if (FieldRec->hasUninitializedReferenceMember() &&
             !Field->hasInClassInitializer())
           data().HasUninitializedReferenceMember = true;
@@ -2148,6 +2158,19 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
   for (conversion_iterator I = conversion_begin(), E = conversion_end();
        I != E; ++I)
     I.setAccess((*I)->getAccess());
+
+  ASTContext &Context = getASTContext();
+  if (!Context.getLangOpts().CPlusPlus20 && hasUserDeclaredConstructor()) {
+    // 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(),
+            diag::warn_cxx20_compat_requires_explicit_init_non_aggregate)
+            << attr->getRange() << Context.getRecordType(this);
+      }
+    }
+  }
 }
 
 bool CXXRecordDecl::mayBeAbstract() const {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 72d82b424c26c8..ced4266f12d37a 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5949,6 +5949,10 @@ static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(NoMergeAttr::Create(S.Context, AL));
 }
 
+static void handleExplicitInitAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  D->addAttr(ExplicitInitAttr::Create(S.Context, AL));
+}
+
 static void handleNoUniqueAddressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(NoUniqueAddressAttr::Create(S.Context, AL));
 }
@@ -6854,6 +6858,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_NoMerge:
     handleNoMergeAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_ExplicitInit:
+    handleExplicitInitAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_NoUniqueAddress:
     handleNoUniqueAddressAttr(S, D, AL);
     break;
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 7dc17187524621..64b6b3e83452e0 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -743,6 +743,12 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
         ILE->updateInit(SemaRef.Context, Init, Filler);
       return;
     }
+
+    if (Field->hasAttr<ExplicitInitAttr>()) {
+      SemaRef.Diag(ILE->getExprLoc(), diag::warn_field_requires_explicit_init)
+          << Field;
+    }
+
     // C++1y [dcl.init.aggr]p7:
     //   If there are fewer initializer-clauses in the list than there are
     //   members in the aggregate, then each member not explicitly initialized
@@ -4548,6 +4554,13 @@ static void TryConstructorInitialization(Sema &S,
 
   CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
   if (Result != OR_Deleted) {
+    if (!IsListInit && Kind.getKind() == InitializationKind::IK_Default &&
+        DestRecordDecl != nullptr && DestRecordDecl->isAggregate() &&
+        DestRecordDecl->hasUninitializedExplicitInitFields()) {
+      S.Diag(Kind.getLocation(), diag::warn_field_requires_explicit_init)
+          << "in class";
+    }
+
     // C++11 [dcl.init]p6:
     //   If a program calls for the default initialization of an object
     //   of a const-qualified type T, T shall be a class type with a
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index baa1816358b156..5cc220e193b28d 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -77,6 +77,7 @@
 // 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)
diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp
index 8a640c9691b321..efcf97abb64aae 100644
--- a/clang/test/SemaCXX/uninitialized.cpp
+++ b/clang/test/SemaCXX/uninitialized.cpp
@@ -1472,3 +1472,49 @@ template<typename T> struct Outer {
   };
 };
 Outer<int>::Inner outerinner;
+
+void aggregate() {
+  struct S {
+    [[clang::requires_explicit_initialization]] int x;
+    int y;
+    int z = 12;
+    [[clang::requires_explicit_initialization]] int q = 100;
+    static void foo(S) { }
+  };
+
+  struct D : S { // expected-warning {{not explicitly initialized}}
+    int f1;
+    int f2 [[clang::requires_explicit_initialization]];
+  };
+
+  struct C {
+    [[clang::requires_explicit_initialization]] int w;
+    C() = default;  // Test pre-C++20 aggregates
+  };
+
+  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}} expected-warning {{'q' is not explicitly initialized}}
+  S s{.x = 100, .q = 100};
+  (void)s;
+  S t{.q = 100}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'x' is not explicitly initialized}}
+  (void)t;
+  S *ptr1 = new S; // expected-warning {{not explicitly initialized}}
+  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}}
+  (void)a;
+#else
+  C a; // expected-warning {{not explicitly initialized}}
+  (void)a;
+#endif
+  D b{.f2 = 1}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'q' is not explicitly initialized}}
+  (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}} 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}}
+  (void)c;
+  D d; // expected-warning {{not explicitly initialized}}  // expected-note {{constructor}}
+  (void)d;
+}



More information about the cfe-commits mailing list