Skip to content

[Clang] Add detailed notes explaining why is_aggregate evaluates to false #152488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Sep 3, 2025
Merged

[Clang] Add detailed notes explaining why is_aggregate evaluates to false #152488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5a49d52
merged
egorshamshura Aug 5, 2025
a6b90f3
Added diagnostic for is_aggregate.
egorshamshura Aug 7, 2025
f208801
Clang-formatted
egorshamshura Aug 7, 2025
850299b
Fixed inherited ctrs.
egorshamshura Aug 8, 2025
51ad2f0
Added llvm::any_of.
egorshamshura Aug 9, 2025
d65412a
Fixed problems.
egorshamshura Aug 18, 2025
ebe8337
Added array tests.
egorshamshura Aug 19, 2025
188200d
Merge branch 'main' into explain-is-aggregate_evaluated_false
egorshamshura Aug 22, 2025
f1b6862
resolved conflicts
egorshamshura Aug 22, 2025
c107f67
Swapped bases and fields diagnostics.
egorshamshura Aug 25, 2025
8d1ce5f
Added polymorphic check.
egorshamshura Aug 25, 2025
1ed4ad7
Changed diags order.
egorshamshura Aug 25, 2025
2e78ccf
Merge branch 'main' into explain-is-aggregate_evaluated_false
erichkeane Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1773,6 +1773,7 @@ def note_unsatisfied_trait
"%TriviallyCopyable{trivially copyable}|"
"%Empty{empty}|"
"%StandardLayout{standard-layout}|"
"%Aggregate{aggregate}|"
"%Final{final}"
"}1">;

Expand All @@ -1794,6 +1795,7 @@ def note_unsatisfied_trait_reason
"%NTCBase{has a non-trivially-copyable base %1}|"
"%NTCField{has a non-trivially-copyable member %1 of type %2}|"
"%NonEmptyMember{has a non-static data member %1 of type %2}|"
"%PolymorphicType{is a polymorphic type}|"
"%VirtualFunction{has a virtual function %1}|"
"%NonEmptyBase{has a base class %1 that is not empty}|"
"%NonZeroLengthField{field %1 is a non-zero-length bit-field}|"
Expand All @@ -1806,6 +1808,8 @@ def note_unsatisfied_trait_reason
"%DeletedDtr{has a %select{deleted|user-provided}1 destructor}|"
"%UserProvidedCtr{has a user provided %select{copy|move}1 "
"constructor}|"
"%UserDeclaredCtr{has a user-declared constructor}|"
"%InheritedCtr{has an inherited constructor}|"
"%DeletedCtr{has a deleted %select{copy|move}1 "
"constructor}|"
"%UserProvidedAssign{has a user provided %select{copy|move}1 "
Expand All @@ -1817,6 +1821,8 @@ def note_unsatisfied_trait_reason
"%FunctionType{is a function type}|"
"%CVVoidType{is a cv void type}|"
"%IncompleteArrayType{is an incomplete array type}|"
"%PrivateProtectedDirectDataMember{has a %select{private|protected}1 direct data member}|"
"%PrivateProtectedDirectBase{has a %select{private|protected}1 direct base}|"
"%NotClassOrUnion{is not a class or union type}|"
"%NotMarkedFinal{is not marked 'final'}"
"}0">;
Expand Down
92 changes: 92 additions & 0 deletions clang/lib/Sema/SemaTypeTraits.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/DiagnosticParse.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Basic/TypeTraits.h"
#include "clang/Sema/EnterExpressionEvaluationContext.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Overload.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaHLSL.h"
#include "llvm/ADT/STLExtras.h"

using namespace clang;

Expand Down Expand Up @@ -2009,6 +2011,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) {
.Case("is_assignable", TypeTrait::BTT_IsAssignable)
.Case("is_empty", TypeTrait::UTT_IsEmpty)
.Case("is_standard_layout", TypeTrait::UTT_IsStandardLayout)
.Case("is_aggregate", TypeTrait::UTT_IsAggregate)
.Case("is_constructible", TypeTrait::TT_IsConstructible)
.Case("is_final", TypeTrait::UTT_IsFinal)
.Default(std::nullopt);
Expand Down Expand Up @@ -2685,6 +2688,92 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D;
}

static void DiagnoseNonAggregateReason(Sema &SemaRef, SourceLocation Loc,
const CXXRecordDecl *D) {
for (const CXXConstructorDecl *Ctor : D->ctors()) {
if (Ctor->isUserProvided())
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::UserDeclaredCtr;
if (Ctor->isInheritingConstructor())
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::InheritedCtr;
}

if (llvm::any_of(D->decls(), [](auto const *Sub) {
return isa<ConstructorUsingShadowDecl>(Sub);
})) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::InheritedCtr;
}

