<div dir="ltr">Thanks, llvmorg-10.0.0-rc1-35-g7a94fc09d17</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 3 Feb 2020 at 04:53, Hans Wennborg via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Yes, go ahead.<br>
<br>
On Fri, Jan 31, 2020 at 2:19 AM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
><br>
> Hi Hans,<br>
><br>
> This is a pretty safe bugfix for a new feature in Clang 10. OK for branch?<br>
><br>
> On Thu, 30 Jan 2020 at 17:17, Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>><br>
>> Author: Richard Smith<br>
>> Date: 2020-01-30T17:16:50-08:00<br>
>> New Revision: 1f3f8c369a5067a132c871f33a955a7feaea8534<br>
>><br>
>> URL: <a href="https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534</a><br>
>> DIFF: <a href="https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/1f3f8c369a5067a132c871f33a955a7feaea8534.diff</a><br>
>><br>
>> LOG: PR44721: Don't consider overloaded operators for built-in comparisons<br>
>> when building a defaulted comparison.<br>
>><br>
>> As a convenient way of asking whether `x @ y` is valid and building it,<br>
>> we previouly always performed overload resolution and built an<br>
>> overloaded expression, which would both end up picking a builtin<br>
>> operator candidate when given a non-overloadable type. But that's not<br>
>> quite right, because it can result in our finding a user-declared<br>
>> operator overload, which we should never do when applying operators<br>
>> non-overloadable types.<br>
>><br>
>> Handle this more correctly: skip overload resolution when building<br>
>> `x @ y` if the operands are not overloadable. But still perform overload<br>
>> resolution (considering only builtin candidates) when checking validity,<br>
>> as we don't have any other good way to ask whether a binary operator<br>
>> expression would be valid.<br>
>><br>
>> Added:<br>
>><br>
>><br>
>> Modified:<br>
>> clang/lib/Sema/SemaDeclCXX.cpp<br>
>> clang/test/CXX/class/class.compare/class.compare.default/p3.cpp<br>
>><br>
>> Removed:<br>
>><br>
>><br>
>><br>
>> ################################################################################<br>
>> diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp<br>
>> index 814a3c64eeba..65526e4020cf 100644<br>
>> --- a/clang/lib/Sema/SemaDeclCXX.cpp<br>
>> +++ b/clang/lib/Sema/SemaDeclCXX.cpp<br>
>> @@ -7373,7 +7373,14 @@ class DefaultedComparisonAnalyzer<br>
>> /// resolution [...]<br>
>> CandidateSet.exclude(FD);<br>
>><br>
>> - S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);<br>
>> + if (Args[0]->getType()->isOverloadableType())<br>
>> + S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);<br>
>> + else {<br>
>> + // FIXME: We determine whether this is a valid expression by checking to<br>
>> + // see if there's a viable builtin operator candidate for it. That isn't<br>
>> + // really what the rules ask us to do, but should give the right results.<br>
>> + S.AddBuiltinOperatorCandidates(OO, FD->getLocation(), Args, CandidateSet);<br>
>> + }<br>
>><br>
>> Result R;<br>
>><br>
>> @@ -7826,10 +7833,14 @@ class DefaultedComparisonSynthesizer<br>
>> return StmtError();<br>
>><br>
>> OverloadedOperatorKind OO = FD->getOverloadedOperator();<br>
>> - ExprResult Op = S.CreateOverloadedBinOp(<br>
>> - Loc, BinaryOperator::getOverloadedOpcode(OO), Fns,<br>
>> - Obj.first.get(), Obj.second.get(), /*PerformADL=*/true,<br>
>> - /*AllowRewrittenCandidates=*/true, FD);<br>
>> + BinaryOperatorKind Opc = BinaryOperator::getOverloadedOpcode(OO);<br>
>> + ExprResult Op;<br>
>> + if (Type->isOverloadableType())<br>
>> + Op = S.CreateOverloadedBinOp(Loc, Opc, Fns, Obj.first.get(),<br>
>> + Obj.second.get(), /*PerformADL=*/true,<br>
>> + /*AllowRewrittenCandidates=*/true, FD);<br>
>> + else<br>
>> + Op = S.CreateBuiltinBinOp(Loc, Opc, Obj.first.get(), Obj.second.get());<br>
>> if (Op.isInvalid())<br>
>> return StmtError();<br>
>><br>
>> @@ -7869,8 +7880,12 @@ class DefaultedComparisonSynthesizer<br>
>> llvm::APInt ZeroVal(S.Context.getIntWidth(S.Context.IntTy), 0);<br>
>> Expr *Zero =<br>
>> IntegerLiteral::Create(S.Context, ZeroVal, S.Context.IntTy, Loc);<br>
>> - ExprResult Comp = S.CreateOverloadedBinOp(Loc, BO_NE, Fns, VDRef.get(),<br>
>> - Zero, true, true, FD);<br>
>> + ExprResult Comp;<br>
>> + if (VDRef.get()->getType()->isOverloadableType())<br>
>> + Comp = S.CreateOverloadedBinOp(Loc, BO_NE, Fns, VDRef.get(), Zero, true,<br>
>> + true, FD);<br>
>> + else<br>
>> + Comp = S.CreateBuiltinBinOp(Loc, BO_NE, VDRef.get(), Zero);<br>
>> if (Comp.isInvalid())<br>
>> return StmtError();<br>
>> Sema::ConditionResult Cond = S.ActOnCondition(<br>
>><br>
>> 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<br>
>> index 3d0ab2c5bde6..81a48a393a06 100644<br>
>> --- a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp<br>
>> +++ b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp<br>
>> @@ -190,3 +190,15 @@ bool operator<(const G&, const G&);<br>
>> bool operator<=(const G&, const G&);<br>
>> bool operator>(const G&, const G&);<br>
>> bool operator>=(const G&, const G&);<br>
>> +<br>
>> +namespace PR44721 {<br>
>> + template <typename T> bool operator==(T const &, T const &) { return true; }<br>
>> + template <typename T, typename U> bool operator!=(T const &, U const &) { return true; }<br>
>> + template <typename T> int operator<=>(T const &, T const &) { return 0; }<br>
>> +<br>
>> + struct S {<br>
>> + friend bool operator==(const S &, const S &) = default;<br>
>> + friend bool operator<=>(const S &, const S &) = default;<br>
>> + int x;<br>
>> + };<br>
>> +}<br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>