Improve the representation of operator expressions like "x + y" within

C++ templates. In particular, keep track of the overloaded operators
that are visible from the template definition, so that they can be
merged with those operators visible via argument-dependent lookup at
instantiation time. 

Refactored the lookup routines for argument-dependent lookup and for
operator name lookup, so they can be called without immediately adding
the results to an overload set.

Instantiation of these expressions is completely wrong. I'll work on
that next.

llvm-svn: 66851

clang/include/clang/AST/Expr.h

@@ -1164,6 +1164,24 @@ public:
   static CStyleCastExpr* CreateImpl(llvm::Deserializer& D, ASTContext& C);
 };
 
+/// \brief A builtin binary operation expression such as "x + y" or "x <= y".
+///
+/// This expression node kind describes a builtin binary operation,
+/// such as "x + y" for integer values "x" and "y". The operands will
+/// already have been converted to appropriate types (e.g., by
+/// performing promotions or conversions).
+///
+/// In C++, where operators may be overloaded, a different kind of
+/// expression node (CXXOperatorCallExpr) is used to express the
+/// invocation of an overloaded operator with operator syntax. Within
+/// a C++ template, whether BinaryOperator or CXXOperatorCallExpr is
+/// used to store an expression "x + y" depends on the subexpressions
+/// for x and y. If neither x or y is type-dependent, and the "+"
+/// operator resolves to a built-in operation, BinaryOperator will be
+/// used to express the computation (x and y may still be
+/// value-dependent). If either x or y is type-dependent, or if the
+/// "+" resolves to an overloaded operator, CXXOperatorCallExpr will
+/// be used to express the computation.
 class BinaryOperator : public Expr {
 public:
   enum Opcode {

clang/include/clang/AST/ExprCXX.h

@@ -26,10 +26,19 @@ namespace clang {
 // C++ Expressions.
 //===--------------------------------------------------------------------===//
 
-/// CXXOperatorCallExpr - Represents a call to an overloaded operator
+/// \brief A call to an overloaded operator written using operator
-/// written using operator syntax, e.g., "x + y" or "*p". While
+/// syntax.
-/// semantically equivalent to a normal call, this AST node provides
+///
-/// better information about the syntactic representation of the call.
+/// Represents a call to an overloaded operator written using operator
+/// syntax, e.g., "x + y" or "*p". While semantically equivalent to a
+/// normal call, this AST node provides better information about the
+/// syntactic representation of the call.
+///
+/// In a C++ template, this expression node kind will be used whenever
+/// any of the arguments are type-dependent. In this case, the
+/// function itself will be a (possibly empty) set of functions and
+/// function templates that were found by name lookup at template
+/// definition time.
 class CXXOperatorCallExpr : public CallExpr {
 public:
   CXXOperatorCallExpr(ASTContext& C, Expr *fn, Expr **args, unsigned numargs,

clang/lib/Sema/Sema.h

@@ -928,10 +928,19 @@ public:
                                 bool RedeclarationOnly = false,
                                 bool AllowBuiltinCreation = true,
                                 SourceLocation Loc = SourceLocation());
-  
+
+  typedef llvm::SmallPtrSet<FunctionDecl *, 16> FunctionSet;
   typedef llvm::SmallPtrSet<NamespaceDecl *, 16> AssociatedNamespaceSet;
   typedef llvm::SmallPtrSet<CXXRecordDecl *, 16> AssociatedClassSet;
 
+  void LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S,
+                                    QualType T1, QualType T2, 
+                                    FunctionSet &Functions);
+                          
+  void ArgumentDependentLookup(DeclarationName Name,
+                               Expr **Args, unsigned NumArgs,
+                               FunctionSet &Functions);
+
   void FindAssociatedClassesAndNamespaces(Expr **Args, unsigned NumArgs,
                                    AssociatedNamespaceSet &AssociatedNamespaces,
                                    AssociatedClassSet &AssociatedClasses);

clang/lib/Sema/SemaExpr.cpp

@@ -3977,6 +3977,32 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
                                                       CompTy, OpLoc));
 }
 
