Skip to content

Commit

Permalink
PR44721: Don't consider overloaded operators for built-in comparisons
Browse files Browse the repository at this point in the history
when building a defaulted comparison.

As a convenient way of asking whether `x @ y` is valid and building it,
we previouly always performed overload resolution and built an
overloaded expression, which would both end up picking a builtin
operator candidate when given a non-overloadable type. But that's not
quite right, because it can result in our finding a user-declared
operator overload, which we should never do when applying operators
non-overloadable types.

Handle this more correctly: skip overload resolution when building
`x @ y` if the operands are not overloadable. But still perform overload
resolution (considering only builtin candidates) when checking validity,
as we don't have any other good way to ask whether a binary operator
expression would be valid.
  • Loading branch information
zygoloid committed Jan 31, 2020
1 parent d28763c commit 1f3f8c3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
29 changes: 22 additions & 7 deletions clang/lib/Sema/SemaDeclCXX.cpp
Expand Up @@ -7373,7 +7373,14 @@ class DefaultedComparisonAnalyzer
/// resolution [...]
CandidateSet.exclude(FD);

S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
if (Args[0]->getType()->isOverloadableType())
S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
else {
// FIXME: We determine whether this is a valid expression by checking to
// see if there's a viable builtin operator candidate for it. That isn't
// really what the rules ask us to do, but should give the right results.
S.AddBuiltinOperatorCandidates(OO, FD->getLocation(), Args, CandidateSet);
}

Result R;

Expand Down Expand Up @@ -7826,10 +7833,14 @@ class DefaultedComparisonSynthesizer
return StmtError();

OverloadedOperatorKind OO = FD->getOverloadedOperator();
ExprResult Op = S.CreateOverloadedBinOp(
Loc, BinaryOperator::getOverloadedOpcode(OO), Fns,
Obj.first.get(), Obj.second.get(), /*PerformADL=*/true,
/*AllowRewrittenCandidates=*/true, FD);
BinaryOperatorKind Opc = BinaryOperator::getOverloadedOpcode(OO);
ExprResult Op;
if (Type->isOverloadableType())
Op = S.CreateOverloadedBinOp(Loc, Opc, Fns, Obj.first.get(),
Obj.second.get(), /*PerformADL=*/true,
/*AllowRewrittenCandidates=*/true, FD);
else
Op = S.CreateBuiltinBinOp(Loc, Opc, Obj.first.get(), Obj.second.get());
if (Op.isInvalid())
return StmtError();

Expand Down Expand Up @@ -7869,8 +7880,12 @@ class DefaultedComparisonSynthesizer
llvm::APInt ZeroVal(S.Context.getIntWidth(S.Context.IntTy), 0);
Expr *Zero =
IntegerLiteral::Create(S.Context, ZeroVal, S.Context.IntTy, Loc);
ExprResult Comp = S.CreateOverloadedBinOp(Loc, BO_NE, Fns, VDRef.get(),
Zero, true, true, FD);
ExprResult Comp;
if (VDRef.get()->getType()->isOverloadableType())
Comp = S.CreateOverloadedBinOp(Loc, BO_NE, Fns, VDRef.get(), Zero, true,
true, FD);
else
Comp = S.CreateBuiltinBinOp(Loc, BO_NE, VDRef.get(), Zero);
if (Comp.isInvalid())
return StmtError();
Sema::ConditionResult Cond = S.ActOnCondition(
Expand Down
12 changes: 12 additions & 0 deletions clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
Expand Up @@ -190,3 +190,15 @@ bool operator<(const G&, const G&);
bool operator<=(const G&, const G&);
bool operator>(const G&, const G&);
bool operator>=(const G&, const G&);

namespace PR44721 {
template <typename T> bool operator==(T const &, T const &) { return true; }
template <typename T, typename U> bool operator!=(T const &, U const &) { return true; }
template <typename T> int operator<=>(T const &, T const &) { return 0; }

struct S {
friend bool operator==(const S &, const S &) = default;
friend bool operator<=>(const S &, const S &) = default;
int x;
};
}

0 comments on commit 1f3f8c3

Please sign in to comment.