Clarify the logic for when to build an overloaded binop. In particular,

build one when either of the operands calls itself type-dependent;
previously we were building when one of the operand types was dependent,
which is not always the same thing and which can lead to unfortunate
inconsistencies later.  Fixes PR8739.

llvm-svn: 120990

4 files changed, 44 insertions, 19 deletions

diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 1afc7602fb9..a43bea298e3 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -515,7 +515,7 @@ static Cl::ModifiableType IsModifiable(ASTContext &Ctx, const Expr *E,
 
   // Records with any const fields (recursively) are not modifiable.
   if (const RecordType *R = CT->getAs<RecordType>()) {
-    assert((isa<ObjCPropertyRefExpr>(E) ||
+    assert((E->getObjectKind() == OK_ObjCProperty ||
             !Ctx.getLangOptions().CPlusPlus) &&
            "C++ struct assignment should be resolved by the "
            "copy assignment operator.");
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3d6d59a406a..34965e6267d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7684,24 +7684,34 @@ ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
 ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
                             BinaryOperatorKind Opc,
                             Expr *lhs, Expr *rhs) {
-  if (getLangOptions().CPlusPlus &&
-      (!isa<ObjCPropertyRefExpr>(lhs)
-       || rhs->isTypeDependent() || Opc != BO_Assign) &&
-      (lhs->getType()->isOverloadableType() ||
-       rhs->getType()->isOverloadableType())) {
-    // Find all of the overloaded operators visible from this
-    // point. We perform both an operator-name lookup from the local
-    // scope and an argument-dependent lookup based on the types of
-    // the arguments.
-    UnresolvedSet<16> Functions;
-    OverloadedOperatorKind OverOp = BinaryOperator::getOverloadedOperator(Opc);
-    if (S && OverOp != OO_None)
-      LookupOverloadedOperatorName(OverOp, S, lhs->getType(), rhs->getType(),
-                                   Functions);
+  if (getLangOptions().CPlusPlus) {
+    bool UseBuiltinOperator;
 
-    // Build the (potentially-overloaded, potentially-dependent)
-    // binary operation.
-    return CreateOverloadedBinOp(OpLoc, Opc, Functions, lhs, rhs);
+    if (lhs->isTypeDependent() || rhs->isTypeDependent()) {
+      UseBuiltinOperator = false;
+    } else if (Opc == BO_Assign && lhs->getObjectKind() == OK_ObjCProperty) {
+      UseBuiltinOperator = true;
+    } else {
+      UseBuiltinOperator = !lhs->getType()->isOverloadableType() &&
+                           !rhs->getType()->isOverloadableType();
+    }
+
+    if (!UseBuiltinOperator) {
+      // Find all of the overloaded operators visible from this
+      // point. We perform both an operator-name lookup from the local
+      // scope and an argument-dependent lookup based on the types of
+      // the arguments.
+      UnresolvedSet<16> Functions;
+      OverloadedOperatorKind OverOp
+        = BinaryOperator::getOverloadedOperator(Opc);
+      if (S && OverOp != OO_None)
+        LookupOverloadedOperatorName(OverOp, S, lhs->getType(), rhs->getType(),
+                                     Functions);
+
+      // Build the (potentially-overloaded, potentially-dependent)
+      // binary operation.
+      return CreateOverloadedBinOp(OpLoc, Opc, Functions, lhs, rhs);
+    }
   }
 
   // Build a built-in binary operation.
diff --git a/clang/test/SemaTemplate/enum-argument.cpp b/clang/test/SemaTemplate/enum-argument.cpp
index a79ed8403e9..7d237570678 100644
--- a/clang/test/SemaTemplate/enum-argument.cpp
+++ b/clang/test/SemaTemplate/enum-argument.cpp
@@ -30,7 +30,7 @@ namespace rdar8020920 {
     unsigned long long bitfield : e0;
 
     void f(int j) {
-      bitfield + j; // expected-warning {{expression result unused}}
+      bitfield + j;
     }
   };
 }
diff --git a/clang/test/SemaTemplate/member-access-expr.cpp b/clang/test/SemaTemplate/member-access-expr.cpp
index 16b9515a15d..f1aa30ec32a 100644
--- a/clang/test/SemaTemplate/member-access-expr.cpp
+++ b/clang/test/SemaTemplate/member-access-expr.cpp
@@ -132,3 +132,18 @@ namespace test5 {
     }
   };
 }
+
+// PR8739
+namespace test6 {
+  struct A {};
+  struct B {};
+  template <class T> class Base;
+  template <class T> class Derived : public Base<T> {
+    A *field;
+    void get(B **ptr) {
+      // It's okay if at some point we figure out how to diagnose this
+      // at instantiation time.
+      *ptr = field;
+    }
+  };
+}