+static OverloadedOperatorKind 
+getOverloadedOperator(BinaryOperator::Opcode Opc) {
+  static const OverloadedOperatorKind OverOps[] = {
+    // Overloading .* is not possible.
+    static_cast<OverloadedOperatorKind>(0), OO_ArrowStar,
+    OO_Star, OO_Slash, OO_Percent,
+    OO_Plus, OO_Minus,
+    OO_LessLess, OO_GreaterGreater,
+    OO_Less, OO_Greater, OO_LessEqual, OO_GreaterEqual,
+    OO_EqualEqual, OO_ExclaimEqual,
+    OO_Amp,
+    OO_Caret,
+    OO_Pipe,
+    OO_AmpAmp,
+    OO_PipePipe,
+    OO_Equal, OO_StarEqual,
+    OO_SlashEqual, OO_PercentEqual,
+    OO_PlusEqual, OO_MinusEqual,
+    OO_LessLessEqual, OO_GreaterGreaterEqual,
+    OO_AmpEqual, OO_CaretEqual,
+    OO_PipeEqual,
+    OO_Comma
+  };
+  return OverOps[Opc];
+}
+
 // Binary Operators.  'Tok' is the token for the operator.
 Action::OwningExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
                                           tok::TokenKind Kind,
@@ -3988,16 +4014,39 @@ Action::OwningExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
   assert((rhs != 0) && "ActOnBinOp(): missing right expression");
 
   // If either expression is type-dependent, just build the AST.
-  // FIXME: We'll need to perform some caching of the result of name
-  // lookup for operator+.
   if (lhs->isTypeDependent() || rhs->isTypeDependent()) {
-    if (Opc > BinaryOperator::Assign && Opc <= BinaryOperator::OrAssign)
+    // .* cannot be overloaded.
-      return Owned(new (Context) CompoundAssignOperator(lhs, rhs, Opc,
+    if (Opc == BinaryOperator::PtrMemD)
-                                              Context.DependentTy,
-                                              Context.DependentTy, TokLoc));
-    else
       return Owned(new (Context) BinaryOperator(lhs, rhs, Opc,
                                                 Context.DependentTy, TokLoc));
+    
+    // Find all of the overloaded operators visible from the template
+    // definition. We perform both an operator-name lookup from the
+    // local scope and an argument-dependent lookup based on the types
+    // of the arguments.
+    FunctionSet Functions;
+    OverloadedOperatorKind OverOp = getOverloadedOperator(Opc);
+    LookupOverloadedOperatorName(OverOp, S, lhs->getType(), rhs->getType(),
+                                 Functions);
+    Expr *Args[2] = { lhs, rhs };
+    DeclarationName OpName 
+      = Context.DeclarationNames.getCXXOperatorName(OverOp);
+    ArgumentDependentLookup(OpName, Args, 2, Functions);
+
+    OverloadedFunctionDecl *Overloads 
+      = OverloadedFunctionDecl::Create(Context, CurContext, OpName);
+    for (FunctionSet::iterator Func = Functions.begin(), 
+                            FuncEnd = Functions.end();
+         Func != FuncEnd; ++Func)
+      Overloads->addOverload(*Func);
+
+    DeclRefExpr *Fn = new (Context) DeclRefExpr(Overloads, Context.OverloadTy,
+                                                TokLoc, false, false);
+    
+    return Owned(new (Context) CXXOperatorCallExpr(Context, Fn,
+                                                   Args, 2, 
+                                                   Context.DependentTy,
+                                                   TokLoc));
   }
 
   if (getLangOptions().CPlusPlus && Opc != BinaryOperator::PtrMemD &&
@@ -4012,32 +4061,11 @@ Action::OwningExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
     }
 
     // Determine which overloaded operator we're dealing with.
