Skip to content

Commit 86dbccd

Browse files
lukesandbergcopybara-github
authored andcommitted
Fix an associativity issue that prevents the peephole pass from folding assignment operators.
PiperOrigin-RevId: 728217236
1 parent b99e83b commit 86dbccd

File tree

2 files changed

+48
-45
lines changed

2 files changed

+48
-45
lines changed

src/com/google/javascript/jscomp/PeepholeFoldConstants.java

+46-45
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,28 @@ private Node tryFoldAssign(Node n, Node left, Node right) {
490490
return n;
491491
}
492492

493+
// Only certain RHS operators are supported.
494+
var operator = right.getToken();
495+
Token newType =
496+
switch (operator) {
497+
case ADD -> Token.ASSIGN_ADD;
498+
case BITAND -> Token.ASSIGN_BITAND;
499+
case BITOR -> Token.ASSIGN_BITOR;
500+
case BITXOR -> Token.ASSIGN_BITXOR;
501+
case DIV -> Token.ASSIGN_DIV;
502+
case LSH -> Token.ASSIGN_LSH;
503+
case MOD -> Token.ASSIGN_MOD;
504+
case MUL -> Token.ASSIGN_MUL;
505+
case RSH -> Token.ASSIGN_RSH;
506+
case SUB -> Token.ASSIGN_SUB;
507+
case URSH -> Token.ASSIGN_URSH;
508+
case EXPONENT -> Token.ASSIGN_EXPONENT;
509+
default -> null;
510+
};
511+
if (newType == null) {
512+
return n;
513+
}
514+
493515
// Tries to convert x = x + y -> x += y;
494516
if (!right.hasChildren() || right.getSecondChild() != right.getLastChild()) {
495517
// RHS must have two children.
@@ -500,60 +522,39 @@ private Node tryFoldAssign(Node n, Node left, Node right) {
500522
return n;
501523
}
502524

525+
// We can fold any 'leaf' that
526+
// 1. is equal to the left side of the assignment
527+
// 2. is either the first child of the operator or any child of the operator if the operator
528+
// is commutative
529+
// 3. Also consider recursively discoverable children of the operator if the operator is
530+
// associative.
531+
//
532+
// For example, consider x = x + y + z;
533+
// because + is left associative, the tree looks like `(x + y) + z` so we need to look deeper
534+
// into the tree to find x.
535+
// Right now we only recurse one level deep which allows use to handle x = x + y + z but not
536+
// x = x + y + z + w.
537+
503538
Node newRight;
504539
if (areNodesEqualForInlining(left, right.getFirstChild())) {
505540
newRight = right.getLastChild();
506-
} else if (NodeUtil.isCommutative(right.getToken())
541+
} else if (NodeUtil.isCommutative(operator)
507542
&& areNodesEqualForInlining(left, right.getLastChild())) {
508543
newRight = right.getFirstChild();
544+
} else if (NodeUtil.isAssociative(operator)
545+
&& right.getFirstChild().getToken() == operator
546+
&& areNodesEqualForInlining(left, right.getFirstFirstChild())) {
547+
548+
var newLeft = right.getFirstChild().getLastChild().detach();
549+
right.getFirstChild().detach();
550+
right.addChildToFront(newLeft);
551+
newRight = right;
552+
509553
} else {
510554
return n;
511555
}
512556

513-
Token newType = null;
514-
switch (right.getToken()) {
515-
case ADD:
516-
newType = Token.ASSIGN_ADD;
517-
break;
518-
case BITAND:
519-
newType = Token.ASSIGN_BITAND;
520-
break;
521-
case BITOR:
522-
newType = Token.ASSIGN_BITOR;
523-
break;
524-
case BITXOR:
525-
newType = Token.ASSIGN_BITXOR;
526-
break;
527-
case DIV:
528-
newType = Token.ASSIGN_DIV;
529-
break;
530-
case LSH:
531-
newType = Token.ASSIGN_LSH;
532-
break;
533-
case MOD:
534-
newType = Token.ASSIGN_MOD;
535-
break;
536-
case MUL:
537-
newType = Token.ASSIGN_MUL;
538-
break;
539-
case RSH:
540-
newType = Token.ASSIGN_RSH;
541-
break;
542-
case SUB:
543-
newType = Token.ASSIGN_SUB;
544-
break;
545-
case URSH:
546-
newType = Token.ASSIGN_URSH;
547-
break;
548-
case EXPONENT:
549-
newType = Token.ASSIGN_EXPONENT;
550-
break;
551-
default:
552-
return n;
553-
}
554-
555-
Node newNode = new Node(newType,
556-
left.detach(), newRight.detach());
557+
Node newNode = new Node(newType, left.detach(), newRight.detach());
557558
n.replaceWith(newNode);
558559

559560
reportChangeToEnclosingScope(newNode);

test/com/google/javascript/jscomp/PeepholeFoldConstantsTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,8 @@ public void testAssignOpsLate() {
15801580
testSame("x=y-x");
15811581
test("x=x|y", "x|=y");
15821582
test("x=y|x", "x|=y");
1583+
test("x=x|y|z", "x|=y|z");
1584+
testSame("x=x&&y&&z");
15831585
test("x=x*y", "x*=y");
15841586
test("x=y*x", "x*=y");
15851587
test("x=x**y", "x**=y");

0 commit comments

Comments
 (0)