if (D->isPolymorphic())
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::PolymorphicType
<< D->getSourceRange();

for (const CXXBaseSpecifier &B : D->bases()) {
Copy link
Contributor

@ojhunt ojhunt Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a nit: I feel like base classes should go prior to fields.

As an actual issue you may need to diagnose bases recursively as the reason for a type being a non-aggregate may not be direct. e.g. I'm not sure what diagnostic would be produced here for something like

class A {};
class B: private A {};
class C: public B {};

require is_aggregate(C)

Copy link
Contributor Author

@egorshamshura egorshamshura Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://godbolt.org/z/fsoP3h4on
I don't think that we need to check it recursively.
(1.3)

if (B.isVirtual()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::VBase << B.getType()
<< B.getSourceRange();
continue;
}
auto AccessSpecifier = B.getAccessSpecifier();
switch (AccessSpecifier) {
case AS_private:
case AS_protected:
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::PrivateProtectedDirectBase
<< (AccessSpecifier == AS_protected);
break;
default:
break;
}
}

for (const CXXMethodDecl *Method : D->methods()) {
Copy link
Contributor

@ojhunt ojhunt Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better noted as the object being polymorphic, rather than enumerating the individual virtual methods

Copy link
Contributor Author

@egorshamshura egorshamshura Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a TraitNotSatisfiedReason::Polymorphic, however, in my opinion, we also need to say why this class is being polymorphic and show every function that is being virtual.
What do you think about it?

Copy link
Collaborator

@erichkeane erichkeane Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect a double-note here is even better. First one that says "because it is a polymorphic type", second of: "check out this method here, which is virtual".

Copy link
Contributor Author

@egorshamshura egorshamshura Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect a double-note here is even better. First one that says "because it is a polymorphic type", second of: "check out this method here, which is virtual".

Added note about polymorphic type.

if (Method->isVirtual()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::VirtualFunction << Method
<< Method->getSourceRange();
}
}

for (const FieldDecl *Field : D->fields()) {
auto AccessSpecifier = Field->getAccess();
switch (AccessSpecifier) {
case AS_private:
case AS_protected:
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::PrivateProtectedDirectDataMember
<< (AccessSpecifier == AS_protected);
break;
default:
break;
}
}

SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D;
}

static void DiagnoseNonAggregateReason(Sema &SemaRef, SourceLocation Loc,
QualType T) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait)
<< T << diag::TraitName::Aggregate;

if (T->isVoidType())
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::CVVoidType;

T = T.getNonReferenceType();
const CXXRecordDecl *D = T->getAsCXXRecordDecl();
if (!D || D->isInvalidDecl())
return;

if (D->hasDefinition())
DiagnoseNonAggregateReason(SemaRef, Loc, D);
}

