Skip to content

Commit 46fa6e5

Browse files
hamishknighthamishknight
committed
[AST] Improve BinaryExpr
Abstract away the TupleExpr gunk and expose `getLHS` and `getRHS` accessors. This is in preparation for completely expunging the use of TupleExpr as an argument list.
1 parent 7910254 commit 46fa6e5

20 files changed

+85
-131
lines changed

‎include/swift/AST/Expr.h‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4687,18 +4687,27 @@ class PostfixUnaryExpr : public ApplyExpr {
46874687
/// BinaryExpr - Infix binary expressions like 'x+y'. The argument is always
46884688
/// an implicit tuple expression of the type expected by the function.
46894689
class BinaryExpr : public ApplyExpr {
4690+
BinaryExpr(Expr *fn, TupleExpr *arg, bool implicit, Type ty = Type())
4691+
: ApplyExpr(ExprKind::Binary, fn, arg, implicit, ty) {
4692+
assert(arg->getNumElements() == 2);
4693+
}
4694+
46904695
public:
4691-
BinaryExpr(Expr *Fn, TupleExpr *Arg, bool Implicit, Type Ty = Type())
4692-
: ApplyExpr(ExprKind::Binary, Fn, Arg, Implicit, Ty) {}
4696+
static BinaryExpr *create(ASTContext &ctx, Expr *lhs, Expr *fn, Expr *rhs,
4697+
bool implicit, Type ty = Type());
4698+
4699+
/// The left-hand argument of the binary operation.
4700+
Expr *getLHS() const { return cast<TupleExpr>(getArg())->getElement(0); }
4701+
4702+
/// The right-hand argument of the binary operation.
4703+
Expr *getRHS() const { return cast<TupleExpr>(getArg())->getElement(1); }
46934704

46944705
SourceLoc getLoc() const { return getFn()->getLoc(); }
46954706

46964707
SourceRange getSourceRange() const { return getArg()->getSourceRange(); }
46974708
SourceLoc getStartLoc() const { return getArg()->getStartLoc(); }
46984709
SourceLoc getEndLoc() const { return getArg()->getEndLoc(); }
46994710

4700-
TupleExpr *getArg() const { return cast<TupleExpr>(ApplyExpr::getArg()); }
4701-
47024711
static bool classof(const Expr *E) { return E->getKind() == ExprKind::Binary;}
47034712
};
47044713

‎lib/AST/Expr.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,6 +1768,14 @@ Expr *CallExpr::getDirectCallee() const {
17681768
}
17691769
}
17701770

