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
