[clang] 1f3f8c3 - PR44721: Don't consider overloaded operators for built-in comparisons
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 4 12:39:00 PST 2020
Thanks, llvmorg-10.0.0-rc1-35-g7a94fc09d17
On Mon, 3 Feb 2020 at 04:53, Hans Wennborg via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Yes, go ahead.
>
> On Fri, Jan 31, 2020 at 2:19 AM Richard Smith <richard at 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 at 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 at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200204/2ab3b181/attachment.html>
More information about the cfe-commits
mailing list