Skip to content

[Clang] Added explanation why is_trivially_copyable evaluated to false. #142341

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.

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 10 commits into from
Jun 7, 2025
Merged
9 changes: 6 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1764,7 +1764,8 @@ def err_user_defined_msg_constexpr : Error<

// Type traits explanations
def note_unsatisfied_trait : Note<"%0 is not %enum_select<TraitName>{"
"%TriviallyRelocatable{trivially relocatable}"
"%TriviallyRelocatable{trivially relocatable}|"
"%TriviallyCopyable{trivially copyable}"
"}1">;

def note_unsatisfied_trait_reason
Expand All @@ -1774,8 +1775,10 @@ def note_unsatisfied_trait_reason
"%HasArcLifetime{has an ARC lifetime qualifier}|"
"%VLA{is a variably-modified type}|"
"%VBase{has a virtual base %1}|"
"%NRBase{has a non-trivially-relocatable base %1}|"
"%NRField{has a non-trivially-relocatable member %1 of type %2}|"
"%NTRBase{has a non-trivially-relocatable base %1}|"
"%NTRField{has a non-trivially-relocatable member %1 of type %2}|"
"%NTCBase{has a non-trivially-copyable base %1}|"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The enum names here are inconsistent... non-trivially-relocatable took only NR whereas this is taking NTC. That said, I VASTLY prefer the T be present. Not in the scope here of course, but a followup tochange those NR to NTR perhaps is valuable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed NR to NTR

"%NTCField{has a non-trivially-copyable member %1 of type %2}|"
"%DeletedDtr{has a %select{deleted|user-provided}1 destructor}|"
"%UserProvidedCtr{has a user provided %select{copy|move}1 "
"constructor}|"
Expand Down
88 changes: 85 additions & 3 deletions clang/lib/Sema/SemaTypeTraits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
//===----------------------------------------------------------------------===//

#include "clang/AST/DeclCXX.h"
#include "clang/AST/Type.h"
#include "clang/Basic/DiagnosticParse.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/TypeTraits.h"
#include "clang/Sema/EnterExpressionEvaluationContext.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
Expand Down Expand Up @@ -1938,6 +1940,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) {
return llvm::StringSwitch<std::optional<TypeTrait>>(Name)
.Case("is_trivially_relocatable",
TypeTrait::UTT_IsCppTriviallyRelocatable)
.Case("is_trivially_copyable", TypeTrait::UTT_IsTriviallyCopyable)
Copy link
Collaborator

Choose a reason for hiding this comment

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

2 isnt the number, but at a certain number of entries here, we're going to want to tablegen/.def file this. Perhaps in TokenKinds.def?

Copy link
Contributor Author

@egorshamshura egorshamshura Jun 2, 2025

Choose a reason for hiding this comment

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

I'm not sure if I did it right, but I couldn't figure out how from TYPE_TRAIT_1 could get a name without the "__" prefix. So I made a TYPE_TRAIT_DIAG for this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, no, I was suggesting adding a new field to the TYPE_TRAIT_1/TYPE_TRAIT_N/etc variants. It does end up being a little bit of extra work, which is why I was mentioning it more as a followup/future direction here instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I revert changes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I'd suggest going back to the original, unless you want to do all the work for updating the TYPE_TRAIT_N/TYPE_TRAIT_1 (which I'm not requiring here at all!). Just something to think about for a future direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, thanks)

Copy link
Contributor

Choose a reason for hiding this comment

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

@erichkeane yeah, maybe we should move the whole type traits thing to a tablegen file at some point... but it seems involved

.Default(std::nullopt);
}

Expand Down Expand Up @@ -2013,15 +2016,15 @@ static void DiagnoseNonTriviallyRelocatableReason(Sema &SemaRef,
<< B.getSourceRange();
if (!SemaRef.IsCXXTriviallyRelocatableType(B.getType()))
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NRBase << B.getType()
<< diag::TraitNotSatisfiedReason::NTRBase << B.getType()
<< B.getSourceRange();
}
for (const FieldDecl *Field : D->fields()) {
if (!Field->getType()->isReferenceType() &&
!SemaRef.IsCXXTriviallyRelocatableType(Field->getType()))
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NRField << Field << Field->getType()
<< Field->getSourceRange();
<< diag::TraitNotSatisfiedReason::NTRField << Field
<< Field->getType() << Field->getSourceRange();
}
if (D->hasDeletedDestructor())
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
Expand Down Expand Up @@ -2099,6 +2102,82 @@ static void DiagnoseNonTriviallyRelocatableReason(Sema &SemaRef,
SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D;
}

static void DiagnoseNonTriviallyCopyableReason(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().isTriviallyCopyableType(D->getASTContext())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this if statement covered in the tests 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.

Added test for it

SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NTCBase << B.getType()
<< B.getSourceRange();
}
}
for (const FieldDecl *Field : D->fields()) {
if (!Field->getType().isTriviallyCopyableType(Field->getASTContext()))
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::NTCField << Field
<< Field->getType() << Field->getSourceRange();
}
CXXDestructorDecl *Dtr = D->getDestructor();
if (D->hasDeletedDestructor() || (Dtr && !Dtr->isTrivial()))
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< diag::TraitNotSatisfiedReason::DeletedDtr
<< !D->hasDeletedDestructor() << D->getDestructor()->getSourceRange();