-    static const OverloadedOperatorKind OverOps[] = {
-      // Overloading .* is not possible.
-      static_cast<OverloadedOperatorKind>(0), OO_ArrowStar,
-      OO_Star, OO_Slash, OO_Percent,
-      OO_Plus, OO_Minus,
-      OO_LessLess, OO_GreaterGreater,
-      OO_Less, OO_Greater, OO_LessEqual, OO_GreaterEqual,
-      OO_EqualEqual, OO_ExclaimEqual,
-      OO_Amp,
-      OO_Caret,
-      OO_Pipe,
-      OO_AmpAmp,
-      OO_PipePipe,
-      OO_Equal, OO_StarEqual,
-      OO_SlashEqual, OO_PercentEqual,
-      OO_PlusEqual, OO_MinusEqual,
-      OO_LessLessEqual, OO_GreaterGreaterEqual,
-      OO_AmpEqual, OO_CaretEqual,
-      OO_PipeEqual,
-      OO_Comma
-    };
-    OverloadedOperatorKind OverOp = OverOps[Opc];
 
     // Add the appropriate overloaded operators (C++ [over.match.oper])
     // to the candidate set.
     OverloadCandidateSet CandidateSet;
+    OverloadedOperatorKind OverOp = getOverloadedOperator(Opc);
     Expr *Args[2] = { lhs, rhs };
     if (AddOperatorCandidates(OverOp, S, TokLoc, Args, 2, CandidateSet))
       return ExprError();

clang/lib/Sema/SemaLookup.cpp

@@ -1382,3 +1382,118 @@ Sema::FindAssociatedClassesAndNamespaces(Expr **Args, unsigned NumArgs,
     }
   }
 }
+
+/// IsAcceptableNonMemberOperatorCandidate - Determine whether Fn is
+/// an acceptable non-member overloaded operator for a call whose
+/// arguments have types T1 (and, if non-empty, T2). This routine
+/// implements the check in C++ [over.match.oper]p3b2 concerning
+/// enumeration types.
+static bool 
+IsAcceptableNonMemberOperatorCandidate(FunctionDecl *Fn,
+                                       QualType T1, QualType T2,
+                                       ASTContext &Context) {
+  if (T1->isRecordType() || (!T2.isNull() && T2->isRecordType()))
+    return true;
+
+  const FunctionProtoType *Proto = Fn->getType()->getAsFunctionProtoType();
+  if (Proto->getNumArgs() < 1)
+    return false;
+
+  if (T1->isEnumeralType()) {
+    QualType ArgType = Proto->getArgType(0).getNonReferenceType();
+    if (Context.getCanonicalType(T1).getUnqualifiedType()
+          == Context.getCanonicalType(ArgType).getUnqualifiedType())
+      return true;
+  }
+
+  if (Proto->getNumArgs() < 2)
+    return false;
+
+  if (!T2.isNull() && T2->isEnumeralType()) {
+    QualType ArgType = Proto->getArgType(1).getNonReferenceType();
+    if (Context.getCanonicalType(T2).getUnqualifiedType()
+          == Context.getCanonicalType(ArgType).getUnqualifiedType())
+      return true;
+  }
+
+  return false;
+}
+
+void Sema::LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S,
+                                        QualType T1, QualType T2, 
+                                        FunctionSet &Functions) {
+  // C++ [over.match.oper]p3:
+  //     -- The set of non-member candidates is the result of the
+  //        unqualified lookup of operator@ in the context of the
+  //        expression according to the usual rules for name lookup in
+  //        unqualified function calls (3.4.2) except that all member
+  //        functions are ignored. However, if no operand has a class
+  //        type, only those non-member functions in the lookup set
+  //        that have a first parameter of type T1 or “reference to
+  //        (possibly cv-qualified) T1”, when T1 is an enumeration
+  //        type, or (if there is a right operand) a second parameter
+  //        of type T2 or “reference to (possibly cv-qualified) T2”,
+  //        when T2 is an enumeration type, are candidate functions.
+  DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(Op);
+  LookupResult Operators = LookupName(S, OpName, LookupOperatorName);
+  
+  assert(!Operators.isAmbiguous() && "Operator lookup cannot be ambiguous");
+
+  if (!Operators)
+    return;
+
+  for (LookupResult::iterator Op = Operators.begin(), OpEnd = Operators.end();
+       Op != OpEnd; ++Op) {
+    if (FunctionDecl *FD = dyn_cast<FunctionDecl>(*Op))
+      if (IsAcceptableNonMemberOperatorCandidate(FD, T1, T2, Context))
+        Functions.insert(FD); // FIXME: canonical FD
+  }
+}
+
+void Sema::ArgumentDependentLookup(DeclarationName Name,
+                                   Expr **Args, unsigned NumArgs,
+                                   FunctionSet &Functions) {
+  // Find all of the associated namespaces and classes based on the
+  // arguments we have.
+  AssociatedNamespaceSet AssociatedNamespaces;
+  AssociatedClassSet AssociatedClasses;
+  FindAssociatedClassesAndNamespaces(Args, NumArgs, 
+                                     AssociatedNamespaces, AssociatedClasses);
+
+  // C++ [basic.lookup.argdep]p3:
+  //
+  //   Let X be the lookup set produced by unqualified lookup (3.4.1)
+  //   and let Y be the lookup set produced by argument dependent
+  //   lookup (defined as follows). If X contains [...] then Y is
+  //   empty. Otherwise Y is the set of declarations found in the
+  //   namespaces associated with the argument types as described
+  //   below. The set of declarations found by the lookup of the name
+  //   is the union of X and Y.
+  //
+  // Here, we compute Y and add its members to the overloaded
+  // candidate set.
+  for (AssociatedNamespaceSet::iterator NS = AssociatedNamespaces.begin(),
+                                     NSEnd = AssociatedNamespaces.end(); 
+       NS != NSEnd; ++NS) { 
+    //   When considering an associated namespace, the lookup is the
+    //   same as the lookup performed when the associated namespace is
+    //   used as a qualifier (3.4.3.2) except that:
+    //
+    //     -- Any using-directives in the associated namespace are
+    //        ignored.
+    //
+    //     -- FIXME: Any namespace-scope friend functions declared in
+    //        associated classes are visible within their respective
+    //        namespaces even if they are not visible during an ordinary
+    //        lookup (11.4).
+    DeclContext::lookup_iterator I, E;
+    for (llvm::tie(I, E) = (*NS)->lookup(Name); I != E; ++I) {
+      FunctionDecl *Func = dyn_cast<FunctionDecl>(*I);
+      if (!Func)
+        break;
+
+      Functions.insert(Func);
+    }
+  }
+}
+

