Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
8 changes: 6 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1777,7 +1777,8 @@ def note_unsatisfied_trait
"%Empty{empty}|"
"%StandardLayout{standard-layout}|"
"%Aggregate{aggregate}|"
"%Final{final}"
"%Final{final}|"
"%Abstract{abstract}"
"}1">;

def note_unsatisfied_trait_reason
Expand Down Expand Up @@ -1827,7 +1828,10 @@ def note_unsatisfied_trait_reason
"%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'}"
"%NotMarkedFinal{is not marked 'final'}|"
"%UnionType{is a union type}|"
"%NotStructOrClass{is not a struct or class type}|"
"%OverridesAllPureVirtual{overrides all pure virtual functions from base class %1}"
"}0">;

def warn_consteval_if_always_true : Warning<
Expand Down
64 changes: 64 additions & 0 deletions clang/lib/Sema/SemaTypeTraits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2014,6 +2014,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) {
.Case("is_aggregate", TypeTrait::UTT_IsAggregate)
.Case("is_constructible", TypeTrait::TT_IsConstructible)
.Case("is_final", TypeTrait::UTT_IsFinal)
.Case("is_abstract", TypeTrait::UTT_IsAbstract)
.Default(std::nullopt);
}

Expand Down Expand Up @@ -2774,6 +2775,66 @@ static void DiagnoseNonAggregateReason(Sema &SemaRef, SourceLocation Loc,
DiagnoseNonAggregateReason(SemaRef, Loc, D);
}

static void DiagnoseNonAbstractReason(Sema &SemaRef, SourceLocation Loc,
const CXXRecordDecl *D) {
// If this type has any abstract base classes, their respective virtual
// functions must have been overridden.
for (const CXXBaseSpecifier &B : D->bases()) {
assert(B.getType()->getAsCXXRecordDecl() && "invalid base?");
if (B.getType()->getAsCXXRecordDecl()->isAbstract()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::OverridesAllPureVirtual
<< B.getType() << B.getSourceRange();
}
}
}

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

if (T->isReferenceType()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::Ref;
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NotStructOrClass;
return;
}

if (T->isUnionType()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This case does not look covered in the tests. Always make sure you cover all the diagnostics and various branches.

SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::UnionType;
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NotStructOrClass;
return;
}

if (SemaRef.Context.getAsArrayType(T)) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NotStructOrClass;
return;
}

if (T->isFunctionType()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::FunctionType;
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NotStructOrClass;
return;
}

if (!T->isStructureOrClassType()) {
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NotStructOrClass;
return;
}

const CXXRecordDecl *D = T->getAsCXXRecordDecl();
if (D->hasDefinition())
DiagnoseNonAbstractReason(SemaRef, Loc, D);
}

void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
E = E->IgnoreParenImpCasts();
if (E->containsErrors())
Expand Down Expand Up @@ -2818,6 +2879,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
DiagnoseIsFinalReason(*this, E->getBeginLoc(), QT); // unsatisfied
break;
}
case UTT_IsAbstract:
DiagnoseNonAbstractReason(*this, E->getBeginLoc(), Args[0]);
break;
default:
break;
}
Expand Down
53 changes: 52 additions & 1 deletion clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ struct is_final {
template <typename T>
constexpr bool is_final_v = __is_final(T);

template <typename T>
struct is_abstract {
static constexpr bool value = __is_abstract(T);
};
template <typename T>
constexpr bool is_abstract_v = __is_abstract(T);

#endif

#ifdef STD2
Expand Down Expand Up @@ -151,6 +158,15 @@ using is_final = __details_is_final<T>;
template <typename T>
constexpr bool is_final_v = __is_final(T);

template <typename T>
struct __details_is_abstract {
static constexpr bool value = __is_abstract(T);
};
template <typename T>
using is_abstract = __details_is_abstract<T>;
template <typename T>
constexpr bool is_abstract_v = __is_abstract(T);

#endif


Expand Down Expand Up @@ -229,6 +245,13 @@ using is_final = __details_is_final<T>;
template <typename T>
constexpr bool is_final_v = is_final<T>::value;

template <typename T>
struct __details_is_abstract : bool_constant<__is_abstract(T)> {};
template <typename T>
using is_abstract = __details_is_abstract<T>;
template <typename T>
constexpr bool is_abstract_v = is_abstract<T>::value;

#endif
}

Expand Down Expand Up @@ -336,6 +359,22 @@ static_assert(std::is_aggregate_v<void>);
// expected-note@-1 {{'void' is not aggregate}} \
// expected-note@-1 {{because it is a cv void type}}


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