for (const CXXMethodDecl *Method : D->methods()) {
if (Method->isTrivial() || !Method->isUserProvided()) {
continue;
}
auto SpecialMemberKind =
SemaRef.getDefaultedFunctionKind(Method).asSpecialMember();
switch (SpecialMemberKind) {
case CXXSpecialMemberKind::CopyConstructor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

If every single case label covered in the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests for every single case

case CXXSpecialMemberKind::MoveConstructor:
case CXXSpecialMemberKind::CopyAssignment:
case CXXSpecialMemberKind::MoveAssignment: {
bool IsAssignment =
SpecialMemberKind == CXXSpecialMemberKind::CopyAssignment ||
SpecialMemberKind == CXXSpecialMemberKind::MoveAssignment;
bool IsMove =
SpecialMemberKind == CXXSpecialMemberKind::MoveConstructor ||
SpecialMemberKind == CXXSpecialMemberKind::MoveAssignment;

SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
<< (IsAssignment ? diag::TraitNotSatisfiedReason::UserProvidedAssign
: diag::TraitNotSatisfiedReason::UserProvidedCtr)
<< IsMove << Method->getSourceRange();
break;
}
default:
break;
}
}
}

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

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

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

if (D->hasDefinition())
DiagnoseNonTriviallyCopyableReason(SemaRef, Loc, D);

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

void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
E = E->IgnoreParenImpCasts();
if (E->containsErrors())
Expand All @@ -2113,6 +2192,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
case UTT_IsCppTriviallyRelocatable:
DiagnoseNonTriviallyRelocatableReason(*this, E->getBeginLoc(), Args[0]);
break;
case UTT_IsTriviallyCopyable:
DiagnoseNonTriviallyCopyableReason(*this, E->getBeginLoc(), Args[0]);
break;
default:
break;
}
Expand Down
72 changes: 72 additions & 0 deletions clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ struct is_trivially_relocatable {

template <typename T>
constexpr bool is_trivially_relocatable_v = __builtin_is_cpp_trivially_relocatable(T);

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

template <typename T>
constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T);
#endif

#ifdef STD2
Expand All @@ -25,6 +33,17 @@ using is_trivially_relocatable = __details_is_trivially_relocatable<T>;

template <typename T>
constexpr bool is_trivially_relocatable_v = __builtin_is_cpp_trivially_relocatable(T);

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

template <typename T>
using is_trivially_copyable = __details_is_trivially_copyable<T>;

template <typename T>
constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T);
#endif


Expand All @@ -45,6 +64,15 @@ using is_trivially_relocatable = __details_is_trivially_relocatable<T>;

template <typename T>
constexpr bool is_trivially_relocatable_v = is_trivially_relocatable<T>::value;

template <typename T>
struct __details_is_trivially_copyable : bool_constant<__is_trivially_copyable(T)> {};

template <typename T>
using is_trivially_copyable = __details_is_trivially_copyable<T>;

template <typename T>
constexpr bool is_trivially_copyable_v = is_trivially_copyable<T>::value;
#endif

}
Expand All @@ -60,6 +88,18 @@ static_assert(std::is_trivially_relocatable_v<int&>);
// expected-note@-1 {{'int &' is not trivially relocatable}} \
// expected-note@-1 {{because it is a reference type}}

static_assert(std::is_trivially_copyable<int>::value);

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


namespace test_namespace {
using namespace std;
static_assert(is_trivially_relocatable<int&>::value);
Expand All @@ -70,6 +110,15 @@ namespace test_namespace {
// expected-error@-1 {{static assertion failed due to requirement 'is_trivially_relocatable_v<int &>'}} \
// expected-note@-1 {{'int &' is not trivially relocatable}} \
// expected-note@-1 {{because it is a reference type}}

static_assert(is_trivially_copyable<int&>::value);
// expected-error-re@-1 {{static assertion failed due to requirement '{{.*}}is_trivially_copyable<int &>::value'}} \
// expected-note@-1 {{'int &' is not trivially copyable}} \
// expected-note@-1 {{because it is a reference type}}
static_assert(is_trivially_copyable_v<int&>);
// 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}}
}


Expand All @@ -82,6 +131,14 @@ concept C = std::is_trivially_relocatable_v<T>; // #concept2

template <C T> void g(); // #cand2

template <typename T>
requires std::is_trivially_copyable<T>::value void f2(); // #cand3

template <typename T>
concept C2 = std::is_trivially_copyable_v<T>; // #concept4

template <C2 T> void g2(); // #cand4

void test() {
f<int&>();
// expected-error@-1 {{no matching function for call to 'f'}} \
Expand All @@ -97,5 +154,20 @@ void test() {
// expected-note@#concept2 {{because 'std::is_trivially_relocatable_v<int &>' evaluated to false}} \
// expected-note@#concept2 {{'int &' is not trivially relocatable}} \
// expected-note@#concept2 {{because it is a reference type}}

f2<int&>();
// expected-error@-1 {{no matching function for call to 'f2'}} \
// expected-note@#cand3 {{candidate template ignored: constraints not satisfied [with T = int &]}} \
// expected-note-re@#cand3 {{because '{{.*}}is_trivially_copyable<int &>::value' evaluated to false}} \
// expected-note@#cand3 {{'int &' is not trivially copyable}} \
// expected-note@#cand3 {{because it is a reference type}}

g2<int&>();
// expected-error@-1 {{no matching function for call to 'g2'}} \
// expected-note@#cand4 {{candidate template ignored: constraints not satisfied [with T = int &]}} \
// expected-note@#cand4 {{because 'int &' does not satisfy 'C2'}} \
// expected-note@#concept4 {{because 'std::is_trivially_copyable_v<int &>' evaluated to false}} \
// expected-note@#concept4 {{'int &' is not trivially copyable}} \
// expected-note@#concept4 {{because it is a reference type}}
}
}
Loading
Loading