[Clang] Explain why is_standard_layout evaluated to false

[snarang181]

As part of the effort in #141911

[llvmbot]

@llvm/pr-subscribers-clang

Author: Samarth Narang (snarang181)

Changes

As part of the effort in #141911

---

Full diff: https://github.com/llvm/llvm-project/pull/143722.diff

4 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7-1)
• (modified) clang/lib/Sema/SemaTypeTraits.cpp (+100)
• (modified) clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp (+54)
• (modified) clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp (+46)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0f77083dac9df..52221462de185 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,11 @@ 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}|"
            "%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..730fc77a34712 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,102 @@ 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();
+    }
+  }
+}
+
+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 +2393,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..cc10cd2405b4d 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,15 @@ 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 +89,15 @@ 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,9 +159,21 @@ 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}}
+    
 }
 
 
+
 namespace concepts {
 template <typename T>
 requires std::is_trivially_relocatable<T>::value void f();  // #cand1
@@ -192,3 +244,5 @@ static_assert(std::is_replaceable_v<int&>);
 // expected-error@-1 {{static assertion failed due to requirement 'std::is_replaceable_v<int &>'}} \
 // expected-note@-1 {{'int &' is not replaceable}} \
 // 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..65c376db7ea02 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -488,3 +488,49 @@ 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}}
+
+}

[snarang181]

@cor3ntin, requesting your review here.

[Copilot]

Pull Request Overview

Adds support for explaining why __is_standard_layout (and its <type_traits> equivalents) evaluates to false.

• Introduce new test cases for unsatisfied standard-layout trait in type-traits-unsatisfied-diags.cpp and its <type_traits> counterpart.
• Extend SemaTypeTraits implementation to diagnose all standard-layout violations.
• Update diagnostics definitions to include StandardLayout in DiagnosticSemaKinds.td.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp	New test cases for __is_standard_layout failures
clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp	Added is_standard_layout traits and std::is_standard_layout tests
clang/lib/Sema/SemaTypeTraits.cpp	Implemented DiagnoseNonStandardLayoutReason and helpers
clang/include/clang/Basic/DiagnosticSemaKinds.td	Added StandardLayout to unsatisfied-trait notes and reasons

Comments suppressed due to low confidence (2)

clang/lib/Sema/SemaTypeTraits.cpp:2295

• [nitpick] The function name 'hasMultipleDataBaseClassesWithFields' is a bit awkward—consider renaming to something like 'hasMultipleBasesWithDataMembers' for clarity and consistency.

static bool hasMultipleDataBaseClassesWithFields(const CXXRecordDecl *D) {

clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp:535

• No test covers the 'multiple base classes with data members' path; consider adding a case where a type derives from two bases each with data members to exercise the corresponding diagnostic.

// end of standard_layout_tests

[cor3ntin]

Thanks a lot for working on that.

Can you add tests for all of these examples?
https://eel.is/c++draft/class.prop#11

In particular

• If there are multiple direct base classes, a type is not a standard layout
• If in the inheritance hierarchy, there are multiple classes with members, it is not standard-layout.
  We have CXXRecordDecl::hasDirectFields() and CXXRecordDecl::getStandardLayoutBaseWithFields that should help

[snarang181]

Thanks a lot for working on that.

Can you add tests for all of these examples? https://eel.is/c++draft/class.prop#11

In particular

• If there are multiple direct base classes, a type is not a standard layout
• If in the inheritance hierarchy, there are multiple classes with members, it is not standard-layout.
  We have CXXRecordDecl::hasDirectFields() and CXXRecordDecl::getStandardLayoutBaseWithFields that should help

@cor3ntin, I added more inheritance related tests. Kindly review, thanks!

[cor3ntin]

I think this case needs more explanation.
maybe "because both BaseC and (an indirect) base class BaseA have data members"

[snarang181]

I have added more descriptive diagnostics. Note that I was not able to use CXXRecordDecl::getStandardLayoutBaseWithFields as the function asserts a standard layout to begin with. As a result, I had to manually walk the inheritance hierarchy to expose this.

[snarang181]

Something broke while resolving merge conflicts. Getting it back to a steady state and will mark it ready for review again. Apologies.

[snarang181]

Closing in favor of #144161