Skip to content

Commit 5b36348

Browse files
philnik777philnik777
authored
[Clang] Fix __is_trivially_equality_comparable returning true with ineligebile defaulted overloads (#93113)
This changes `__is_trivially_equality_comparable` to do overload resolution instead, which fixes a couple of false-positives (and a false-negative as a drive-by). Fixes #89293
1 parent 4134b33 commit 5b36348

File tree

5 files changed

+124
-65
lines changed

5 files changed

+124
-65
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ ABI Changes in This Version
104104
ifuncs. Its purpose was to preserve backwards compatibility when the ".ifunc"
105105
suffix got removed from the name mangling. The alias interacts badly with
106106
GlobalOpt (see the issue #96197).
107-
107+
108108
- Fixed Microsoft name mangling for auto non-type template arguments of pointer
109109
type for MSVC 1920+. This change resolves incompatibilities with code compiled
110110
by MSVC 1920+ but will introduce incompatibilities with code compiled by
@@ -740,6 +740,9 @@ Bug Fixes in This Version
740740
negatives where the analysis failed to detect unchecked access to guarded
741741
data.
742742

743+
- ``__is_trivially_equality_comparable`` no longer returns true for types which
744+
have a constrained defaulted comparison operator (#GH89293).
745+
743746
Bug Fixes to Compiler Builtins
744747
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
745748

‎clang/include/clang/AST/Type.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,9 +1142,6 @@ class QualType {
11421142
/// Return true if this is a trivially relocatable type.
11431143
bool isTriviallyRelocatableType(const ASTContext &Context) const;
11441144

1145-
/// Return true if this is a trivially equality comparable type.
1146-
bool isTriviallyEqualityComparableType(const ASTContext &Context) const;
1147-
11481145
/// Returns true if it is a class and it might be dynamic.
11491146
bool mayBeDynamicClass() const;
11501147

‎clang/lib/AST/Type.cpp‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2815,66 +2815,6 @@ bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
28152815
}
28162816
}
28172817

2818-
static bool
2819-
HasNonDeletedDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
2820-
if (Decl->isUnion())
2821-
return false;
2822-
if (Decl->isLambda())
2823-
return Decl->isCapturelessLambda();
2824-
2825-
auto IsDefaultedOperatorEqualEqual = [&](const FunctionDecl *Function) {
2826-
return Function->getOverloadedOperator() ==
2827-
OverloadedOperatorKind::OO_EqualEqual &&
2828-
Function->isDefaulted() && Function->getNumParams() > 0 &&
2829-
(Function->getParamDecl(0)->getType()->isReferenceType() ||
2830-
Decl->isTriviallyCopyable());
2831-
};
2832-
2833-
if (llvm::none_of(Decl->methods(), IsDefaultedOperatorEqualEqual) &&
2834-
llvm::none_of(Decl->friends(), [&](const FriendDecl *Friend) {
2835-
if (NamedDecl *ND = Friend->getFriendDecl()) {
2836-
return ND->isFunctionOrFunctionTemplate() &&
2837-
IsDefaultedOperatorEqualEqual(ND->getAsFunction());
2838-
}
2839-
return false;
2840-
}))
2841-
return false;
2842-
2843-
return llvm::all_of(Decl->bases(),
2844-
[](const CXXBaseSpecifier &BS) {
2845-
if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
2846-
return HasNonDeletedDefaultedEqualityComparison(RD);
2847-
return true;
2848-
}) &&
2849-
llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
2850-
auto Type = FD->getType();
2851-
if (Type->isArrayType())
2852-
Type = Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
2853-
2854-
if (Type->isReferenceType() || Type->isEnumeralType())
2855-
return false;
2856-
if (const auto *RD = Type->getAsCXXRecordDecl())
2857-
return HasNonDeletedDefaultedEqualityComparison(RD);
2858-
return true;
2859-
});
2860-
}
2861-
2862-
bool QualType::isTriviallyEqualityComparableType(
2863-
const ASTContext &Context) const {
2864-
QualType CanonicalType = getCanonicalType();
2865-
if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
2866-
CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
2867-
return false;
2868-
2869-
if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
2870-
if (!HasNonDeletedDefaultedEqualityComparison(RD))
2871-
return false;
2872-
}
2873-
2874-
return Context.hasUniqueObjectRepresentations(
2875-
CanonicalType, /*CheckIfTriviallyCopyable=*/false);
2876-
}
2877-
28782818
bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
28792819
return !Context.getLangOpts().ObjCAutoRefCount &&
28802820
Context.getLangOpts().ObjCWeak &&

‎clang/lib/Sema/SemaExprCXX.cpp‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5201,6 +5201,82 @@ static bool HasNoThrowOperator(const RecordType *RT, OverloadedOperatorKind Op,
52015201
return false;
52025202
}
52035203