clang/lib/Sema/SemaOverload.cpp

@@ -2308,42 +2308,6 @@ void Sema::AddSurrogateCandidate(CXXConversionDecl *Conversion,
   }
 }
 
-/// IsAcceptableNonMemberOperatorCandidate - Determine whether Fn is
-/// an acceptable non-member overloaded operator for a call whose
-/// arguments have types T1 (and, if non-empty, T2). This routine
-/// implements the check in C++ [over.match.oper]p3b2 concerning
-/// enumeration types.
-static bool 
-IsAcceptableNonMemberOperatorCandidate(FunctionDecl *Fn,
-                                       QualType T1, QualType T2,
-                                       ASTContext &Context) {
-  if (T1->isRecordType() || (!T2.isNull() && T2->isRecordType()))
-    return true;
-
-  const FunctionProtoType *Proto = Fn->getType()->getAsFunctionProtoType();
-  if (Proto->getNumArgs() < 1)
-    return false;
-
-  if (T1->isEnumeralType()) {
-    QualType ArgType = Proto->getArgType(0).getNonReferenceType();
-    if (Context.getCanonicalType(T1).getUnqualifiedType()
-          == Context.getCanonicalType(ArgType).getUnqualifiedType())
-      return true;
-  }
-
-  if (Proto->getNumArgs() < 2)
-    return false;
-
-  if (!T2.isNull() && T2->isEnumeralType()) {
-    QualType ArgType = Proto->getArgType(1).getNonReferenceType();
-    if (Context.getCanonicalType(T2).getUnqualifiedType()
-          == Context.getCanonicalType(ArgType).getUnqualifiedType())
-      return true;
-  }
-
-  return false;
-}
-
 /// AddOperatorCandidates - Add the overloaded operator candidates for
 /// the operator Op that was used in an operator expression such as "x
 /// Op y". S is the scope in which the expression occurred (used for
