https://github.com/snarang181 updated https://github.com/llvm/llvm-project/pull/144161
>From 10b723750e120420970f864ebda05e0495d4632f Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Fri, 13 Jun 2025 23:22:18 +0200 Subject: [PATCH 1/2] Implement diagnostics for why `std::is_standard_layout` is false --- .../clang/Basic/DiagnosticSemaKinds.td | 9 +- clang/lib/Sema/SemaTypeTraits.cpp | 132 ++++++++++++++++++ .../type-traits-unsatisfied-diags-std.cpp | 50 +++++++ .../SemaCXX/type-traits-unsatisfied-diags.cpp | 80 +++++++++++ 4 files changed, 270 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8fe7ad6138aa0..8660ba6231998 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1767,7 +1767,8 @@ def note_unsatisfied_trait : Note<"%0 is not %enum_select<TraitName>{" "%TriviallyRelocatable{trivially relocatable}|" "%Replaceable{replaceable}|" - "%TriviallyCopyable{trivially copyable}" + "%TriviallyCopyable{trivially copyable}|" + "%StandardLayout{standard-layout}" "}1">; def note_unsatisfied_trait_reason @@ -1787,6 +1788,12 @@ def note_unsatisfied_trait_reason "%NonReplaceableField{has a non-replaceable member %1 of type %2}|" "%NTCBase{has a non-trivially-copyable base %1}|" "%NTCField{has a non-trivially-copyable member %1 of type %2}|" + "%NonStdLayoutBase{has a non-standard-layout base %1}|" + "%MixedAccess{has mixed access specifiers}|" + "%MultipleDataBase{has multiple base classes with data members}|" + "%VirtualFunction{has virtual functions}|" + "%NonStdLayoutMember{has a non-standard-layout member %1 of type %2}|" + "%IndirectBaseWithFields{has an indirect base %1 with data members}|" "%DeletedDtr{has a %select{deleted|user-provided}1 destructor}|" "%UserProvidedCtr{has a user provided %select{copy|move}1 " "constructor}|" diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp index 1738ab4466001..ef9f48918f5dc 100644 --- a/clang/lib/Sema/SemaTypeTraits.cpp +++ b/clang/lib/Sema/SemaTypeTraits.cpp @@ -1947,6 +1947,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) { TypeTrait::UTT_IsCppTriviallyRelocatable) .Case("is_replaceable", TypeTrait::UTT_IsReplaceable) .Case("is_trivially_copyable", TypeTrait::UTT_IsTriviallyCopyable) + .Case("is_standard_layout", TypeTrait::UTT_IsStandardLayout) .Default(std::nullopt); } @@ -2276,6 +2277,134 @@ static void DiagnoseNonTriviallyCopyableReason(Sema &SemaRef, SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; } +static bool hasMixedAccessSpecifier(const CXXRecordDecl *D) { + AccessSpecifier FirstAccess = AS_none; + for (const FieldDecl *Field : D->fields()) { + + if (Field->isUnnamedBitField()) + continue; + AccessSpecifier FieldAccess = Field->getAccess(); + if (FirstAccess == AS_none) { + FirstAccess = FieldAccess; + } else if (FieldAccess != FirstAccess) { + return true; + } + } + return false; +} + +static bool hasMultipleDataBaseClassesWithFields(const CXXRecordDecl *D) { + int NumBasesWithFields = 0; + for (const CXXBaseSpecifier &Base : D->bases()) { + const CXXRecordDecl *BaseRD = Base.getType()->getAsCXXRecordDecl(); + if (!BaseRD || BaseRD->isInvalidDecl()) + continue; + + for (const FieldDecl *Field : BaseRD->fields()) { + if (!Field->isUnnamedBitField()) { + ++NumBasesWithFields; + break; // Only count the base once. + } + } + } + return NumBasesWithFields > 1; +} + +static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc, + const CXXRecordDecl *D) { + for (const CXXBaseSpecifier &B : D->bases()) { + assert(B.getType()->getAsCXXRecordDecl() && "invalid base?"); + if (B.isVirtual()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::VBase << B.getType() + << B.getSourceRange(); + } + if (!B.getType()->isStandardLayoutType()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::NonStdLayoutBase << B.getType() + << B.getSourceRange(); + } + } + if (hasMixedAccessSpecifier(D)) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::MixedAccess; + } + if (hasMultipleDataBaseClassesWithFields(D)) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::MultipleDataBase; + } + if (D->isPolymorphic()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::VirtualFunction; + } + for (const FieldDecl *Field : D->fields()) { + if (!Field->getType()->isStandardLayoutType()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::NonStdLayoutMember << Field + << Field->getType() << Field->getSourceRange(); + } + } + + // if this class and an indirect base + // both have non-static data members, grab the first such base. + if (D->hasDirectFields()) { + SmallVector<const CXXRecordDecl *, 4> Records; + + // Recursive lambda to collect all bases that declare fields + std::function<void(const CXXRecordDecl *)> collect = + [&](const CXXRecordDecl *R) { + for (const CXXBaseSpecifier &B : R->bases()) { + const auto *BR = B.getType()->getAsCXXRecordDecl(); + if (!BR || !BR->hasDefinition()) + continue; + if (BR->hasDirectFields()) + Records.push_back(BR); + // Recurse into the base class. + collect(BR); + } + }; + + // Collect all bases that declare fields. + collect(D); + + // If more than one record has fields, then the layout is non-standard. + if (!Records.empty()) { + const CXXRecordDecl *Indirect = Records.front(); + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::IndirectBaseWithFields << Indirect + << Indirect->getSourceRange(); + } + } +} + +static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc, + QualType T) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait) + << T << diag::TraitName::StandardLayout; + + // Check type-level exclusion first + if (T->isVariablyModifiedType()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::VLA; + return; + } + + if (T->isReferenceType()) { + SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) + << diag::TraitNotSatisfiedReason::Ref; + return; + } + T = T.getNonReferenceType(); + const CXXRecordDecl *D = T->getAsCXXRecordDecl(); + if (!D || D->isInvalidDecl()) + return; + + if (D->hasDefinition()) + DiagnoseNonStandardLayoutReason(SemaRef, Loc, D); + + SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; +} + void Sema::DiagnoseTypeTraitDetails(const Expr *E) { E = E->IgnoreParenImpCasts(); if (E->containsErrors()) @@ -2296,6 +2425,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) { case UTT_IsTriviallyCopyable: DiagnoseNonTriviallyCopyableReason(*this, E->getBeginLoc(), Args[0]); break; + case UTT_IsStandardLayout: + DiagnoseNonStandardLayoutReason(*this, E->getBeginLoc(), Args[0]); + break; default: break; } diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp index 329b611110c1d..59f3d95a4d215 100644 --- a/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp +++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp @@ -20,6 +20,13 @@ struct is_trivially_copyable { template <typename T> constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T); + +template <typename T> +struct is_standard_layout { +static constexpr bool value = __is_standard_layout(T); +}; +template <typename T> +constexpr bool is_standard_layout_v = __is_standard_layout(T); #endif #ifdef STD2 @@ -44,6 +51,17 @@ using is_trivially_copyable = __details_is_trivially_copyable<T>; template <typename T> constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T); + +template <typename T> +struct __details_is_standard_layout { +static constexpr bool value = __is_standard_layout(T); + + +}; +template <typename T> +using is_standard_layout = __details_is_standard_layout<T>; +template <typename T> +constexpr bool is_standard_layout_v = __is_standard_layout(T); #endif @@ -73,6 +91,13 @@ using is_trivially_copyable = __details_is_trivially_copyable<T>; template <typename T> constexpr bool is_trivially_copyable_v = is_trivially_copyable<T>::value; + +template <typename T> +struct __details_is_standard_layout : bool_constant<__is_standard_layout(T)> {}; +template <typename T> +using is_standard_layout = __details_is_standard_layout<T>; +template <typename T> +constexpr bool is_standard_layout_v = is_standard_layout<T>::value; #endif } @@ -100,6 +125,21 @@ static_assert(std::is_trivially_copyable_v<int&>); // expected-note@-1 {{because it is a reference type}} + // Direct tests + static_assert(std::is_standard_layout<int>::value); + static_assert(std::is_standard_layout_v<int>); + + static_assert(std::is_standard_layout<int&>::value); + // expected-error-re@-1 {{static assertion failed due to requirement 'std::{{.*}}is_standard_layout<int &>::value'}} \ + // expected-note@-1 {{'int &' is not standard-layout}} \ + // expected-note@-1 {{because it is a reference type}} + + static_assert(std::is_standard_layout_v<int&>); + // expected-error@-1 {{static assertion failed due to requirement 'std::is_standard_layout_v<int &>'}} \ + // expected-note@-1 {{'int &' is not standard-layout}} \ + // expected-note@-1 {{because it is a reference type}} + + namespace test_namespace { using namespace std; static_assert(is_trivially_relocatable<int&>::value); @@ -119,6 +159,16 @@ namespace test_namespace { // expected-error@-1 {{static assertion failed due to requirement 'is_trivially_copyable_v<int &>'}} \ // expected-note@-1 {{'int &' is not trivially copyable}} \ // expected-note@-1 {{because it is a reference type}} + + static_assert(is_standard_layout<int&>::value); + // expected-error-re@-1 {{static assertion failed due to requirement '{{.*}}is_standard_layout<int &>::value'}} \ + // expected-note@-1 {{'int &' is not standard-layout}} \ + // expected-note@-1 {{because it is a reference type}} + + static_assert(is_standard_layout_v<int&>); + // expected-error@-1 {{static assertion failed due to requirement 'is_standard_layout_v<int &>'}} \ + // expected-note@-1 {{'int &' is not standard-layout}} \ + // expected-note@-1 {{because it is a reference type}} } diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp index a8c78f6304ca9..404f6e914b3f6 100644 --- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp +++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp @@ -488,3 +488,83 @@ static_assert(__is_trivially_copyable(S12)); // expected-note@-1 {{'S12' is not trivially copyable}} \ // expected-note@#tc-S12 {{'S12' defined here}} } + +namespace standard_layout_tests { +struct WithVirtual { // #sl-Virtual + virtual void foo(); +}; +static_assert(__is_standard_layout(WithVirtual)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::WithVirtual)'}} \ +// expected-note@-1 {{'WithVirtual' is not standard-layout}} \ +// expected-note@-1 {{because it has virtual functions}} \ +// expected-note@#sl-Virtual {{'WithVirtual' defined here}} + +struct MixedAccess { // #sl-Mixed +public: + int a; +private: + int b; +}; +static_assert(__is_standard_layout(MixedAccess)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::MixedAccess)'}} \ +// expected-note@-1 {{'MixedAccess' is not standard-layout}} \ +// expected-note@-1 {{because it has mixed access specifiers}} \ +// expected-note@#sl-Mixed {{'MixedAccess' defined here}} + +struct VirtualBase { virtual ~VirtualBase(); }; // #sl-VirtualBase +struct VB : virtual VirtualBase {}; // #sl-VB +static_assert(__is_standard_layout(VB)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::VB)'}} \ +// expected-note@-1 {{'VB' is not standard-layout}} \ +// expected-note@-1 {{because it has a virtual base 'VirtualBase'}} \ +// expected-note@-1 {{because it has a non-standard-layout base 'VirtualBase'}} \ +// expected-note@-1 {{because it has virtual functions}} +// expected-note@#sl-VB {{'VB' defined here}} + +union U { // #sl-U +public: + int x; +private: + int y; +}; +static_assert(__is_standard_layout(U)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::U)'}} \ +// expected-note@-1 {{'U' is not standard-layout}} \ +// expected-note@-1 {{because it has mixed access specifiers}} +// expected-note@#sl-U {{'U' defined here}} + +// Single base class is OK +struct BaseClass{ int a; }; // #sl-BaseClass +struct DerivedOK : BaseClass {}; // #sl-DerivedOK +static_assert(__is_standard_layout(DerivedOK)); + +// Primitive types should be standard layout +static_assert(__is_standard_layout(int)); // #sl-Int +static_assert(__is_standard_layout(float)); // #sl-Float + +// Multi-level inheritance: Non-standard layout +struct Base1 { int a; }; // #sl-Base1 +struct Base2 { int b; }; // #sl-Base2 +struct DerivedClass : Base1, Base2 {}; // #sl-DerivedClass +static_assert(__is_standard_layout(DerivedClass)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::DerivedClass)'}} \ +// expected-note@-1 {{'DerivedClass' is not standard-layout}} \ +// expected-note@-1 {{because it has multiple base classes with data members}} \ +// expected-note@#sl-DerivedClass {{'DerivedClass' defined here}} + +// Inheritance hierarchy with multiple classes having data members +struct BaseA { int a; }; // #sl-BaseA +struct BaseB : BaseA {}; // inherits BaseA, has no new members +struct BaseC: BaseB { int c; }; // #sl-BaseC +static_assert(__is_standard_layout(BaseC)); +// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::BaseC)'}} \ +// expected-note@-1 {{'BaseC' is not standard-layout}} \ +// expected-note@-1 {{because it has an indirect base 'BaseA' with data members}} \ +// expected-note@#sl-BaseC {{'BaseC' defined here}} \ +// Multiple direct base classes with no data members --> standard layout +struct BaseX {}; // #sl-BaseX +struct BaseY {}; // #sl-BaseY +struct MultiBase : BaseX, BaseY {}; // #sl-MultiBase +static_assert(__is_standard_layout(MultiBase)); + + } >From 46f2a26b05a4cef58de1102c03842af92f62f3dc Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Sat, 14 Jun 2025 08:56:44 +0200 Subject: [PATCH 2/2] Replace hand-written recursion with forAllBases --- clang/lib/Sema/SemaTypeTraits.cpp | 35 +++++++++---------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp index ef9f48918f5dc..08682cc09f350 100644 --- a/clang/lib/Sema/SemaTypeTraits.cpp +++ b/clang/lib/Sema/SemaTypeTraits.cpp @@ -2344,32 +2344,17 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc, << Field->getType() << Field->getSourceRange(); } } - - // if this class and an indirect base - // both have non-static data members, grab the first such base. + // find any indirect base classes that have fields if (D->hasDirectFields()) { - SmallVector<const CXXRecordDecl *, 4> Records; - - // Recursive lambda to collect all bases that declare fields - std::function<void(const CXXRecordDecl *)> collect = - [&](const CXXRecordDecl *R) { - for (const CXXBaseSpecifier &B : R->bases()) { - const auto *BR = B.getType()->getAsCXXRecordDecl(); - if (!BR || !BR->hasDefinition()) - continue; - if (BR->hasDirectFields()) - Records.push_back(BR); - // Recurse into the base class. - collect(BR); - } - }; - - // Collect all bases that declare fields. - collect(D); - - // If more than one record has fields, then the layout is non-standard. - if (!Records.empty()) { - const CXXRecordDecl *Indirect = Records.front(); + const CXXRecordDecl *Indirect = nullptr; + D->forallBases([&](const CXXRecordDecl *BaseDef) { + if (BaseDef->hasDirectFields()) { + Indirect = BaseDef; + return false; // stop traversal + } + return true; // continue to the next base + }); + if (Indirect) { SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) << diag::TraitNotSatisfiedReason::IndirectBaseWithFields << Indirect << Indirect->getSourceRange(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits