[llvm-branch-commits] [clang] 7a94fc0 - PR44721: Don't consider overloaded operators for built-in comparisons

[Richard Smith via llvm-branch-commits]

Author: Richard Smith
Date: 2020-02-04T12:25:40-08:00
New Revision: 7a94fc09d17bc317032eb9605eba05dced8c87e5

URL: https://github.com/llvm/llvm-project/commit/7a94fc09d17bc317032eb9605eba05dced8c87e5
DIFF: https://github.com/llvm/llvm-project/commit/7a94fc09d17bc317032eb9605eba05dced8c87e5.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.

(cherry picked from commit 1f3f8c369a5067a132c871f33a955a7feaea8534)

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 19403e050850..831e55046e80 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;
 
@@ -7851,10 +7858,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();
 
@@ -7894,8 +7905,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;
+  };
+}