https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/102040
>From e98e6f210f02af0813393d88e1bc4f02c0682e5f Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Mon, 5 Aug 2024 15:04:19 -0400 Subject: [PATCH 1/8] Add Clang attribute to ensure that fields are initialized explicitly --- .../clang/AST/CXXRecordDeclDefinitionBits.def | 9 ++++ 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 | 24 ++++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 7 +++ clang/lib/Sema/SemaInit.cpp | 15 ++++++ ...a-attribute-supported-attributes-list.test | 1 + clang/test/SemaCXX/uninitialized.cpp | 46 +++++++++++++++++++ 11 files changed, 152 insertions(+) diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def index 6620840df0ced2..54f28046c63ebb 100644 --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def @@ -119,6 +119,15 @@ 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. /// diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 2693cc0e95b4b2..7a9c28b6af9d9e 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 6035a563d5fce7..2878ed0a9fec1b 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1885,6 +1885,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 94d6d15365cef6..fb5def41deadc8 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1599,6 +1599,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 df9bf94b5d0398..a18072e7dd5cc8 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -790,6 +790,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 157d77b38b354e..b52cc3c870295e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2318,6 +2318,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>; @@ -2347,6 +2351,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 39c548e9c22539..e1a59620158d9a 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" @@ -80,6 +81,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), @@ -1113,6 +1115,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; @@ -1367,6 +1373,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; @@ -2183,6 +2193,20 @@ 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 && isAggregate() && + 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 146d9c86e0715a..045bc23af52b00 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6147,6 +6147,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)); } @@ -7061,6 +7065,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 7c03a12e812809..3c78555c4fadab 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -738,6 +738,14 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, ILE->updateInit(SemaRef.Context, Init, Filler); return; } + + if (!VerifyOnly && Field->hasAttr<ExplicitInitAttr>()) { + SemaRef.Diag(ILE->getExprLoc(), diag::warn_field_requires_explicit_init) + << Field; + SemaRef.Diag(Field->getLocation(), diag::note_entity_declared_at) + << 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 @@ -4562,6 +4570,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 4a6ac39da18ad2..55f196625770ab 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -79,6 +79,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..37cf6d1dd60c0a 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; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} + int y; + int z = 12; + [[clang::requires_explicit_initialization]] int q = 100; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} + static void foo(S) { } + }; + + struct D : S { // expected-warning {{not explicitly initialized}} + int f1; + int f2 [[clang::requires_explicit_initialization]]; // expected-note {{declared}} // expected-note {{declared}} + }; + + 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}} + S s{.x = 100, .q = 100}; + (void)s; + S t{.q = 100}; // 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}} + 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; +} >From 539c0cdaaa1cfb28d24b83eadfba85248a42ebe8 Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 25 Sep 2024 13:28:46 -0400 Subject: [PATCH 2/8] Implement explicit field initialization checks in RecordDecl and make it work in both C and C++ --- .../clang/AST/CXXRecordDeclDefinitionBits.def | 9 -- clang/include/clang/AST/Decl.h | 8 ++ clang/include/clang/AST/DeclBase.h | 13 ++- clang/include/clang/AST/DeclCXX.h | 5 - clang/include/clang/Basic/Attr.td | 3 +- clang/include/clang/Basic/AttrDocs.td | 31 +++--- .../clang/Basic/DiagnosticSemaKinds.td | 8 +- clang/lib/AST/Decl.cpp | 8 +- clang/lib/AST/DeclCXX.cpp | 32 ++++-- clang/lib/Sema/SemaDecl.cpp | 3 + clang/lib/Sema/SemaInit.cpp | 26 ++++- clang/lib/Serialization/ASTReaderDecl.cpp | 1 + clang/lib/Serialization/ASTWriterDecl.cpp | 4 +- clang/test/Sema/uninit-variables.c | 11 ++ clang/test/SemaCXX/uninitialized.cpp | 100 ++++++++++++++---- 15 files changed, 196 insertions(+), 66 deletions(-) diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def index 54f28046c63ebb..6620840df0ced2 100644 --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/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. /// diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8c39ef3d5a9fa6..5e8939384fc39d 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -4273,6 +4273,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. diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index a3447d19909752..885c4c7a0bd27c 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -1450,6 +1450,9 @@ class DeclContext { /// hasLazyLocalLexicalLookups, hasLazyExternalLexicalLookups friend class ASTWriter; +protected: + enum { NumOdrHashBits = 25 }; + // We use uint64_t in the bit-fields below since some bit-fields // cross the unsigned boundary and this breaks the packing. @@ -1671,6 +1674,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; @@ -1685,7 +1696,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 : NumOdrHashBits; }; /// Number of inherited and non-inherited bits in RecordDeclBitfields. diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 7a9c28b6af9d9e..2693cc0e95b4b2 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/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. /// diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 2878ed0a9fec1b..80fb55a403ac9b 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1886,10 +1886,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; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index fb5def41deadc8..34fef2caf841cf 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1606,24 +1606,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 + }); + } }]; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b52cc3c870295e..31f846c1b0825c 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2352,8 +2352,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< @@ -3156,6 +3156,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>; diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index bfeb4827f79587..d5e8f75b877d89 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5029,6 +5029,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); @@ -5229,9 +5230,10 @@ unsigned RecordDecl::getODRHash() { // Only calculate hash on first call of getODRHash per record. ODRHash Hash; Hash.AddRecordDecl(this); - // For RecordDecl the ODRHash is stored in the remaining 26 - // bit of RecordDeclBits, adjust the hash to accomodate. - setODRHash(Hash.CalculateHash() >> 6); + // For RecordDecl the ODRHash is stored in the remaining + // bits of RecordDeclBits, adjust the hash to accommodate. + static_assert(sizeof(Hash.CalculateHash()) * CHAR_BIT == 32); + setODRHash(Hash.CalculateHash() >> (32 - NumOdrHashBits)); return RecordDeclBits.ODRHash; } diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index e1a59620158d9a..11eaff4284d5fd 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -81,7 +81,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), @@ -459,6 +458,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; @@ -1116,7 +1119,7 @@ void CXXRecordDecl::addedMember(Decl *D) { data().PlainOldData = false; if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) { - data().HasUninitializedExplicitInitFields = true; + setHasUninitializedExplicitInitFields(true); } if (T->isReferenceType()) { @@ -1375,7 +1378,7 @@ void CXXRecordDecl::addedMember(Decl *D) { if (FieldRec->hasUninitializedExplicitInitFields() && FieldRec->isAggregate() && !Field->hasInClassInitializer()) - data().HasUninitializedExplicitInitFields = true; + setHasUninitializedExplicitInitFields(true); if (FieldRec->hasUninitializedReferenceMember() && !Field->hasInClassInitializer()) @@ -2195,17 +2198,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); } } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index be570f3a1829d0..6885403169ee76 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19189,6 +19189,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); diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 3c78555c4fadab..a553f76e0caad6 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -264,6 +264,14 @@ static void CheckStringInit(Expr *Str, QualType &DeclT, const ArrayType *AT, updateStringLiteralType(Str, DeclT); } +void emitUninitializedExplicitInitFields(Sema &S, const RecordDecl *R) { + for (const FieldDecl *Field : R->fields()) { + if (Field->hasAttr<ExplicitInitAttr>()) { + S.Diag(Field->getLocation(), diag::note_entity_declared_at) << Field; + } + } +} + //===----------------------------------------------------------------------===// // Semantic checking for initializer lists. //===----------------------------------------------------------------------===// @@ -741,7 +749,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; } @@ -4574,7 +4582,8 @@ 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; + emitUninitializedExplicitInitFields(S, DestRecordDecl); } // C++11 [dcl.init]p6: @@ -6476,6 +6485,19 @@ 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; + emitUninitializedExplicitInitFields(S, Rec); + } + } + } + // - If the destination type is a reference type, see 8.5.3. if (DestType->isReferenceType()) { // C++0x [dcl.init.ref]p1: diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 6ece3ba7af9f4b..6f78daad97fd71 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/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)); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index ad357e30d57529..f9b35a65bf95eb 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -573,6 +573,7 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) { RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveDefaultInitializeCUnion()); RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveDestructCUnion()); RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveCopyCUnion()); + RecordDeclBits.addBit(D->hasUninitializedExplicitInitFields()); RecordDeclBits.addBit(D->isParamDestroyedInCallee()); RecordDeclBits.addBits(llvm::to_underlying(D->getArgPassingRestrictions()), 2); Record.push_back(RecordDeclBits); @@ -2430,7 +2431,8 @@ void ASTWriter::WriteDeclAbbrevs() { // isNonTrivialToPrimitiveCopy, isNonTrivialToPrimitiveDestroy, // hasNonTrivialToPrimitiveDefaultInitializeCUnion, // hasNonTrivialToPrimitiveDestructCUnion, - // hasNonTrivialToPrimitiveCopyCUnion, isParamDestroyedInCallee, + // hasNonTrivialToPrimitiveCopyCUnion, + // hasUninitializedExplicitInitFields, isParamDestroyedInCallee, // getArgPassingRestrictions // ODRHash Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 26)); diff --git a/clang/test/Sema/uninit-variables.c b/clang/test/Sema/uninit-variables.c index 70a00793fd29ed..d4f4686b600c40 100644 --- a/clang/test/Sema/uninit-variables.c +++ b/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]]; // #FIELD_Y +}; + +void aggregate() { + struct with_explicit_field a; // expected-warning {{field in 'with_explicit_field' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}} + struct with_explicit_field b = {1}; // expected-warning {{field 'y' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}} + (void)(&a != &b); +} diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 37cf6d1dd60c0a..95e04cdfe5909a 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -1474,47 +1474,103 @@ template<typename T> struct Outer { Outer<int>::Inner outerinner; void aggregate() { - struct S { - [[clang::requires_explicit_initialization]] int x; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} - int y; - int z = 12; - [[clang::requires_explicit_initialization]] int q = 100; // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} // expected-note {{declared}} - static void foo(S) { } + struct NonAgg { + NonAgg() { } + [[clang::requires_explicit_initialization]] int na; // expected-warning {{'requires_explicit_initialization' attribute is ignored in non-aggregate type 'NonAgg'}} }; + NonAgg nonagg; // no-warning + (void)nonagg; - struct D : S { // expected-warning {{not explicitly initialized}} - int f1; - int f2 [[clang::requires_explicit_initialization]]; // expected-note {{declared}} // expected-note {{declared}} + struct S { + [[clang::requires_explicit_initialization]] int s1; // #FIELD_S1 + int s2; + int s3 = 12; + [[clang::requires_explicit_initialization]] int s4 = 100; // #FIELD_S4 + static void foo(S) { } }; struct C { - [[clang::requires_explicit_initialization]] int w; + [[clang::requires_explicit_initialization]] int c1; // #FIELD_C1 C() = default; // Test pre-C++20 aggregates }; + struct D : S { // #TYPE_D + int d1; + int d2 [[clang::requires_explicit_initialization]]; // #FIELD_D2 + }; + + struct D2 : D { // #TYPE_D2 + }; + + struct E { // #TYPE_E + int e1; + D e2 [[clang::requires_explicit_initialization]]; // #FIELD_E2 + struct { + [[clang::requires_explicit_initialization]] D e3; + D2 e4 [[clang::requires_explicit_initialization]]; + }; + }; + 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 s{.x = 100, .q = 100}; + S::foo(S{.s1 = 100, .s4 = 100}); + S::foo(S{.s1 = 100}); // expected-warning {{field 's4' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S4 {{'s4' declared here}} + + S s{.s1 = 100, .s4 = 100}; (void)s; - S t{.q = 100}; // expected-warning {{'x' is not explicitly initialized}} + + S t{.s4 = 100}; // expected-warning {{field 's1' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} (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}} expected-note@#FIELD_S4 {{'s4' declared here}} expected-note@#FIELD_S1 {{'s1' declared here}} delete ptr1; - S *ptr2 = new S{.x = 100, .q = 100}; + + S *ptr2 = new S{.s1 = 100, .s4 = 100}; delete ptr2; + #if __cplusplus >= 202002L - D a({}, 0); // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'f2' is not explicitly initialized}} + // expected-warning@+2 {{field 'd2' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_D2 {{'d2' declared here}} + // expected-warning {{field 's1' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} + D a({}, 0); (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}} expected-note@#FIELD_C1 {{'c1' declared here}} (void)a; #endif - D b{.f2 = 1}; // expected-warning {{'x' is not explicitly initialized}} expected-warning {{'q' is not explicitly initialized}} + + // expected-warning@+2 {{field 's4' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S4 {{'s4' declared here}} + // expected-warning@+1 {{field 's1' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} + D b{.d2 = 1}; (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}} + + // expected-warning@+3 {{field 's4' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S4 {{'s4' declared here}} + // expected-warning@+2 {{field 'd2' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_D2 {{'d2' declared here}} + // expected-warning@+1 {{field 's1' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} + D c{.d1 = 5}; + + // expected-warning@+3 {{field 's4' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S4 {{'s4' declared here}} + // expected-warning@+2 {{field 'd2' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_D2 {{'d2' declared here}} + // expected-warning@+1 {{field 's1' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} + c = {{}, 0}; (void)c; - D d; // expected-warning {{not explicitly initialized}} // expected-note {{constructor}} + + // expected-note@+3 {{in implicit default constructor for 'D' first required here}} + // expected-warning@#TYPE_D {{field in 'S' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_S1 {{'s1' declared here}} expected-note@#FIELD_S4 {{'s4' declared here}} + // expected-warning@+1 {{field in 'D' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_D2 {{'d2' declared here}} + D d; (void)d; + + // expected-warning@+12 {{field in 'E' is not explicitly initialized, but was marked as requiring explicit initialization}} + // expected-note@#FIELD_E2 {{'e2' declared here}} + // expected-warning@#TYPE_E {{field in 'D' is not explicitly initialized, but was marked as requiring explicit initialization}} + // expected-note@+9 {{in implicit default constructor for 'E' first required here}} + // expected-note@#FIELD_D2 {{'d2' declared here}} + // expected-warning@#TYPE_E {{field in 'D' is not explicitly initialized, but was marked as requiring explicit initialization}} + // expected-note@#FIELD_D2 {{'d2' declared here}} + // expected-warning@#TYPE_E {{field in 'D2' is not explicitly initialized, but was marked as requiring explicit initialization}} + // expected-note@#TYPE_E {{in implicit default constructor for 'D2' first required here}} + // expected-warning@#TYPE_D2 {{field in 'D' is not explicitly initialized, but was marked as requiring explicit initialization}} + // expected-note@+2 {{in implicit default constructor for 'E' first required here}} + // expected-note@#FIELD_D2 {{'d2' declared here}} + E e; + (void)e; } >From 47c244a88a4a65c4d26566a7e3a660598c32c40c Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:05:55 -0500 Subject: [PATCH 3/8] Remove handleExplicitInitAttr() as suggested --- clang/lib/Sema/SemaDeclAttr.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 045bc23af52b00..146d9c86e0715a 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6147,10 +6147,6 @@ 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)); } @@ -7065,9 +7061,6 @@ 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; >From 44961d247feae0b2581e70cb50d33a54c4e35068 Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:07:04 -0500 Subject: [PATCH 4/8] Linting --- clang/lib/AST/DeclCXX.cpp | 21 +++++++++------------ clang/lib/Sema/SemaDecl.cpp | 3 +-- clang/lib/Sema/SemaInit.cpp | 3 +-- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 11eaff4284d5fd..21e1cdc2ac4541 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -1118,9 +1118,8 @@ void CXXRecordDecl::addedMember(Decl *D) { } else if (!T.isCXX98PODType(Context)) data().PlainOldData = false; - if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) { + if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) setHasUninitializedExplicitInitFields(true); - } if (T->isReferenceType()) { if (!Field->hasInClassInitializer()) @@ -2202,13 +2201,12 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) { 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>()) { + for (const FieldDecl *FD : fields()) { + if (const auto *AT = FD->getAttr<ExplicitInitAttr>()) Context.getDiagnostics().Report( - attr->getLocation(), + AT->getLocation(), diag::warn_cxx20_compat_requires_explicit_init_non_aggregate) - << attr << Context.getRecordType(this); - } + << AT << Context.getRecordType(this); } } @@ -2216,12 +2214,11 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) { // 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(), + for (const FieldDecl *FD : fields()) { + if (const auto *AT = FD->getAttr<ExplicitInitAttr>()) + Context.getDiagnostics().Report(AT->getLocation(), diag::warn_attribute_needs_aggregate) - << attr << Context.getRecordType(this); - } + << AT << Context.getRecordType(this); } setHasUninitializedExplicitInitFields(false); } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 6885403169ee76..f2847555f803e6 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -19189,9 +19189,8 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl, if (FT.hasNonTrivialToPrimitiveCopyCUnion() || Record->isUnion()) Record->setHasNonTrivialToPrimitiveCopyCUnion(true); } - if (FD->hasAttr<ExplicitInitAttr>()) { + if (FD->hasAttr<ExplicitInitAttr>()) Record->setHasUninitializedExplicitInitFields(true); - } if (FT.isDestructedType()) { Record->setNonTrivialToPrimitiveDestroy(true); Record->setParamDestroyedInCallee(true); diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index a553f76e0caad6..d293e2d9944675 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -266,9 +266,8 @@ static void CheckStringInit(Expr *Str, QualType &DeclT, const ArrayType *AT, void emitUninitializedExplicitInitFields(Sema &S, const RecordDecl *R) { for (const FieldDecl *Field : R->fields()) { - if (Field->hasAttr<ExplicitInitAttr>()) { + if (Field->hasAttr<ExplicitInitAttr>()) S.Diag(Field->getLocation(), diag::note_entity_declared_at) << Field; - } } } >From 3d38ed3a533861c1bd10ca03ea89f02ca6d5079f Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:15:29 -0500 Subject: [PATCH 5/8] Error message improvements --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 31f846c1b0825c..3937afeeaa2a22 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2319,9 +2319,9 @@ 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>; + "explicit initialization of field %0 will not be enforced in C++20 and later because %1 " + "has a user-declared constructor, making the type no longer an aggregate">, + 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>; @@ -2352,8 +2352,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 %select{%1|in %1}0 is not explicitly initialized, but was marked as " - "requiring explicit initialization">, InGroup<UninitializedExplicitInit>; + "field %select{%1|in %1}0 requires explicit initialization but is not explicitly initialized">, + InGroup<UninitializedExplicitInit>; def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">, InGroup<Uninitialized>; def warn_base_class_is_uninit : Warning< >From 85b977a66717ca90732ce1a39aa810c07383f223 Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:26:37 -0500 Subject: [PATCH 6/8] Update documentation --- clang/include/clang/Basic/AttrDocs.td | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 34fef2caf841cf..66dadabb07f031 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1603,30 +1603,41 @@ 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. +field of an aggregate must be initialized explicitly by users when the type +is constructed. The attribute supports both C and C++, but its usage is invalid +on non-aggregates. 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. +Rather, it is intended for use in "parameter-objects", used to simulate, +for example, the passing of named parameters. The attribute generates a warning when explicit initializers for such -"named parameters" are not provided: +variables are not provided (this occurs regardless of whether any in-class field +initializers exist): .. code-block:: c++ + struct Buffer { + void *address [[clang::requires_explicit_initialization]]; + size_t length [[clang::requires_explicit_initialization]] = 0; + }; + struct ArrayIOParams { size_t count [[clang::requires_explicit_initialization]]; size_t element_size [[clang::requires_explicit_initialization]]; int flags = 0; }; - size_t ReadArray(FILE *file, void *buffer, ArrayIOParams params); + size_t ReadArray(FILE *file, struct Buffer buffer, + struct ArrayIOParams params); int main() { unsigned int buf[512]; - ReadArray(stdin, buf, { + ReadArray(stdin, { + buf + // warning: field 'length' is not explicitly initialized + }, { .count = sizeof(buf) / sizeof(*buf), // warning: field 'element_size' is not explicitly initialized }); >From d43ea1b7221737646fbb9b9f79c2b22d77136143 Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:22:33 -0500 Subject: [PATCH 7/8] Fix bug in detecting in-class member initializers --- clang/lib/AST/DeclCXX.cpp | 4 ++-- clang/test/SemaCXX/uninitialized.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 21e1cdc2ac4541..99fc884c2ca0dc 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -1118,7 +1118,7 @@ void CXXRecordDecl::addedMember(Decl *D) { } else if (!T.isCXX98PODType(Context)) data().PlainOldData = false; - if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) + if (Field->hasAttr<ExplicitInitAttr>()) setHasUninitializedExplicitInitFields(true); if (T->isReferenceType()) { @@ -1376,7 +1376,7 @@ void CXXRecordDecl::addedMember(Decl *D) { data().ImplicitCopyAssignmentHasConstParam = false; if (FieldRec->hasUninitializedExplicitInitFields() && - FieldRec->isAggregate() && !Field->hasInClassInitializer()) + FieldRec->isAggregate()) setHasUninitializedExplicitInitFields(true); if (FieldRec->hasUninitializedReferenceMember() && diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 95e04cdfe5909a..05877b9bc1b4f6 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -1490,7 +1490,7 @@ void aggregate() { }; struct C { - [[clang::requires_explicit_initialization]] int c1; // #FIELD_C1 + [[clang::requires_explicit_initialization]] int c1 = 2; // #FIELD_C1 C() = default; // Test pre-C++20 aggregates }; >From 56f5c64390ed4d31f91d9eb9078b4ed00131bc18 Mon Sep 17 00:00:00 2001 From: higher-performance <higher.performance.git...@gmail.com> Date: Wed, 20 Nov 2024 15:42:31 -0500 Subject: [PATCH 8/8] Improve tests --- clang/include/clang/Basic/AttrDocs.td | 2 ++ clang/test/Sema/uninit-variables.c | 18 ++++++++++++ clang/test/SemaCXX/uninitialized.cpp | 40 ++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 66dadabb07f031..c08346b19a2ad6 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1640,6 +1640,8 @@ initializers exist): }, { .count = sizeof(buf) / sizeof(*buf), // warning: field 'element_size' is not explicitly initialized + // (Note that a missing initializer for 'flags' is not diagnosed, because + // the field is not marked as requiring explicit initialization.) }); } diff --git a/clang/test/Sema/uninit-variables.c b/clang/test/Sema/uninit-variables.c index d4f4686b600c40..fd8f9bceeb6b50 100644 --- a/clang/test/Sema/uninit-variables.c +++ b/clang/test/Sema/uninit-variables.c @@ -557,8 +557,26 @@ struct with_explicit_field { int y [[clang::requires_explicit_initialization]]; // #FIELD_Y }; +struct with_explicit_array { + int arr[2] [[clang::requires_explicit_initialization]]; // #FIELD_ARR +}; + +struct with_explicit_flex_array { + int flex_arr[] [[clang::requires_explicit_initialization]]; // #FIELD_FLEX_ARR +}; + void aggregate() { struct with_explicit_field a; // expected-warning {{field in 'with_explicit_field' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}} struct with_explicit_field b = {1}; // expected-warning {{field 'y' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}} (void)(&a != &b); + + struct with_explicit_field c = {1, 2}; + struct with_explicit_field d = {.y = 3}; + (void)(&c != &d); + + struct with_explicit_array e = {{1}}; // OK -- part of array is still initialized + (void)e; + + struct with_explicit_array f = {}; // expected-warning {{field 'flex_arr' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_FLEX_ARR {{'flex_arr' declared here}} + (void)f; } diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 05877b9bc1b4f6..a29e8dfc97e15a 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -1473,6 +1473,30 @@ template<typename T> struct Outer { }; Outer<int>::Inner outerinner; +struct Polymorphic { virtual ~Polymorphic() { } }; + +template<class... Bases> +struct Inherit : Bases... { // #TYPE_INHERIT + int g1; // #FIELD_G1 +}; + +template<class... Bases> +struct InheritWithExplicit : Bases... { // #TYPE_INHERIT_WITH_EXPLICIT + int g2 [[clang::requires_explicit_initialization]]; // #FIELD_G2 +}; + +struct Special {}; + +template<> +struct Inherit<Special> { + int g3 [[clang::requires_explicit_initialization]]; // #FIELD_G3 +}; + +template<> +struct InheritWithExplicit<Special> { + int g4; // #FIELD_G4 +}; + void aggregate() { struct NonAgg { NonAgg() { } @@ -1490,7 +1514,7 @@ void aggregate() { }; struct C { - [[clang::requires_explicit_initialization]] int c1 = 2; // #FIELD_C1 + [[clang::requires_explicit_initialization]] int c1; // #FIELD_C1 C() = default; // Test pre-C++20 aggregates }; @@ -1573,4 +1597,18 @@ void aggregate() { // expected-note@#FIELD_D2 {{'d2' declared here}} E e; (void)e; + + InheritWithExplicit<> agg; + (void)agg; + // expected-warning@#TYPE_INHERIT_WITH_EXPLICIT {{field in 'InheritWithExplicit<>' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_G2 {{'g2' declared here}} + + InheritWithExplicit<Polymorphic> polymorphic; // no-warning + (void)polymorphic; + + Inherit<Special> specialized_explicit; + (void)specialized_explicit; + // expected-warning@#TYPE_INHERIT_WITH_EXPLICIT {{field in 'Inherit<Special>' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_G3 {{'g3' declared here}} + + InheritWithExplicit<Special> specialized_implicit; // no-warning + (void)specialized_implicit; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits