[clang] [Clang] Implement diagnostics for why `std::is_standard_layout` is false (PR #144161)

Samarth Narang via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 25 06:21:57 PDT 2025


https://github.com/snarang181 updated https://github.com/llvm/llvm-project/pull/144161

>From 7f11d3392e3f3ead823c8af2ea9a3b1f9ef9e0c9 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Fri, 13 Jun 2025 23:22:18 +0200
Subject: [PATCH 1/9] 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 9392cbb39c021..232e1b94ee1da 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 4dbb2450857e0..9e34ea2fd5184 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -1956,6 +1956,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);
 }
 
@@ -2285,6 +2286,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())
@@ -2305,6 +2434,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 at -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 at -1 {{static assertion failed due to requirement 'std::{{.*}}is_standard_layout<int &>::value'}} \
+ // expected-note at -1 {{'int &' is not standard-layout}} \
+ // expected-note at -1 {{because it is a reference type}}
+
+ static_assert(std::is_standard_layout_v<int&>);
+ // expected-error at -1 {{static assertion failed due to requirement 'std::is_standard_layout_v<int &>'}} \
+ // expected-note at -1 {{'int &' is not standard-layout}} \
+ // expected-note at -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 at -1 {{static assertion failed due to requirement 'is_trivially_copyable_v<int &>'}} \
     // expected-note at -1 {{'int &' is not trivially copyable}} \
     // expected-note at -1 {{because it is a reference type}}
+
+    static_assert(is_standard_layout<int&>::value);
+     // expected-error-re at -1 {{static assertion failed due to requirement '{{.*}}is_standard_layout<int &>::value'}} \
+     // expected-note at -1 {{'int &' is not standard-layout}} \
+     // expected-note at -1 {{because it is a reference type}}
+
+     static_assert(is_standard_layout_v<int&>);
+     // expected-error at -1 {{static assertion failed due to requirement 'is_standard_layout_v<int &>'}} \
+     // expected-note at -1 {{'int &' is not standard-layout}} \
+     // expected-note at -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 5210354a66d43..128a69f0c413e 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 at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::WithVirtual)'}} \
+// expected-note at -1 {{'WithVirtual' is not standard-layout}} \
+// expected-note at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::MixedAccess)'}} \
+// expected-note at -1 {{'MixedAccess' is not standard-layout}} \
+// expected-note at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::VB)'}} \
+// expected-note at -1 {{'VB' is not standard-layout}} \
+// expected-note at -1 {{because it has a virtual base 'VirtualBase'}} \
+// expected-note at -1 {{because it has a non-standard-layout base 'VirtualBase'}} \
+// expected-note at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::U)'}} \
+// expected-note at -1 {{'U' is not standard-layout}} \
+// expected-note at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::DerivedClass)'}} \
+// expected-note at -1 {{'DerivedClass' is not standard-layout}} \
+// expected-note at -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 at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::BaseC)'}} \
+// expected-note at -1 {{'BaseC' is not standard-layout}} \
+// expected-note at -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 d2b293a7a23b61d70c2e55cb03661bc7fb75c350 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Sat, 14 Jun 2025 08:56:44 +0200
Subject: [PATCH 2/9] 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 9e34ea2fd5184..6504ea911e399 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2353,32 +2353,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();

>From 8f0a6a8f096abf74b0b4166c82b221c53b1eaa81 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Thu, 19 Jun 2025 09:35:11 +0200
Subject: [PATCH 3/9] Address PR suggestions - Part 1

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  2 +-
 clang/lib/Sema/SemaTypeTraits.cpp                | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 232e1b94ee1da..984cf203b285c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1791,7 +1791,7 @@ def note_unsatisfied_trait_reason
            "%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}|"
+           "%VirtualFunction{has a virtual function}|"
            "%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}|"
diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 6504ea911e399..46e10d54eaff4 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2293,11 +2293,10 @@ static bool hasMixedAccessSpecifier(const CXXRecordDecl *D) {
     if (Field->isUnnamedBitField())
       continue;
     AccessSpecifier FieldAccess = Field->getAccess();
-    if (FirstAccess == AS_none) {
+    if (FirstAccess == AS_none)
       FirstAccess = FieldAccess;
-    } else if (FieldAccess != FirstAccess) {
+    else if (FieldAccess != FirstAccess)
       return true;
-    }
   }
   return false;
 }