@@ -2383,6 +2347,8 @@ bool Sema::AddOperatorCandidates(OverloadedOperatorKind Op, Scope *S,
                          /*SuppressUserConversions=*/false);
   }
 
+  FunctionSet Functions;
+
   //     -- The set of non-member candidates is the result of the
   //        unqualified lookup of operator@ in the context of the
   //        expression according to the usual rules for name lookup in
@@ -2394,24 +2360,19 @@ bool Sema::AddOperatorCandidates(OverloadedOperatorKind Op, Scope *S,
   //        type, or (if there is a right operand) a second parameter
   //        of type T2 or “reference to (possibly cv-qualified) T2”,
   //        when T2 is an enumeration type, are candidate functions.
-  LookupResult Operators = LookupName(S, OpName, LookupOperatorName);
+  LookupOverloadedOperatorName(Op, S, T1, T2, Functions);
-  
-  if (Operators.isAmbiguous())
-    return DiagnoseAmbiguousLookup(Operators, OpName, OpLoc, OpRange);
-  else if (Operators) {
-    for (LookupResult::iterator Op = Operators.begin(), OpEnd = Operators.end();
-         Op != OpEnd; ++Op) {
-      if (FunctionDecl *FD = dyn_cast<FunctionDecl>(*Op))
-        if (IsAcceptableNonMemberOperatorCandidate(FD, T1, T2, Context))
-          AddOverloadCandidate(FD, Args, NumArgs, CandidateSet,
-                               /*SuppressUserConversions=*/false);
-    }
-  }
 
   // Since the set of non-member candidates corresponds to
   // *unqualified* lookup of the operator name, we also perform
   // argument-dependent lookup (C++ [basic.lookup.argdep]).
-  AddArgumentDependentLookupCandidates(OpName, Args, NumArgs, CandidateSet);
+  ArgumentDependentLookup(OpName, Args, NumArgs, Functions);
+
+  // Add all of the functions found via operator name lookup and
+  // argument-dependent lookup to the candidate set.
+  for (FunctionSet::iterator Func = Functions.begin(),
+                          FuncEnd = Functions.end();
+       Func != FuncEnd; ++Func)
+    AddOverloadCandidate(*Func, Args, NumArgs, CandidateSet);
 
   // Add builtin overload candidates (C++ [over.built]).
   AddBuiltinOperatorCandidates(Op, Args, NumArgs, CandidateSet);
@@ -3250,62 +3211,33 @@ void
 Sema::AddArgumentDependentLookupCandidates(DeclarationName Name,
                                            Expr **Args, unsigned NumArgs,
                                            OverloadCandidateSet& CandidateSet) {
-  // Find all of the associated namespaces and classes based on the
+  FunctionSet Functions;
-  // arguments we have.
-  AssociatedNamespaceSet AssociatedNamespaces;
-  AssociatedClassSet AssociatedClasses;
-  FindAssociatedClassesAndNamespaces(Args, NumArgs, 
-                                     AssociatedNamespaces, AssociatedClasses);
 
-  // C++ [basic.lookup.argdep]p3:
+  // Record all of the function candidates that we've already
-  //
+  // added to the overload set, so that we don't add those same
-  //   Let X be the lookup set produced by unqualified lookup (3.4.1)
+  // candidates a second time.
-  //   and let Y be the lookup set produced by argument dependent
+  for (OverloadCandidateSet::iterator Cand = CandidateSet.begin(),
-  //   lookup (defined as follows). If X contains [...] then Y is
+                                   CandEnd = CandidateSet.end();
-  //   empty. Otherwise Y is the set of declarations found in the
+       Cand != CandEnd; ++Cand)
-  //   namespaces associated with the argument types as described
+    if (Cand->Function)
-  //   below. The set of declarations found by the lookup of the name
+      Functions.insert(Cand->Function);
-  //   is the union of X and Y.
-  //
-  // Here, we compute Y and add its members to the overloaded
-  // candidate set.
-  llvm::SmallPtrSet<FunctionDecl *, 16> KnownCandidates;
-  for (AssociatedNamespaceSet::iterator NS = AssociatedNamespaces.begin(),
-                                     NSEnd = AssociatedNamespaces.end(); 
-       NS != NSEnd; ++NS) { 
-    //   When considering an associated namespace, the lookup is the
-    //   same as the lookup performed when the associated namespace is
-    //   used as a qualifier (3.4.3.2) except that:
-    //
-    //     -- Any using-directives in the associated namespace are
-    //        ignored.
-    //
-    //     -- FIXME: Any namespace-scope friend functions declared in
-    //        associated classes are visible within their respective
-    //        namespaces even if they are not visible during an ordinary
-    //        lookup (11.4).
-    DeclContext::lookup_iterator I, E;
-    for (llvm::tie(I, E) = (*NS)->lookup(Name); I != E; ++I) {
-      FunctionDecl *Func = dyn_cast<FunctionDecl>(*I);
-      if (!Func)
-        break;
 
-      if (KnownCandidates.empty()) {
+  ArgumentDependentLookup(Name, Args, NumArgs, Functions);
-        // Record all of the function candidates that we've already
-        // added to the overload set, so that we don't add those same
-        // candidates a second time.
-        for (OverloadCandidateSet::iterator Cand = CandidateSet.begin(),
-                                         CandEnd = CandidateSet.end();
-             Cand != CandEnd; ++Cand)
-          KnownCandidates.insert(Cand->Function);
-      }
 
-      // If we haven't seen this function before, add it as a
+  // Erase all of the candidates we already knew about.
-      // candidate.
+  // FIXME: This is suboptimal. Is there a better way?
-      if (KnownCandidates.insert(Func))
+  for (OverloadCandidateSet::iterator Cand = CandidateSet.begin(),
-        AddOverloadCandidate(Func, Args, NumArgs, CandidateSet);
+                                   CandEnd = CandidateSet.end();
-    }
+       Cand != CandEnd; ++Cand)
-  }
+    if (Cand->Function)
+      Functions.erase(Cand->Function);
+
+  // For each of the ADL candidates we found, add it to the overload
+  // set.
+  for (FunctionSet::iterator Func = Functions.begin(),
+                          FuncEnd = Functions.end();
+       Func != FuncEnd; ++Func)
+    AddOverloadCandidate(*Func, Args, NumArgs, CandidateSet);
 }
 
 /// isBetterOverloadCandidate - Determines whether the first overload

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -577,6 +577,7 @@ namespace {
     Sema::OwningExprResult VisitDeclRefExpr(DeclRefExpr *E);
     Sema::OwningExprResult VisitParenExpr(ParenExpr *E);
     Sema::OwningExprResult VisitBinaryOperator(BinaryOperator *E);
+    Sema::OwningExprResult VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E);
 
     // Base case. I'm supposed to ignore this.
     Sema::OwningExprResult VisitStmt(Stmt *) { 
@@ -645,6 +646,32 @@ TemplateExprInstantiator::VisitBinaryOperator(BinaryOperator *E) {
   return move(Result);
 }
 
+Sema::OwningExprResult 
+TemplateExprInstantiator::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
+  // FIXME: HACK HACK HACK. This is so utterly and completely wrong
+  // that I don't want to explain it here. I'll just fix it tomorrow
+  // instead.
+  Sema::OwningExprResult LHS = Visit(E->getArg(0));
+  if (LHS.isInvalid())
+    return SemaRef.ExprError();
+
+  Sema::OwningExprResult RHS = Visit(E->getArg(1));
+  if (RHS.isInvalid())
+    return SemaRef.ExprError();
+
+  Sema::OwningExprResult Result
+    = SemaRef.CreateBuiltinBinOp(E->getOperatorLoc(), 
+                                 BinaryOperator::Add,
+                                 (Expr *)LHS.get(),
+                                 (Expr *)RHS.get());
+  if (Result.isInvalid())
+    return SemaRef.ExprError();
+
+  LHS.release();
+  RHS.release();
+  return move(Result);  
+}
+
 Sema::OwningExprResult 
 Sema::InstantiateExpr(Expr *E, const TemplateArgument *TemplateArgs,
                       unsigned NumTemplateArgs) {