Yes, go ahead.
On Fri, Jan 31, 2020 at 2:19 AM Richard Smith <rich...@metafoo.co.uk> wrote: > > Hi Hans, > > This is a pretty safe bugfix for a new feature in Clang 10. OK for branch? > > On Thu, 30 Jan 2020 at 17:17, Richard Smith via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> >> Author: Richard Smith >> Date: 2020-01-30T17:16:50-08:00 >> New Revision: 1f3f8c369a5067a132c871f33a955a7feaea8534 >> >> URL: >> https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534 >> DIFF: >> https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534.diff >> >> LOG: PR44721: Don't consider overloaded operators for built-in comparisons >> 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. >> >> Added: >> >> >> Modified: >> clang/lib/Sema/SemaDeclCXX.cpp >> clang/test/CXX/class/class.compare/class.compare.default/p3.cpp >> >> Removed: >> >> >> >> ################################################################################ >> diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp >> index 814a3c64eeba..65526e4020cf 100644 >> --- a/clang/lib/Sema/SemaDeclCXX.cpp >> +++ b/clang/lib/Sema/SemaDeclCXX.cpp >> @@ -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; >> >> @@ -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(); >> >> @@ -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( >> >> diff --git >> a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp >> b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp >> index 3d0ab2c5bde6..81a48a393a06 100644 >> --- a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp >> +++ b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp >> @@ -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; >> + }; >> +} >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits