[AST] Improve BinaryExpr

include/swift/AST/Expr.h

@@ -4687,18 +4687,27 @@ class PostfixUnaryExpr : public ApplyExpr {
 /// BinaryExpr - Infix binary expressions like 'x+y'.  The argument is always
 /// an implicit tuple expression of the type expected by the function.
 class BinaryExpr : public ApplyExpr {
+  BinaryExpr(Expr *fn, TupleExpr *arg, bool implicit, Type ty = Type())
+      : ApplyExpr(ExprKind::Binary, fn, arg, implicit, ty) {
+    assert(arg->getNumElements() == 2);
+  }
+
 public:
-  BinaryExpr(Expr *Fn, TupleExpr *Arg, bool Implicit, Type Ty = Type())
-    : ApplyExpr(ExprKind::Binary, Fn, Arg, Implicit, Ty) {}
+  static BinaryExpr *create(ASTContext &ctx, Expr *lhs, Expr *fn, Expr *rhs,
+                            bool implicit, Type ty = Type());
+
+  /// The left-hand argument of the binary operation.
+  Expr *getLHS() const { return cast<TupleExpr>(getArg())->getElement(0); }
+
+  /// The right-hand argument of the binary operation.
+  Expr *getRHS() const { return cast<TupleExpr>(getArg())->getElement(1); }
 
   SourceLoc getLoc() const { return getFn()->getLoc(); }
 
   SourceRange getSourceRange() const { return getArg()->getSourceRange(); }
   SourceLoc getStartLoc() const { return getArg()->getStartLoc(); }
   SourceLoc getEndLoc() const { return getArg()->getEndLoc(); }
 
-  TupleExpr *getArg() const { return cast<TupleExpr>(ApplyExpr::getArg()); }
-
   static bool classof(const Expr *E) { return E->getKind() == ExprKind::Binary;}
 };
 

lib/AST/Expr.cpp

@@ -1768,6 +1768,14 @@ Expr *CallExpr::getDirectCallee() const {
   }
 }
 
+BinaryExpr *BinaryExpr::create(ASTContext &ctx, Expr *lhs, Expr *fn, Expr *rhs,
+                               bool implicit, Type ty) {
+  auto *packedArg = TupleExpr::createImplicit(ctx, {lhs, rhs}, /*labels*/ {});
+  computeSingleArgumentType(ctx, packedArg, /*implicit*/ true,
+                            [](Expr *E) { return E->getType(); });
+  return new (ctx) BinaryExpr(fn, packedArg, implicit, ty);
+}
+
 SourceLoc DotSyntaxCallExpr::getLoc() const {
   if (isImplicit()) {
     SourceLoc baseLoc = getBase()->getLoc();

lib/IDE/Refactoring.cpp

@@ -2469,11 +2469,11 @@ bool RefactoringActionConvertToSwitchStmt::performChange() {
     SmallString<64> ConditionalPattern = SmallString<64>();
 
     Expr *walkToExprPost(Expr *E) override {
-      if (E->getKind() != ExprKind::Binary)
+      auto *BE = dyn_cast<BinaryExpr>(E);
+      if (!BE)
         return E;
-      auto BE = dyn_cast<BinaryExpr>(E);
       if (isFunctionNameAllowed(BE))
-        appendPattern(dyn_cast<BinaryExpr>(E)->getArg());
+        appendPattern(BE->getLHS(), BE->getRHS());
       return E;
     }
 
@@ -2499,10 +2499,10 @@ bool RefactoringActionConvertToSwitchStmt::performChange() {
       || FunctionName == "__derived_struct_equals";
     }
 
-    void appendPattern(TupleExpr *Tuple) {
-      auto PatternArgument = Tuple->getElements().back();
+    void appendPattern(Expr *LHS, Expr *RHS) {
+      auto *PatternArgument = RHS;
       if (PatternArgument->getKind() == ExprKind::DeclRef)
-        PatternArgument = Tuple->getElements().front();
+        PatternArgument = LHS;
       if (ConditionalPattern.size() > 0)
         ConditionalPattern.append(", ");
       ConditionalPattern.append(Lexer::getCharSourceRangeFromSourceRange(SM, PatternArgument->getSourceRange()).str());
@@ -4424,7 +4424,7 @@ struct CallbackCondition {
   ///   - `<Subject> == nil`
   CallbackCondition(const BinaryExpr *BE, const FuncDecl *Operator) {
     bool FoundNil = false;
-    for (auto *Operand : BE->getArg()->getElements()) {
+    for (auto *Operand : {BE->getLHS(), BE->getRHS()}) {
       if (isa<NilLiteralExpr>(Operand)) {
         FoundNil = true;
       } else if (auto *DRE = dyn_cast<DeclRefExpr>(Operand)) {
@@ -4504,8 +4504,8 @@ struct CallbackCondition {
             auto *Operator = isOperator(BE);
             if (Operator) {
               if (Operator->getBaseName() == "&&") {
-                auto Args = BE->getArg()->getElements();
-                Exprs.insert(Exprs.end(), Args.begin(), Args.end());
+                Exprs.push_back(BE->getLHS());
+                Exprs.push_back(BE->getRHS());
               } else {
                 addCond(CallbackCondition(BE, Operator), Decls, AddTo, Handled);
               }

lib/IDE/SourceEntityWalker.cpp

@@ -439,11 +439,11 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
     }
   } else if (auto *BinE = dyn_cast<BinaryExpr>(E)) {
     // Visit in source order.
-    if (!BinE->getArg()->getElement(0)->walk(*this))
+    if (!BinE->getLHS()->walk(*this))
       return doStopTraversal();
     if (!BinE->getFn()->walk(*this))
       return doStopTraversal();
-    if (!BinE->getArg()->getElement(1)->walk(*this))
+    if (!BinE->getRHS()->walk(*this))
       return doStopTraversal();
 
     // We already visited the children.

lib/IDE/SwiftSourceDocInfo.cpp

@@ -412,11 +412,11 @@ std::pair<bool, Expr*> NameMatcher::walkToExprPre(Expr *E) {
       case ExprKind::Binary: {
         BinaryExpr *BinE = cast<BinaryExpr>(E);
         // Visit in source order.
-        if (!BinE->getArg()->getElement(0)->walk(*this))
+        if (!BinE->getLHS()->walk(*this))
           return {false, nullptr};
         if (!BinE->getFn()->walk(*this))
           return {false, nullptr};
-        if (!BinE->getArg()->getElement(1)->walk(*this))
+        if (!BinE->getRHS()->walk(*this))
           return {false, nullptr};
 
         // We already visited the children.

lib/Parse/ParseIfConfig.cpp

@@ -204,11 +204,7 @@ class ValidateIfConfigCondition :
 
       // Apply the operator with left-associativity by folding the first two
       // operands.
-      TupleExpr *Arg = TupleExpr::create(Ctx, SourceLoc(), { LHS, RHS },
-                                         { }, { }, SourceLoc(),
-                                         /*HasTrailingClosure=*/false,
-                                         /*Implicit=*/true);
-      LHS = new (Ctx) BinaryExpr(Op, Arg, /*implicit*/false);
+      LHS = BinaryExpr::create(Ctx, LHS, Op, RHS, /*implicit*/ false);
 
       // If we don't have the next operator, we're done.
       if (IsEnd)
@@ -527,9 +523,8 @@ class EvaluateIfConfigCondition :
 
   bool visitBinaryExpr(BinaryExpr *E) {
     auto OpName = getDeclRefStr(E->getFn());
-    auto Args = E->getArg()->getElements();
-    if (OpName == "||") return visit(Args[0]) || visit(Args[1]);
-    if (OpName == "&&") return visit(Args[0]) && visit(Args[1]);
+    if (OpName == "||") return visit(E->getLHS()) || visit(E->getRHS());
+    if (OpName == "&&") return visit(E->getLHS()) && visit(E->getRHS());
     llvm_unreachable("unsupported binary operator");
   }
 
@@ -557,9 +552,8 @@ class IsVersionIfConfigCondition :
 
   bool visitBinaryExpr(BinaryExpr *E) {
     auto OpName = getDeclRefStr(E->getFn());
-    auto Args = E->getArg()->getElements();
-    if (OpName == "||") return visit(Args[0]) && visit(Args[1]);
-    if (OpName == "&&") return visit(Args[0]) || visit(Args[1]);
+    if (OpName == "||") return visit(E->getLHS()) && visit(E->getRHS());
+    if (OpName == "&&") return visit(E->getLHS()) || visit(E->getRHS());
     llvm_unreachable("unsupported binary operator");
   }
 
@@ -592,9 +586,8 @@ static bool isPlatformConditionDisjunction(Expr *E, PlatformConditionKind Kind,
                                            ArrayRef<StringRef> Vals) {
   if (auto *Or = dyn_cast<BinaryExpr>(E)) {
     if (getDeclRefStr(Or->getFn()) == "||") {
-      auto Args = Or->getArg()->getElements();
-      return (isPlatformConditionDisjunction(Args[0], Kind, Vals) &&
-              isPlatformConditionDisjunction(Args[1], Kind, Vals));
+      return (isPlatformConditionDisjunction(Or->getLHS(), Kind, Vals) &&
+              isPlatformConditionDisjunction(Or->getRHS(), Kind, Vals));
     }
   } else if (auto *P = dyn_cast<ParenExpr>(E)) {
     return isPlatformConditionDisjunction(P->getSubExpr(), Kind, Vals);
@@ -655,11 +648,10 @@ static Expr *findAnyLikelySimulatorEnvironmentTest(Expr *Condition) {
 
   if (auto *And = dyn_cast<BinaryExpr>(Condition)) {
     if (getDeclRefStr(And->getFn()) == "&&") {
-      auto Args = And->getArg()->getElements();
-      if ((isSimulatorPlatformOSTest(Args[0]) &&
-           isSimulatorPlatformArchTest(Args[1])) ||
-          (isSimulatorPlatformOSTest(Args[1]) &&
-           isSimulatorPlatformArchTest(Args[0]))) {
+      if ((isSimulatorPlatformOSTest(And->getLHS()) &&
+           isSimulatorPlatformArchTest(And->getRHS())) ||
+          (isSimulatorPlatformOSTest(And->getRHS()) &&
+           isSimulatorPlatformArchTest(And->getLHS()))) {
         return And;
       }
     }

lib/Sema/CSDiagnostics.cpp

@@ -457,7 +457,7 @@ bool MissingConformanceFailure::diagnoseAsError() {
   if (isPatternMatchingOperator(anchor)) {
     auto *expr = castToExpr(anchor);
     if (auto *binaryOp = dyn_cast_or_null<BinaryExpr>(findParentExpr(expr))) {
-      auto *caseExpr = binaryOp->getArg()->getElement(0);
+      auto *caseExpr = binaryOp->getLHS();
 
       llvm::SmallPtrSet<Expr *, 4> anchors;
       for (const auto *fix : getSolution().Fixes) {
@@ -4080,11 +4080,9 @@ bool AllowTypeOrInstanceMemberFailure::diagnoseAsError() {
           ValueDecl *decl0 = overloadedFn->getDecls()[0];
 
           if (decl0->getBaseName() == decl0->getASTContext().Id_MatchOperator) {
-            assert(binaryExpr->getArg()->getElements().size() == 2);
-
             // If the rhs of '~=' is the enum type, a single dot suffixes
             // since the type can be inferred
-            Type secondArgType = getType(binaryExpr->getArg()->getElement(1));
+            Type secondArgType = getType(binaryExpr->getRHS());
             if (secondArgType->isEqual(baseTy)) {
               Diag->fixItInsert(loc, ".");
               return true;
@@ -6154,8 +6152,8 @@ bool ArgumentMismatchFailure::diagnoseUseOfReferenceEqualityOperator() const {
     return false;
 
   auto *binaryOp = castToExpr<BinaryExpr>(getRawAnchor());
-  auto *lhs = binaryOp->getArg()->getElement(0);
-  auto *rhs = binaryOp->getArg()->getElement(1);
+  auto *lhs = binaryOp->getLHS();
+  auto *rhs = binaryOp->getRHS();
 
   auto name = *getOperatorName(binaryOp->getFn());
 
@@ -6221,8 +6219,8 @@ bool ArgumentMismatchFailure::diagnosePatternMatchingMismatch() const {
     return false;
 
   auto *op = castToExpr<BinaryExpr>(getRawAnchor());
-  auto *lhsExpr = op->getArg()->getElement(0);
-  auto *rhsExpr = op->getArg()->getElement(1);
+  auto *lhsExpr = op->getLHS();
+  auto *rhsExpr = op->getRHS();
 
   auto lhsType = getType(lhsExpr);
   auto rhsType = getType(rhsExpr);

lib/Sema/CSSolver.cpp

@@ -884,8 +884,8 @@ void ConstraintSystem::shrink(Expr *expr) {
         return isArithmeticExprOfLiterals(postfix->getArg());
 
       if (auto binary = dyn_cast<BinaryExpr>(expr))
-        return isArithmeticExprOfLiterals(binary->getArg()->getElement(0)) &&
-               isArithmeticExprOfLiterals(binary->getArg()->getElement(1));
+        return isArithmeticExprOfLiterals(binary->getLHS()) &&
+               isArithmeticExprOfLiterals(binary->getRHS());
 
       return isa<IntegerLiteralExpr>(expr) || isa<FloatLiteralExpr>(expr);
     }

lib/Sema/ConstraintSystem.cpp

@@ -3200,8 +3200,8 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
 
   const auto &solution = solutions.front();
   if (auto *binaryOp = dyn_cast<BinaryExpr>(applyExpr)) {
-    auto *lhs = binaryOp->getArg()->getElement(0);
-    auto *rhs = binaryOp->getArg()->getElement(1);
+    auto *lhs = binaryOp->getLHS();
+    auto *rhs = binaryOp->getRHS();
 
     auto lhsType =
         solution.simplifyType(solution.getType(lhs))->getRValueType();

lib/Sema/DerivedConformanceAdditiveArithmetic.cpp

@@ -143,13 +143,8 @@ deriveBodyMathOperator(AbstractFunctionDecl *funcDecl, MathOperator op) {
                                          DeclNameLoc(), /*Implicit*/ true);
     auto *rhsArg = new (C) MemberRefExpr(rhsDRE, SourceLoc(), member,
                                          DeclNameLoc(), /*Implicit*/ true);
-    auto *memberOpArgs =
-        TupleExpr::create(C, SourceLoc(), {lhsArg, rhsArg}, {}, {}, SourceLoc(),
-                          /*HasTrailingClosure*/ false,
-                          /*Implicit*/ true);
-    auto *memberOpCallExpr =
-        new (C) BinaryExpr(memberOpExpr, memberOpArgs, /*Implicit*/ true);
-    return memberOpCallExpr;
+    return BinaryExpr::create(C, lhsArg, memberOpExpr, rhsArg,
+                              /*implicit*/ true);
   };
 
   // Create array of member operator call expressions.

lib/Sema/DerivedConformanceCodable.cpp

@@ -1590,11 +1590,8 @@ deriveBodyDecodable_enum_init(AbstractFunctionDecl *initDecl, void *) {
                     /*implicit*/ true, AccessSemantics::Ordinary, fnType);
     auto *oneExpr = IntegerLiteralExpr::createFromUnsigned(C, 1);
 
-    auto *tupleExpr = TupleExpr::createImplicit(C, {keysCountExpr, oneExpr},
-                                                {Identifier(), Identifier()});
-
-    auto *cmpExpr =
-        new (C) BinaryExpr(cmpFuncExpr, tupleExpr, /*implicit*/ true);
+    auto *cmpExpr = BinaryExpr::create(C, keysCountExpr, cmpFuncExpr, oneExpr,
+                                       /*implicit*/ true);
     cmpExpr->setThrows(false);
 
     auto *guardBody = BraceStmt::create(C, SourceLoc(), {throwStmt},

lib/Sema/DerivedConformanceComparable.cpp

@@ -78,13 +78,8 @@ deriveBodyComparable_enum_noAssociatedValues_lt(AbstractFunctionDecl *ltDecl,
                                           /*implicit*/ true,
                                           AccessSemantics::Ordinary);
 
-  TupleExpr *abTuple = TupleExpr::create(C, SourceLoc(), { aIndex, bIndex },
-                                         { }, { }, SourceLoc(),
-                                         /*HasTrailingClosure*/ false,
-                                         /*Implicit*/ true);
-
-  auto *cmpExpr = new (C) BinaryExpr(
-      cmpFuncExpr, abTuple, /*implicit*/ true);
+  auto *cmpExpr =
+      BinaryExpr::create(C, aIndex, cmpFuncExpr, bIndex, /*implicit*/ true);
   statements.push_back(new (C) ReturnStmt(SourceLoc(), cmpExpr));
 
   BraceStmt *body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -134,16 +134,9 @@ deriveBodyEquatable_enum_noAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
                                       fnType);
   }
 
-  TupleTypeElt abTupleElts[2] = { aIndex->getType(), bIndex->getType() };
-  TupleExpr *abTuple = TupleExpr::create(C, SourceLoc(), { aIndex, bIndex },
-                                         { }, { }, SourceLoc(),
-                                         /*HasTrailingClosure*/ false,
-                                         /*Implicit*/ true,
-                                         TupleType::get(abTupleElts, C));
-
-  auto *cmpExpr = new (C) BinaryExpr(
-      cmpFuncExpr, abTuple, /*implicit*/ true,
-      fnType->castTo<FunctionType>()->getResult());
+  auto *cmpExpr =
+      BinaryExpr::create(C, aIndex, cmpFuncExpr, bIndex, /*implicit*/ true,
+                         fnType->castTo<FunctionType>()->getResult());
   cmpExpr->setThrows(false);
   statements.push_back(new (C) ReturnStmt(SourceLoc(), cmpExpr));
 

lib/Sema/DerivedConformances.cpp

@@ -577,13 +577,8 @@ GuardStmt *DerivedConformance::returnIfNotEqualGuard(ASTContext &C,
   auto cmpFuncExpr = new (C) UnresolvedDeclRefExpr(
     DeclNameRef(C.Id_EqualsOperator), DeclRefKind::BinaryOperator,
     DeclNameLoc());
-  auto cmpArgsTuple = TupleExpr::create(C, SourceLoc(),
-                                        { lhsExpr, rhsExpr },
-                                        { }, { }, SourceLoc(),
-                                        /*HasTrailingClosure*/false,
-                                        /*Implicit*/true);
-  auto cmpExpr = new (C) BinaryExpr(cmpFuncExpr, cmpArgsTuple,
-                                    /*Implicit*/true);
+  auto *cmpExpr = BinaryExpr::create(C, lhsExpr, cmpFuncExpr, rhsExpr,
+                                     /*implicit*/ true);
   conditions.emplace_back(cmpExpr);
 
   // Build and return the complete guard statement.
@@ -617,12 +612,8 @@ GuardStmt *DerivedConformance::returnComparisonIfNotEqualGuard(ASTContext &C,
   auto ltFuncExpr = new (C) UnresolvedDeclRefExpr(
     DeclNameRef(C.Id_LessThanOperator), DeclRefKind::BinaryOperator,
     DeclNameLoc());
-  auto ltArgsTuple = TupleExpr::create(C, SourceLoc(),
-                                        { lhsExpr, rhsExpr },
-                                        { }, { }, SourceLoc(),
-                                        /*HasTrailingClosure*/false,
-                                        /*Implicit*/true);
-  auto ltExpr = new (C) BinaryExpr(ltFuncExpr, ltArgsTuple, /*Implicit*/true);
+  auto *ltExpr = BinaryExpr::create(C, lhsExpr, ltFuncExpr, rhsExpr,
+                                    /*implicit*/ true);
   return returnIfNotEqualGuard(C, lhsExpr, rhsExpr, ltExpr);
 }
 

lib/Sema/MiscDiagnostics.cpp

@@ -4532,11 +4532,6 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
     void tryDiagnoseComparisonWithNaN(BinaryExpr *BE) {
       ValueDecl *comparisonDecl = nullptr;
 
-      // Comparison functions like == or <= take two arguments.
-      if (BE->getArg()->getNumElements() != 2) {
-        return;
-      }
-
       // Dig out the function declaration.
       if (auto Fn = BE->getFn()) {
         if (auto DSCE = dyn_cast<DotSyntaxCallExpr>(Fn)) {
@@ -4557,8 +4552,8 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
         return;
       }
 
-      auto firstArg = BE->getArg()->getElement(0);
-      auto secondArg = BE->getArg()->getElement(1);
+      auto *firstArg = BE->getLHS();
+      auto *secondArg = BE->getRHS();
 
       // Both arguments must conform to FloatingPoint protocol.
       if (!TypeChecker::conformsToKnownProtocol(firstArg->getType(),

lib/Sema/PreCheckExpr.cpp

@@ -1799,7 +1799,7 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
       // The protocols we are composing
       SmallVector<TypeRepr *, 4> Types;
 
-      auto lhsExpr = binaryExpr->getArg()->getElement(0);
+      auto *lhsExpr = binaryExpr->getLHS();
       if (auto *lhs = dyn_cast<TypeExpr>(lhsExpr)) {
         Types.push_back(lhs->getTypeRepr());
       } else if (isa<BinaryExpr>(lhsExpr)) {
@@ -1819,7 +1819,7 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
         return nullptr;
 
       // Add the rhs which is just a TypeExpr
-      auto *rhs = dyn_cast<TypeExpr>(binaryExpr->getArg()->getElement(1));
+      auto *rhs = dyn_cast<TypeExpr>(binaryExpr->getRHS());
       if (!rhs) return nullptr;
       Types.push_back(rhs->getTypeRepr());