5204+
static bool
5205+
HasNonDeletedDefaultedEqualityComparison(Sema &S, const CXXRecordDecl *Decl) {
5206+
if (Decl->isUnion())
5207+
return false;
5208+
if (Decl->isLambda())
5209+
return Decl->isCapturelessLambda();
5210+
5211+
{
5212+
EnterExpressionEvaluationContext UnevaluatedContext(
5213+
S, Sema::ExpressionEvaluationContext::Unevaluated);
5214+
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
5215+
Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
5216+
5217+
// const ClassT& obj;
5218+
OpaqueValueExpr Operand(
5219+
{}, Decl->getTypeForDecl()->getCanonicalTypeUnqualified().withConst(),
5220+
ExprValueKind::VK_LValue);
5221+
UnresolvedSet<16> Functions;
5222+
// obj == obj;
5223+
S.LookupBinOp(S.TUScope, {}, BinaryOperatorKind::BO_EQ, Functions);
5224+
5225+
auto Result = S.CreateOverloadedBinOp({}, BinaryOperatorKind::BO_EQ,
5226+
Functions, &Operand, &Operand);
5227+
if (Result.isInvalid() || SFINAE.hasErrorOccurred())
5228+
return false;
5229+
5230+
const auto *CallExpr = dyn_cast<CXXOperatorCallExpr>(Result.get());
5231+
if (!CallExpr)
5232+
return false;
5233+
const auto *Callee = CallExpr->getDirectCallee();
5234+
auto ParamT = Callee->getParamDecl(0)->getType();
5235+
if (!Callee->isDefaulted())
5236+
return false;
5237+
if (!ParamT->isReferenceType() && !Decl->isTriviallyCopyable())
5238+
return false;
5239+
if (ParamT.getNonReferenceType()->getUnqualifiedDesugaredType() !=
5240+
Decl->getTypeForDecl())
5241+
return false;
5242+
}
5243+
5244+
return llvm::all_of(Decl->bases(),
5245+
[&](const CXXBaseSpecifier &BS) {
5246+
if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
5247+
return HasNonDeletedDefaultedEqualityComparison(S,
5248+
RD);
5249+
return true;
5250+
}) &&
5251+
llvm::all_of(Decl->fields(), [&](const FieldDecl *FD) {
5252+
auto Type = FD->getType();
5253+
if (Type->isArrayType())
5254+
Type = Type->getBaseElementTypeUnsafe()
5255+
->getCanonicalTypeUnqualified();
5256+
5257+
if (Type->isReferenceType() || Type->isEnumeralType())
5258+
return false;
5259+
if (const auto *RD = Type->getAsCXXRecordDecl())
5260+
return HasNonDeletedDefaultedEqualityComparison(S, RD);
5261+
return true;
5262+
});
5263+
}
5264+
5265+
static bool isTriviallyEqualityComparableType(Sema &S, QualType Type) {
5266+
QualType CanonicalType = Type.getCanonicalType();
5267+
if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
5268+
CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
5269+
return false;
5270+
5271+
if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
5272+
if (!HasNonDeletedDefaultedEqualityComparison(S, RD))
5273+
return false;
5274+
}
5275+
5276+
return S.getASTContext().hasUniqueObjectRepresentations(
5277+
CanonicalType, /*CheckIfTriviallyCopyable=*/false);
5278+
}
5279+
52045280
static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
52055281
SourceLocation KeyLoc,
52065282
TypeSourceInfo *TInfo) {
@@ -5633,7 +5709,7 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
56335709
Self.Diag(KeyLoc, diag::err_builtin_pass_in_regs_non_class) << T;
56345710
return false;
56355711
case UTT_IsTriviallyEqualityComparable:
5636-
return T.isTriviallyEqualityComparableType(C);
5712+
return isTriviallyEqualityComparableType(Self, T);
56375713
}
56385714
}
56395715

‎clang/test/SemaCXX/type-traits.cpp‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3677,6 +3677,12 @@ struct NonTriviallyEqualityComparableNoComparator {
36773677
};
36783678
static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableNoComparator));
36793679

3680+
struct NonTriviallyEqualityComparableConvertibleToBuiltin {
3681+
int i;
3682+
operator unsigned() const;
3683+
};
3684+
static_assert(!__is_trivially_equality_comparable(NonTriviallyEqualityComparableConvertibleToBuiltin));
3685+
36803686
struct NonTriviallyEqualityComparableNonDefaultedComparator {
36813687
int i;
36823688
int j;
@@ -3885,8 +3891,45 @@ struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
38853891

38863892
bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = default;
38873893
};
3894+
38883895
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
38893896

3897+
template<bool B>
3898+
struct MaybeTriviallyEqualityComparable {
3899+
int i;
3900+
bool operator==(const MaybeTriviallyEqualityComparable&) const requires B = default;
3901+
bool operator==(const MaybeTriviallyEqualityComparable& rhs) const { return (i % 3) == (rhs.i % 3); }
3902+
};
3903+
static_assert(__is_trivially_equality_comparable(MaybeTriviallyEqualityComparable<true>));
3904+
static_assert(!__is_trivially_equality_comparable(MaybeTriviallyEqualityComparable<false>));
3905+
3906+
struct NotTriviallyEqualityComparableMoreConstrainedExternalOp {
3907+
int i;
3908+
bool operator==(const NotTriviallyEqualityComparableMoreConstrainedExternalOp&) const = default;
3909+
};
3910+
3911+
bool operator==(const NotTriviallyEqualityComparableMoreConstrainedExternalOp&,
3912+
const NotTriviallyEqualityComparableMoreConstrainedExternalOp&) __attribute__((enable_if(true, ""))) {}
3913+
3914+
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableMoreConstrainedExternalOp));
3915+
3916+
struct TriviallyEqualityComparableExternalDefaultedOp {
3917+
int i;
3918+
friend bool operator==(TriviallyEqualityComparableExternalDefaultedOp, TriviallyEqualityComparableExternalDefaultedOp);
3919+
};
3920+
bool operator==(TriviallyEqualityComparableExternalDefaultedOp, TriviallyEqualityComparableExternalDefaultedOp) = default;
3921+
3922+
static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableExternalDefaultedOp));
3923+
3924+
struct EqualityComparableBase {
3925+
bool operator==(const EqualityComparableBase&) const = default;
3926+
};
3927+
3928+
struct ComparingBaseOnly : EqualityComparableBase {
3929+
int j_ = 0;
3930+
};
3931+
static_assert(!__is_trivially_equality_comparable(ComparingBaseOnly));
3932+
38903933
namespace hidden_friend {
38913934

38923935
struct TriviallyEqualityComparable {

0 commit comments

Comments
 (0)