static_assert(std::is_abstract<int&>::value);
// expected-error-re@-1 {{static assertion failed due to requirement 'std::{{.*}}is_abstract<int &>::value'}} \
// expected-note@-1 {{'int &' is not abstract}} \
// expected-note@-1 {{because it is a reference type}} \
// expected-note@-1 {{because it is not a struct or class type}}

static_assert(std::is_abstract_v<int&>);
// expected-error@-1 {{static assertion failed due to requirement 'std::is_abstract_v<int &>'}} \
// expected-note@-1 {{'int &' is not abstract}} \
// expected-note@-1 {{because it is a reference type}} \
// expected-note@-1 {{because it is not a struct or class type}}


namespace test_namespace {
using namespace std;
static_assert(is_trivially_relocatable<int&>::value);
Expand Down Expand Up @@ -388,7 +427,7 @@ 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}} \
Expand Down Expand Up @@ -422,6 +461,18 @@ namespace test_namespace {
// expected-note@-1 {{'Fn' (aka 'void ()') is not final}} \
// expected-note@-1 {{because it is a function type}} \
// expected-note@-1 {{because it is not a class or union type}}

static_assert(is_abstract<int&>::value);
// expected-error-re@-1 {{static assertion failed due to requirement '{{.*}}is_abstract<int &>::value'}} \
// expected-note@-1 {{'int &' is not abstract}} \
// expected-note@-1 {{because it is a reference type}} \
// expected-note@-1 {{because it is not a struct or class type}}

static_assert(is_abstract_v<int&>);
// expected-error@-1 {{static assertion failed due to requirement 'is_abstract_v<int &>'}} \
// expected-note@-1 {{'int &' is not abstract}} \
// expected-note@-1 {{because it is a reference type}} \
// expected-note@-1 {{because it is not a struct or class type}}
}


Expand Down
76 changes: 76 additions & 0 deletions clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -964,3 +964,79 @@ namespace is_aggregate {

static_assert(__is_aggregate(S7[10]));
}

namespace is_abstract_tests {
struct Abstract1 {
virtual void fn1() = 0;
};

struct Abstract2 {
virtual void fn2() = 0;
};

struct NonAbstract
{
virtual void f() {}
};

// Multiple inheritance reports all abstract base classes that had their pure virtual functions overridden.
struct Overrides : Abstract1, Abstract2, NonAbstract {
void fn1() override {}
void fn2() override {}
};

static_assert(__is_abstract(Overrides));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(is_abstract_tests::Overrides)'}} \
// expected-note@-1 {{'Overrides' is not abstract}} \
// expected-note@-1 {{because it overrides all pure virtual functions from base class 'Abstract1'}} \
// expected-note@-1 {{because it overrides all pure virtual functions from base class 'Abstract2'}} \

// Inheriting over two levels reports the last class only although the source of the pure virtual function
// is the top-most base.
struct Derived : Abstract1 {
};

struct Derived2 : Derived {
void fn1() override {}
};

static_assert(__is_abstract(Derived2));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(is_abstract_tests::Derived2)'}} \
// expected-note@-1 {{'Derived2' is not abstract}} \
// expected-note@-1 {{because it overrides all pure virtual functions from base class 'Derived'}} \


using I = int;
static_assert(__is_abstract(I));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(int)'}} \
// expected-note@-1 {{'I' (aka 'int') is not abstract}} \
// expected-note@-1 {{because it is not a struct or class type}}

using Fty = void(); // function type
static_assert(__is_abstract(Fty));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(void ())'}} \
// expected-note@-1 {{'Fty' (aka 'void ()') is not abstract}} \
// expected-note@-1 {{because it is a function type}} \
// expected-note@-1 {{because it is not a struct or class type}}

using Arr = int[3];
static_assert(__is_abstract(Arr));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(int[3])'}} \
// expected-note@-1 {{'Arr' (aka 'int[3]') is not abstract}} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

is not abstract kind of feels weak considering we get a better message for functions and references. Same w/ the pointer case below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I was looking at is_final for ideas on what to check for. I added a nicer message for these cases. Would it be useful to extend the other diagnostics that are already implemented in the same way? I could do that in another PR once this one is merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can deal w/ this be done as a follow-up

// expected-note@-1 {{because it is not a struct or class type}}

using Ref = int&;
static_assert(__is_abstract(Ref));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(int &)'}} \
// expected-note@-1 {{'Ref' (aka 'int &') is not abstract}} \
// expected-note@-1 {{because it is a reference type}} \
// expected-note@-1 {{because it is not a struct or class type}}

using Ptr = int*;
static_assert(__is_abstract(Ptr));
// expected-error@-1 {{static assertion failed due to requirement '__is_abstract(int *)'}} \
// expected-note@-1 {{'Ptr' (aka 'int *') is not abstract}} \
// expected-note@-1 {{because it is not a struct or class type}}


}