1771+
BinaryExpr *BinaryExpr::create(ASTContext &ctx, Expr *lhs, Expr *fn, Expr *rhs,
1772+
bool implicit, Type ty) {
1773+
auto *packedArg = TupleExpr::createImplicit(ctx, {lhs, rhs}, /*labels*/ {});
1774+
computeSingleArgumentType(ctx, packedArg, /*implicit*/ true,
1775+
[](Expr *E) { return E->getType(); });
1776+
return new (ctx) BinaryExpr(fn, packedArg, implicit, ty);
1777+
}
1778+
17711779
SourceLoc DotSyntaxCallExpr::getLoc() const {
17721780
if (isImplicit()) {
17731781
SourceLoc baseLoc = getBase()->getLoc();

‎lib/IDE/Refactoring.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2469,11 +2469,11 @@ bool RefactoringActionConvertToSwitchStmt::performChange() {
24692469
SmallString<64> ConditionalPattern = SmallString<64>();
24702470

24712471
Expr *walkToExprPost(Expr *E) override {
2472-
if (E->getKind() != ExprKind::Binary)
2472+
auto *BE = dyn_cast<BinaryExpr>(E);
2473+
if (!BE)
24732474
return E;
2474-
auto BE = dyn_cast<BinaryExpr>(E);
24752475
if (isFunctionNameAllowed(BE))
2476-
appendPattern(dyn_cast<BinaryExpr>(E)->getArg());
2476+
appendPattern(BE->getLHS(), BE->getRHS());
24772477
return E;
24782478
}
24792479

@@ -2499,10 +2499,10 @@ bool RefactoringActionConvertToSwitchStmt::performChange() {
24992499
|| FunctionName == "__derived_struct_equals";
25002500
}
25012501

2502-
void appendPattern(TupleExpr *Tuple) {
2503-
auto PatternArgument = Tuple->getElements().back();
2502+
void appendPattern(Expr *LHS, Expr *RHS) {
2503+
auto *PatternArgument = RHS;
25042504
if (PatternArgument->getKind() == ExprKind::DeclRef)
2505-
PatternArgument = Tuple->getElements().front();
2505+
PatternArgument = LHS;
25062506
if (ConditionalPattern.size() > 0)
25072507
ConditionalPattern.append(", ");
25082508
ConditionalPattern.append(Lexer::getCharSourceRangeFromSourceRange(SM, PatternArgument->getSourceRange()).str());
@@ -4424,7 +4424,7 @@ struct CallbackCondition {
44244424
/// - `<Subject> == nil`
44254425
CallbackCondition(const BinaryExpr *BE, const FuncDecl *Operator) {
44264426
bool FoundNil = false;
4427-
for (auto *Operand : BE->getArg()->getElements()) {
4427+
for (auto *Operand : {BE->getLHS(), BE->getRHS()}) {
44284428
if (isa<NilLiteralExpr>(Operand)) {
44294429
FoundNil = true;
44304430
} else if (auto *DRE = dyn_cast<DeclRefExpr>(Operand)) {
@@ -4504,8 +4504,8 @@ struct CallbackCondition {
45044504
auto *Operator = isOperator(BE);
45054505
if (Operator) {
45064506
if (Operator->getBaseName() == "&&") {
4507-
auto Args = BE->getArg()->getElements();
4508-
Exprs.insert(Exprs.end(), Args.begin(), Args.end());
4507+
Exprs.push_back(BE->getLHS());
4508+
Exprs.push_back(BE->getRHS());
45094509
} else {
45104510
addCond(CallbackCondition(BE, Operator), Decls, AddTo, Handled);
45114511
}

‎lib/IDE/SourceEntityWalker.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,11 +439,11 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
439439
}
440440
} else if (auto *BinE = dyn_cast<BinaryExpr>(E)) {
441441
// Visit in source order.
442-
if (!BinE->getArg()->getElement(0)->walk(*this))
442+
if (!BinE->getLHS()->walk(*this))
443443
return doStopTraversal();
444444
if (!BinE->getFn()->walk(*this))
445445
return doStopTraversal();
446-
if (!BinE->getArg()->getElement(1)->walk(*this))
446+
if (!BinE->getRHS()->walk(*this))
447447
return doStopTraversal();
448448

449449
// We already visited the children.

‎lib/IDE/SwiftSourceDocInfo.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,11 @@ std::pair<bool, Expr*> NameMatcher::walkToExprPre(Expr *E) {
412412
case ExprKind::Binary: {
413413
BinaryExpr *BinE = cast<BinaryExpr>(E);
414414
// Visit in source order.
415-
if (!BinE->getArg()->getElement(0)->walk(*this))
415+
if (!BinE->getLHS()->walk(*this))
416416
return {false, nullptr};
417417
if (!BinE->getFn()->walk(*this))
418418
return {false, nullptr};
419-
if (!BinE->getArg()->getElement(1)->walk(*this))
419+
if (!BinE->getRHS()->walk(*this))
420420
return {false, nullptr};
421421

422422
// We already visited the children.

‎lib/Parse/ParseIfConfig.cpp‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,7 @@ class ValidateIfConfigCondition :
204204

205205
// Apply the operator with left-associativity by folding the first two
206206
// operands.
207-
TupleExpr *Arg = TupleExpr::create(Ctx, SourceLoc(), { LHS, RHS },
208-
{ }, { }, SourceLoc(),
209-
/*HasTrailingClosure=*/false,
210-
/*Implicit=*/true);
211-
LHS = new (Ctx) BinaryExpr(Op, Arg, /*implicit*/false);
207+
LHS = BinaryExpr::create(Ctx, LHS, Op, RHS, /*implicit*/ false);
212208

213209
// If we don't have the next operator, we're done.
214210
if (IsEnd)
@@ -527,9 +523,8 @@ class EvaluateIfConfigCondition :
527523

528524
bool visitBinaryExpr(BinaryExpr *E) {
529525
auto OpName = getDeclRefStr(E->getFn());
530-
auto Args = E->getArg()->getElements();
531-
if (OpName == "||") return visit(Args[0]) || visit(Args[1]);
532-
if (OpName == "&&") return visit(Args[0]) && visit(Args[1]);
526+
if (OpName == "||") return visit(E->getLHS()) || visit(E->getRHS());
527+
if (OpName == "&&") return visit(E->getLHS()) && visit(E->getRHS());
533528
llvm_unreachable("unsupported binary operator");
534529
}
535530

@@ -557,9 +552,8 @@ class IsVersionIfConfigCondition :
557552

558553
bool visitBinaryExpr(BinaryExpr *E) {
559554
auto OpName = getDeclRefStr(E->getFn());
560-
auto Args = E->getArg()->getElements();
561-
if (OpName == "||") return visit(Args[0]) && visit(Args[1]);
562-
if (OpName == "&&") return visit(Args[0]) || visit(Args[1]);
555+
if (OpName == "||") return visit(E->getLHS()) && visit(E->getRHS());
556+
if (OpName == "&&") return visit(E->getLHS()) || visit(E->getRHS());
563557
llvm_unreachable("unsupported binary operator");
564558
}
565559

@@ -592,9 +586,8 @@ static bool isPlatformConditionDisjunction(Expr *E, PlatformConditionKind Kind,
592586
ArrayRef<StringRef> Vals) {
593587
if (auto *Or = dyn_cast<BinaryExpr>(E)) {
594588
if (getDeclRefStr(Or->getFn()) == "||") {
595-
auto Args = Or->getArg()->getElements();
596-
return (isPlatformConditionDisjunction(Args[0], Kind, Vals) &&
597-
isPlatformConditionDisjunction(Args[1], Kind, Vals));
589+
return (isPlatformConditionDisjunction(Or->getLHS(), Kind, Vals) &&
590+
isPlatformConditionDisjunction(Or->getRHS(), Kind, Vals));
598591
}
599592
} else if (auto *P = dyn_cast<ParenExpr>(E)) {
600593
return isPlatformConditionDisjunction(P->getSubExpr(), Kind, Vals);
@@ -655,11 +648,10 @@ static Expr *findAnyLikelySimulatorEnvironmentTest(Expr *Condition) {
655648

656649
if (auto *And = dyn_cast<BinaryExpr>(Condition)) {
657650
if (getDeclRefStr(And->getFn()) == "&&") {
658-
auto Args = And->getArg()->getElements();
659-
if ((isSimulatorPlatformOSTest(Args[0]) &&
660-
isSimulatorPlatformArchTest(Args[1])) ||
661-
(isSimulatorPlatformOSTest(Args[1]) &&
662-
isSimulatorPlatformArchTest(Args[0]))) {
651+
if ((isSimulatorPlatformOSTest(And->getLHS()) &&
652+
isSimulatorPlatformArchTest(And->getRHS())) ||
653+
(isSimulatorPlatformOSTest(And->getRHS()) &&
654+
isSimulatorPlatformArchTest(And->getLHS()))) {
663655
return And;
664656
}
665657
}

‎lib/Sema/CSDiagnostics.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ bool MissingConformanceFailure::diagnoseAsError() {
457457
if (isPatternMatchingOperator(anchor)) {
458458
auto *expr = castToExpr(anchor);
459459
if (auto *binaryOp = dyn_cast_or_null<BinaryExpr>(findParentExpr(expr))) {
460-
auto *caseExpr = binaryOp->getArg()->getElement(0);
460+
auto *caseExpr = binaryOp->getLHS();
461461

462462
llvm::SmallPtrSet<Expr *, 4> anchors;
463463
for (const auto *fix : getSolution().Fixes) {
@@ -4080,11 +4080,9 @@ bool AllowTypeOrInstanceMemberFailure::diagnoseAsError() {
40804080
ValueDecl *decl0 = overloadedFn->getDecls()[0];
40814081

40824082
if (decl0->getBaseName() == decl0->getASTContext().Id_MatchOperator) {
4083-
assert(binaryExpr->getArg()->getElements().size() == 2);
4084-
40854083
// If the rhs of '~=' is the enum type, a single dot suffixes
40864084
// since the type can be inferred
4087-
Type secondArgType = getType(binaryExpr->getArg()->getElement(1));
4085+
Type secondArgType = getType(binaryExpr->getRHS());
40884086
if (secondArgType->isEqual(baseTy)) {
40894087
Diag->fixItInsert(loc, ".");
40904088
return true;
@@ -6154,8 +6152,8 @@ bool ArgumentMismatchFailure::diagnoseUseOfReferenceEqualityOperator() const {
61546152
return false;
61556153

61566154
auto *binaryOp = castToExpr<BinaryExpr>(getRawAnchor());
6157-
auto *lhs = binaryOp->getArg()->getElement(0);
6158-
auto *rhs = binaryOp->getArg()->getElement(1);
6155+
auto *lhs = binaryOp->getLHS();
6156+
auto *rhs = binaryOp->getRHS();
61596157

61606158
auto name = *getOperatorName(binaryOp->getFn());
61616159

@@ -6221,8 +6219,8 @@ bool ArgumentMismatchFailure::diagnosePatternMatchingMismatch() const {
62216219
return false;
62226220

62236221
auto *op = castToExpr<BinaryExpr>(getRawAnchor());
6224-
auto *lhsExpr = op->getArg()->getElement(0);
6225-
auto *rhsExpr = op->getArg()->getElement(1);
6222+
auto *lhsExpr = op->getLHS();
6223+
auto *rhsExpr = op->getRHS();
62266224

62276225
auto lhsType = getType(lhsExpr);
62286226
auto rhsType = getType(rhsExpr);

‎lib/Sema/CSSolver.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -884,8 +884,8 @@ void ConstraintSystem::shrink(Expr *expr) {
884884
return isArithmeticExprOfLiterals(postfix->getArg());
885885

886886
if (auto binary = dyn_cast<BinaryExpr>(expr))
887-
return isArithmeticExprOfLiterals(binary->getArg()->getElement(0)) &&
888-
isArithmeticExprOfLiterals(binary->getArg()->getElement(1));
887+
return isArithmeticExprOfLiterals(binary->getLHS()) &&
888+
isArithmeticExprOfLiterals(binary->getRHS());
889889

890890
return isa<IntegerLiteralExpr>(expr) || isa<FloatLiteralExpr>(expr);
891891
}

‎lib/Sema/ConstraintSystem.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3200,8 +3200,8 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
32003200

32013201
const auto &solution = solutions.front();
32023202
if (auto *binaryOp = dyn_cast<BinaryExpr>(applyExpr)) {
3203-
auto *lhs = binaryOp->getArg()->getElement(0);
3204-
auto *rhs = binaryOp->getArg()->getElement(1);
3203+
auto *lhs = binaryOp->getLHS();
3204+
auto *rhs = binaryOp->getRHS();
32053205

32063206
auto lhsType =
32073207
solution.simplifyType(solution.getType(lhs))->getRValueType();

‎lib/Sema/DerivedConformanceAdditiveArithmetic.cpp‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,8 @@ deriveBodyMathOperator(AbstractFunctionDecl *funcDecl, MathOperator op) {
143143
DeclNameLoc(), /*Implicit*/ true);
144144
auto *rhsArg = new (C) MemberRefExpr(rhsDRE, SourceLoc(), member,
145145
DeclNameLoc(), /*Implicit*/ true);
146-
auto *memberOpArgs =
147-
TupleExpr::create(C, SourceLoc(), {lhsArg, rhsArg}, {}, {}, SourceLoc(),
148-
/*HasTrailingClosure*/ false,
149-
/*Implicit*/ true);
150-
auto *memberOpCallExpr =
151-
new (C) BinaryExpr(memberOpExpr, memberOpArgs, /*Implicit*/ true);
152-
return memberOpCallExpr;
146+
return BinaryExpr::create(C, lhsArg, memberOpExpr, rhsArg,
147+
/*implicit*/ true);
153148
};
154149

155150
// Create array of member operator call expressions.

0 commit comments

Comments
 (0)