void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
E = E->IgnoreParenImpCasts();
if (E->containsErrors())
Expand Down Expand Up @@ -2717,6 +2806,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
case TT_IsConstructible:
DiagnoseNonConstructibleReason(*this, E->getBeginLoc(), Args);
break;
case UTT_IsAggregate:
DiagnoseNonAggregateReason(*this, E->getBeginLoc(), Args[0]);
break;
case UTT_IsFinal: {
QualType QT = Args[0];
if (QT->isDependentType())
Expand Down
81 changes: 75 additions & 6 deletions clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ struct is_constructible {
template <typename... Args>
constexpr bool is_constructible_v = __is_constructible(Args...);

template <typename T>
struct is_aggregate {
static constexpr bool value = __is_aggregate(T);
};

template <typename T>
constexpr bool is_aggregate_v = __is_aggregate(T);

template <typename T>
struct is_final {
static constexpr bool value = __is_final(T);
Expand Down Expand Up @@ -96,7 +104,7 @@ constexpr bool is_assignable_v = __is_assignable(T, U);

template <typename T>
struct __details_is_empty {
static constexpr bool value = __is_empty(T);
static constexpr bool value = __is_empty(T);
};
template <typename T>
using is_empty = __details_is_empty<T>;
Expand All @@ -105,9 +113,7 @@ constexpr bool is_empty_v = __is_empty(T);

template <typename T>
struct __details_is_standard_layout {
static constexpr bool value = __is_standard_layout(T);


static constexpr bool value = __is_standard_layout(T);
};
template <typename T>
using is_standard_layout = __details_is_standard_layout<T>;
Expand All @@ -125,6 +131,17 @@ using is_constructible = __details_is_constructible<Args...>;
template <typename... Args>
constexpr bool is_constructible_v = __is_constructible(Args...);

template <typename T>
struct __details_is_aggregate {
static constexpr bool value = __is_aggregate(T);
};

template <typename T>
using is_aggregate = __details_is_aggregate<T>;

template <typename T>
constexpr bool is_aggregate_v = __is_aggregate(T);

template <typename T>
struct __details_is_final {
static constexpr bool value = __is_final(T);
Expand Down Expand Up @@ -191,11 +208,20 @@ template <typename... Args>
struct __details_is_constructible : bool_constant<__is_constructible(Args...)> {};

template <typename... Args>
using is_constructible = __details_is_constructible<Args...>;
using is_constructible = __details_is_constructible<Args...>;

template <typename... Args>
constexpr bool is_constructible_v = is_constructible<Args...>::value;

template <typename T>
struct __details_is_aggregate : bool_constant<__is_aggregate(T)> {};

template <typename T>
using is_aggregate = __details_is_aggregate<T>;

template <typename T>
constexpr bool is_aggregate_v = is_aggregate<T>::value;

template <typename T>
struct __details_is_final : bool_constant<__is_final(T)> {};
template <typename T>
Expand All @@ -204,7 +230,6 @@ template <typename T>
constexpr bool is_final_v = is_final<T>::value;

#endif

}

static_assert(std::is_trivially_relocatable<int>::value);
Expand Down Expand Up @@ -299,6 +324,18 @@ static_assert(std::is_final_v<Arr>);
// expected-note@-1 {{'int[3]' is not final}} \
// expected-note@-1 {{because it is not a class or union type}}


static_assert(!std::is_aggregate<int>::value);

static_assert(std::is_aggregate<void>::value);
// expected-error-re@-1 {{static assertion failed due to requirement 'std::{{.*}}is_aggregate<void>::value'}} \
// expected-note@-1 {{'void' is not aggregate}} \
// expected-note@-1 {{because it is a cv void type}}
static_assert(std::is_aggregate_v<void>);
// expected-error@-1 {{static assertion failed due to requirement 'std::is_aggregate_v<void>'}} \
// expected-note@-1 {{'void' is not aggregate}} \
// expected-note@-1 {{because it is a cv void type}}

namespace test_namespace {
using namespace std;
static_assert(is_trivially_relocatable<int&>::value);
Expand Down Expand Up @@ -351,6 +388,15 @@ namespace test_namespace {
static_assert(is_constructible_v<void>);
// expected-error@-1 {{static assertion failed due to requirement 'is_constructible_v<void>'}} \
// expected-note@-1 {{because it is a cv void type}}

static_assert(std::is_aggregate<void>::value);
// expected-error-re@-1 {{static assertion failed due to requirement 'std::{{.*}}is_aggregate<void>::value'}} \
// expected-note@-1 {{'void' is not aggregate}} \
// expected-note@-1 {{because it is a cv void type}}
static_assert(std::is_aggregate_v<void>);
// expected-error@-1 {{static assertion failed due to requirement 'std::is_aggregate_v<void>'}} \
// expected-note@-1 {{'void' is not aggregate}} \
// expected-note@-1 {{because it is a cv void type}}

static_assert(is_final<int&>::value);
// expected-error-re@-1 {{static assertion failed due to requirement '{{.*}}is_final<int &>::value'}} \
Expand Down Expand Up @@ -413,6 +459,14 @@ concept C3 = std::is_constructible_v<Args...>; // #concept6
template <C3 T> void g3(); // #cand6


template <typename T>
requires std::is_aggregate<T>::value void f5(); // #cand9

template <typename T>
concept C5 = std::is_aggregate_v<T>; // #concept10

template <C5 T> void g5(); // #cand10

void test() {
f<int&>();
// expected-error@-1 {{no matching function for call to 'f'}} \
Expand Down Expand Up @@ -469,6 +523,21 @@ void test() {
// expected-note@#cand6 {{because 'void' does not satisfy 'C3'}} \
// expected-note@#concept6 {{because 'std::is_constructible_v<void>' evaluated to false}} \
// expected-note@#concept6 {{because it is a cv void type}}

f5<void>();
// expected-error@-1 {{no matching function for call to 'f5'}} \
// expected-note@#cand9 {{candidate template ignored: constraints not satisfied [with T = void]}} \
// expected-note-re@#cand9 {{because '{{.*}}is_aggregate<void>::value' evaluated to false}} \
// expected-note@#cand9 {{'void' is not aggregate}} \
// expected-note@#cand9 {{because it is a cv void type}}

g5<void>();
// expected-error@-1 {{no matching function for call to 'g5'}} \
// expected-note@#cand10 {{candidate template ignored: constraints not satisfied [with T = void]}} \
// expected-note@#cand10 {{because 'void' does not satisfy 'C5'}} \
// expected-note@#concept10 {{because 'std::is_aggregate_v<void>' evaluated to false}} \
// expected-note@#concept10 {{'void' is not aggregate}} \
// expected-note@#concept10 {{because it is a cv void type}}
}
}

Expand Down
Loading