@@ -2311,12 +2310,13 @@ static bool hasMultipleDataBaseClassesWithFields(const CXXRecordDecl *D) {
 
     for (const FieldDecl *Field : BaseRD->fields()) {
       if (!Field->isUnnamedBitField()) {
-        ++NumBasesWithFields;
-        break; // Only count the base once.
+        if (++NumBasesWithFields > 1)
+          return true; // found more than one base class with fields
+        break;         // no need to check further fields in this base class
       }
     }
   }
-  return NumBasesWithFields > 1;
+  return false;
 }
 
 static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
@@ -2353,7 +2353,7 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
           << Field->getType() << Field->getSourceRange();
     }
   }
-  // find any indirect base classes that have fields
+  // Find any indirect base classes that have fields.
   if (D->hasDirectFields()) {
     const CXXRecordDecl *Indirect = nullptr;
     D->forallBases([&](const CXXRecordDecl *BaseDef) {
@@ -2376,7 +2376,7 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
   SemaRef.Diag(Loc, diag::note_unsatisfied_trait)
       << T << diag::TraitName::StandardLayout;
 
-  // Check type-level exclusion first
+  // Check type-level exclusion first.
   if (T->isVariablyModifiedType()) {
     SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
         << diag::TraitNotSatisfiedReason::VLA;

>From fb70b44e06d51479c3f1ba5c6123859c96199d00 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Thu, 19 Jun 2025 10:58:48 +0200
Subject: [PATCH 4/9] Fix tests

---
 clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index 128a69f0c413e..14a00d1555842 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -496,7 +496,7 @@ struct WithVirtual { // #sl-Virtual
 static_assert(__is_standard_layout(WithVirtual));
 // expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::WithVirtual)'}} \
 // expected-note at -1 {{'WithVirtual' is not standard-layout}} \
-// expected-note at -1 {{because it has virtual functions}} \
+// expected-note at -1 {{because it has a virtual function}} \
 // expected-note@#sl-Virtual {{'WithVirtual' defined here}}
 
 struct MixedAccess { // #sl-Mixed
@@ -518,7 +518,7 @@ static_assert(__is_standard_layout(VB));
 // expected-note at -1 {{'VB' is not standard-layout}} \
 // expected-note at -1 {{because it has a virtual base 'VirtualBase'}} \
 // expected-note at -1 {{because it has a non-standard-layout base 'VirtualBase'}} \
-// expected-note at -1 {{because it has virtual functions}}
+// expected-note at -1 {{because it has a virtual function}}
 // expected-note@#sl-VB {{'VB' defined here}}
 
 union U {      // #sl-U

>From 8aa95e2d21a3708eceae0ee85fb7ea81c6bdd98a Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Thu, 19 Jun 2025 11:11:39 +0200
Subject: [PATCH 5/9] Add method-specific diagnostics to diagnose virtual
 function failures

---
 clang/lib/Sema/SemaTypeTraits.cpp                    | 7 +++++++
 clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 46e10d54eaff4..46f461bc79091 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2345,6 +2345,13 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
   if (D->isPolymorphic()) {
     SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
         << diag::TraitNotSatisfiedReason::VirtualFunction;
+
+    for (const CXXMethodDecl *Method : D->methods()) {
+      if (Method->isVirtual()) {
+        SemaRef.Diag(Method->getLocation(), diag::note_defined_here) << Method;
+        break;
+      }
+    }
   }
   for (const FieldDecl *Field : D->fields()) {
     if (!Field->getType()->isStandardLayoutType()) {
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index 14a00d1555842..5d8994a6c868d 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -491,12 +491,13 @@ static_assert(__is_trivially_copyable(S12));
 
 namespace standard_layout_tests {
 struct WithVirtual { // #sl-Virtual
-    virtual void foo();
+    virtual void foo(); // #sl-Virtual-Foo
 };
 static_assert(__is_standard_layout(WithVirtual));
 // expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::WithVirtual)'}} \
 // expected-note at -1 {{'WithVirtual' is not standard-layout}} \
 // expected-note at -1 {{because it has a virtual function}} \
+// expected-note@#sl-Virtual-Foo {{'foo' defined here}} \
 // expected-note@#sl-Virtual {{'WithVirtual' defined here}}
 
 struct MixedAccess { // #sl-Mixed
@@ -520,6 +521,7 @@ static_assert(__is_standard_layout(VB));
 // expected-note at -1 {{because it has a non-standard-layout base 'VirtualBase'}} \
 // expected-note at -1 {{because it has a virtual function}}
 // expected-note@#sl-VB {{'VB' defined here}}
+// expected-note@#sl-VB {{'~VB' defined here}}
 
 union U {      // #sl-U
 public:

>From 952535f107f967872a853483959b10037f0014b4 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Thu, 19 Jun 2025 11:28:04 +0200
Subject: [PATCH 6/9] MixedAccess specifier diagnostics improvement Diagnose
 fields which have different access

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  1 +
 clang/lib/Sema/SemaTypeTraits.cpp             | 21 +++++++++++++++++++
 .../SemaCXX/type-traits-unsatisfied-diags.cpp | 12 +++++++----
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 984cf203b285c..ad93598888ce6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1790,6 +1790,7 @@ def note_unsatisfied_trait_reason
            "%NTCField{has a non-trivially-copyable member %1 of type %2}|"
            "%NonStdLayoutBase{has a non-standard-layout base %1}|"
            "%MixedAccess{has mixed access specifiers}|"
+           "%MixedAccessField{field %1 has a different access specifier than field %2}|"
            "%MultipleDataBase{has multiple base classes with data members}|"
            "%VirtualFunction{has a virtual function}|"
            "%NonStdLayoutMember{has a non-standard-layout member %1 of type %2}|"
diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 46f461bc79091..be07ef5e8faf6 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2337,6 +2337,27 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
   if (hasMixedAccessSpecifier(D)) {
     SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
         << diag::TraitNotSatisfiedReason::MixedAccess;
+
+    const FieldDecl *FirstField = nullptr;
+    AccessSpecifier FirstAcc = AS_none;
+    for (const FieldDecl *F : D->fields()) {
+      if (F->isUnnamedBitField())
+        continue;
+      if (FirstField == nullptr) {
+        FirstField = F;
+        FirstAcc = F->getAccess();
+      } else if (F->getAccess() != FirstAcc) {
+        SemaRef.Diag(FirstField->getLocation(), diag::note_defined_here)
+            << FirstField;
+
+        SemaRef.Diag(F->getLocation(), diag::note_unsatisfied_trait_reason)
+            << diag::TraitNotSatisfiedReason::MixedAccessField
+            << F           // %0: second field
+            << FirstField; // %1: first field
+        break;
+      }
+    }
+    return;
   }
   if (hasMultipleDataBaseClassesWithFields(D)) {
     SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index 5d8994a6c868d..2c9f2cfa6f6cf 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -502,14 +502,16 @@ static_assert(__is_standard_layout(WithVirtual));
 
 struct MixedAccess { // #sl-Mixed
 public:
-    int a;
+    int a; // #sl-MixedF1
 private:
-    int b;
+    int b; // #sl-MixedF2
 };
 static_assert(__is_standard_layout(MixedAccess));
 // expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::MixedAccess)'}} \
 // expected-note at -1 {{'MixedAccess' is not standard-layout}} \
 // expected-note at -1 {{because it has mixed access specifiers}} \
+// expected-note@#sl-MixedF1 {{'a' defined here}}
+// expected-note@#sl-MixedF2 {{field 'b' has a different access specifier than field 'a'}}
 // expected-note@#sl-Mixed {{'MixedAccess' defined here}}
 
 struct VirtualBase { virtual ~VirtualBase(); };               // #sl-VirtualBase
@@ -525,14 +527,16 @@ static_assert(__is_standard_layout(VB));
 
 union U {      // #sl-U
 public:
-    int x;
+    int x; // #sl-UF1
 private:
-    int y;
+    int y; // #sl-UF2
 };                                                       
 static_assert(__is_standard_layout(U));
 // expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::U)'}} \
 // expected-note at -1 {{'U' is not standard-layout}} \
 // expected-note at -1 {{because it has mixed access specifiers}}
+// expected-note@#sl-UF1 {{'x' defined here}}
+// expected-note@#sl-UF2 {{field 'y' has a different access specifier than field 'x'}}
 // expected-note@#sl-U {{'U' defined here}}
 
 // Single base class is OK

>From 1de5966a22f16600ef046268147e123430fbde18 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Thu, 19 Jun 2025 16:34:58 +0200
Subject: [PATCH 7/9] Remove misleading comment

---
 clang/lib/Sema/SemaTypeTraits.cpp             | 66 ++++++++-----------
 .../SemaCXX/type-traits-unsatisfied-diags.cpp | 63 +++++++++++++++++-
 2 files changed, 90 insertions(+), 39 deletions(-)

diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index be07ef5e8faf6..46f62c440349a 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2286,21 +2286,6 @@ 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()) {
@@ -2334,30 +2319,37 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
           << B.getSourceRange();
     }
   }
-  if (hasMixedAccessSpecifier(D)) {
-    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
-        << diag::TraitNotSatisfiedReason::MixedAccess;
+  // Check for mixed access specifiers in fields.
+  const FieldDecl *FirstField = nullptr;
+  AccessSpecifier FirstAccess = AS_none;
 
-    const FieldDecl *FirstField = nullptr;
-    AccessSpecifier FirstAcc = AS_none;
-    for (const FieldDecl *F : D->fields()) {
-      if (F->isUnnamedBitField())
-        continue;
-      if (FirstField == nullptr) {
-        FirstField = F;
-        FirstAcc = F->getAccess();
-      } else if (F->getAccess() != FirstAcc) {
-        SemaRef.Diag(FirstField->getLocation(), diag::note_defined_here)
-            << FirstField;
-
-        SemaRef.Diag(F->getLocation(), diag::note_unsatisfied_trait_reason)
-            << diag::TraitNotSatisfiedReason::MixedAccessField
-            << F           // %0: second field
-            << FirstField; // %1: first field
-        break;
-      }
+  for (const FieldDecl *Field : D->fields()) {
+    if (Field->isUnnamedBitField())
+      continue;
+
+    // Record the first field we see
+    if (!FirstField) {
+      FirstField = Field;
+      FirstAccess = Field->getAccess();
+      continue;
+    }
+
+    // Check if the field has a different access specifier than the first one.
+    if (Field->getAccess() != FirstAccess) {
+      // Emit a diagnostic about mixed access specifiers.
+      SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+          << diag::TraitNotSatisfiedReason::MixedAccess;
+
+      SemaRef.Diag(FirstField->getLocation(), diag::note_defined_here)
+          << FirstField;
+
+      SemaRef.Diag(Field->getLocation(), diag::note_unsatisfied_trait_reason)
+          << diag::TraitNotSatisfiedReason::MixedAccessField << Field
+          << FirstField;
+
+      // No need to check further fields, as we already found mixed access.
+      return;
     }
-    return;
   }
   if (hasMultipleDataBaseClassesWithFields(D)) {
     SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index 2c9f2cfa6f6cf..4ece45808e533 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-vla-cxx-extension -Wno-c++26-extensions -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-vla-cxx-extension -Wno-c++26-extensions -std=c++20 -fms-extensions %s
 
 struct S : A {}; // expected-error{{expected class name}}
 
@@ -573,4 +573,63 @@ struct BaseY {};                                              // #sl-BaseY
 struct MultiBase : BaseX, BaseY {};                          // #sl-MultiBase
 static_assert(__is_standard_layout(MultiBase));
 
- }
+struct A {
+  int x;
+};
+
+struct B : A {
+};
+// Indirect base with data members
+struct C : B { int y; }; // #sl-C
+static_assert(__is_standard_layout(C));
+// expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::C)'}} \
+// expected-note at -1 {{'C' is not standard-layout}} \
+// expected-note at -1 {{because it has an indirect base 'A' with data members}} \
+// expected-note@#sl-C {{'C' defined here}}
+
+struct D {
+    union { int a; float b; };
+  }; // #sl-D
+static_assert(__is_standard_layout(D)); // no diagnostics
+
+// E inherits D but adds a new member
+struct E : D { int x; }; // #sl-E
+static_assert(__is_standard_layout(E));
+// expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::E)'}} \
+// expected-note at -1 {{'E' is not standard-layout}} \
+// expected-note at -1 {{because it has an indirect base 'D' with data members}} \
+// expected-note@#sl-E {{'E' defined here}}
+
+// F inherits D but only an unnamed bitfield
+// This should still fail because F ends up with a 
+// base class with a data member and its own unnamed bitfield
+// which is not allowed in standard layout
+struct F : D { int : 0; }; // #sl-F
+static_assert(__is_standard_layout(F));
+// expected-error at -1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::F)'}} \
+// expected-note at -1 {{'F' is not standard-layout}} \
+// expected-note@#sl-F {{'F' defined here}}
+
+struct Empty {};
+struct G { Empty a, b; }; // #sl-G
+static_assert(__is_standard_layout(G)); // no diagnostics
+
+struct H { Empty a; int x; }; // #sl-H
+static_assert(__is_standard_layout(H)); // no diagnostics
+
+ struct I { Empty a; int : 0; int x; }; // #sl-I
+static_assert(__is_standard_layout(I)); // no diagnostics
+
+//  [[no_unique_address]] or [[msvc::no_unique_address]] should not affect standard layout
+#if __has_cpp_attribute(no_unique_address)
+  #define NO_UNIQUE_ADDRESS [[no_unique_address]]
+#elif __has_cpp_attribute(msvc::no_unique_address)
+  #define NO_UNIQUE_ADDRESS [[msvc::no_unique_address]]
+#elif defined(_MSC_VER)
+  #define NO_UNIQUE_ADDRESS __declspec(no_unique_address)
+#else
+  #define NO_UNIQUE_ADDRESS /* nothing */
+#endif
+struct J { NO_UNIQUE_ADDRESS Empty a; int x; };
+static_assert(__is_standard_layout(J)); // no diagnostics
+}

>From 5ca99e4acf7afa2463265d7b75d35fec5e9b37e6 Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Wed, 25 Jun 2025 09:07:28 -0400
Subject: [PATCH 8/9] Address review comments: minor changes to tablegen
 descriptions Revert note in ReleaseNotes Remove no_unique_address test

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td   |  4 ++--
 clang/lib/Sema/SemaTypeTraits.cpp                  |  4 ++--
 .../test/SemaCXX/type-traits-unsatisfied-diags.cpp | 14 +-------------
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ad93598888ce6..e3ab3d63a0779 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1788,12 +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}|"
+           "%NonStandardLayoutBase{has a non-standard-layout base %1}|"
            "%MixedAccess{has mixed access specifiers}|"
            "%MixedAccessField{field %1 has a different access specifier than field %2}|"
            "%MultipleDataBase{has multiple base classes with data members}|"
            "%VirtualFunction{has a virtual function}|"
-           "%NonStdLayoutMember{has a non-standard-layout member %1 of type %2}|"
+           "%NonStandardLayoutMember{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 "
diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 46f62c440349a..1611e588c9b17 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2315,7 +2315,7 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
     }
     if (!B.getType()->isStandardLayoutType()) {
       SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
-          << diag::TraitNotSatisfiedReason::NonStdLayoutBase << B.getType()
+          << diag::TraitNotSatisfiedReason::NonStandardLayoutBase << B.getType()
           << B.getSourceRange();
     }
   }
@@ -2369,7 +2369,7 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
   for (const FieldDecl *Field : D->fields()) {
     if (!Field->getType()->isStandardLayoutType()) {
       SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
-          << diag::TraitNotSatisfiedReason::NonStdLayoutMember << Field
+          << diag::TraitNotSatisfiedReason::NonStandardLayoutMember << Field
           << Field->getType() << Field->getSourceRange();
     }
   }
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index 4ece45808e533..56d974556365b 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -619,17 +619,5 @@ static_assert(__is_standard_layout(H)); // no diagnostics
 
  struct I { Empty a; int : 0; int x; }; // #sl-I
 static_assert(__is_standard_layout(I)); // no diagnostics
-
-//  [[no_unique_address]] or [[msvc::no_unique_address]] should not affect standard layout
-#if __has_cpp_attribute(no_unique_address)
-  #define NO_UNIQUE_ADDRESS [[no_unique_address]]
-#elif __has_cpp_attribute(msvc::no_unique_address)
-  #define NO_UNIQUE_ADDRESS [[msvc::no_unique_address]]
-#elif defined(_MSC_VER)
-  #define NO_UNIQUE_ADDRESS __declspec(no_unique_address)
-#else
-  #define NO_UNIQUE_ADDRESS /* nothing */
-#endif
-struct J { NO_UNIQUE_ADDRESS Empty a; int x; };
-static_assert(__is_standard_layout(J)); // no diagnostics
 }
+

>From 7d6b60899419a1f10509a15b5a0e58a487b8dfed Mon Sep 17 00:00:00 2001
From: Samarth Narang <snarang at umass.edu>
Date: Wed, 25 Jun 2025 09:21:15 -0400
Subject: [PATCH 9/9] The return should be a break

---
 clang/lib/Sema/SemaTypeTraits.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index d246e21eb11cd..ccf596edbd98e 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -2376,7 +2376,7 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
           << FirstField;
 
       // No need to check further fields, as we already found mixed access.
-      return;
+      break;
     }
   }
   if (hasMultipleDataBaseClassesWithFields(D)) {



More information about the cfe-commits mailing list