Compile no line events for the arms a literal predicate rules out · jruby/jruby@89c158f · GitHub
Skip to content

Commit 89c158f

Browse files
sferiksferik
committed
Compile no line events for the arms a literal predicate rules out
MRI folds a conditional on a literal away (`if false`, `unless true`, `x = 1 unless true`): it compiles neither the arm that cannot run nor, when only literals decide it, the predicate, and a statement made of such a conditional starts with the first instruction of the arm that runs. JRuby marked every one of those lines and counted the predicate's, so the folded `if` line read 1 and the dead arm's lines 0 where MRI reports nil, and a void `if false ... end` read as a statement that ran. new_if now records whether the predicate folds and unmarks the dead arm, LineEvents starts a folded conditional at its live arm, and a folded conditional whose live arm compiles to nothing compiles to nothing itself. A constant predicate that still runs code, as in `if x and false`, keeps its line event. The constant predicate is now found once, in new_if, so markBranch no longer needs the predicate.
1 parent 6fa7726 commit 89c158f

6 files changed

Lines changed: 105 additions & 31 deletions

File tree

‎core/src/main/java/org/jruby/ast/IfNode.java‎

Lines changed: 13 additions & 0 deletions

‎core/src/main/java/org/jruby/ast/util/LineEvents.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private static Node firstEvaluated(Node node) {
5757
case Match3Node match -> match.getReceiverNode();
5858
case AndNode and -> and.getFirstNode();
5959
case OrNode or -> or.getFirstNode();
60-
case IfNode ifNode -> ifNode.getCondition();
60+
case IfNode ifNode -> liveCode(ifNode);
6161
case CaseNode caseNode -> caseNode.getCaseNode() != null ? caseNode.getCaseNode() : firstWhen(caseNode.getCases());
6262
case Colon2Node path -> path.getLeftNode();
6363
case DotNode dot -> dot.getBeginNode();
@@ -77,6 +77,16 @@ private static Node firstEvaluated(Node node) {
7777
};
7878
}
7979

80+
// MRI compiles only the arm that can run of a conditional on a literal (see IfNode#hasConstantPredicate), and
81+
// nothing for the predicate when it is made of literals only.
82+
private static Node liveCode(IfNode ifNode) {
83+
if (!ifNode.hasFoldedPredicate()) return ifNode.getCondition();
84+
85+
Node arm = ifNode.isConstantlyTrue() ? ifNode.getThenBody() : ifNode.getElseBody();
86+
87+
return arm instanceof NilImplicitNode ? null : arm;
88+
}
89+
8090
// Targets with a receiver have it evaluated before the value.
8191
private static Node firstReceiver(ListNode targets, Node value) {
8292
if (targets != null) {

‎core/src/main/java/org/jruby/parser/RubyParser.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,7 +2216,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
22162216
/*%%%*/
22172217
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[0+yyTop].value), p.remove_begin(((Node)yyVals[-2+yyTop].value)), null);
22182218
p.fixpos(((Node)yyVal), ((Node)yyVals[0+yyTop].value));
2219-
p.branch_modifier(((Node)yyVal), ((Node)yyVals[0+yyTop].value), ((Node)yyVals[-2+yyTop].value), false, yyVals[yyTop - count + 3].end);
2219+
p.branch_modifier(((Node)yyVal), ((Node)yyVals[-2+yyTop].value), false, yyVals[yyTop - count + 3].end);
22202220
/*% %*/
22212221
/*% ripper: if_mod!($3, $1) %*/
22222222
return yyVal;
@@ -2225,7 +2225,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
22252225
/*%%%*/
22262226
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[0+yyTop].value), null, p.remove_begin(((Node)yyVals[-2+yyTop].value)));
22272227
p.fixpos(((Node)yyVal), ((Node)yyVals[0+yyTop].value));
2228-
p.branch_modifier(((Node)yyVal), ((Node)yyVals[0+yyTop].value), ((Node)yyVals[-2+yyTop].value), true, yyVals[yyTop - count + 3].end);
2228+
p.branch_modifier(((Node)yyVal), ((Node)yyVals[-2+yyTop].value), true, yyVals[yyTop - count + 3].end);
22292229
/*% %*/
22302230
/*% ripper: unless_mod!($3, $1) %*/
22312231
return yyVal;
@@ -3732,7 +3732,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
37323732
p.value_expr(((Node)yyVals[-5+yyTop].value));
37333733
/* Each arm of a ternary is a statement of its own for line events, as in MRI.*/
37343734
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[-5+yyTop].value), p.newline_node(((Node)yyVals[-3+yyTop].value), yyVals[yyTop - count + 3].start()), p.newline_node(((Node)yyVals[0+yyTop].value), yyVals[yyTop - count + 6].start()));
3735-
p.branch_ternary(((Node)yyVal), ((Node)yyVals[-5+yyTop].value), yyVals[yyTop - count + 1].end);
3735+
p.branch_ternary(((Node)yyVal), yyVals[yyTop - count + 1].end);
37363736
/*% %*/
37373737
/*% ripper: ifop!($1, $3, $6) %*/
37383738
return yyVal;
@@ -4240,15 +4240,15 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
42404240
states[372] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
42414241
/*%%%*/
42424242
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[-4+yyTop].value), ((Node)yyVals[-2+yyTop].value), ((Node)yyVals[-1+yyTop].value));
4243-
p.branch_if(((Node)yyVal), ((Node)yyVals[-4+yyTop].value), yyVals[yyTop - count + 2].end, ((Node)yyVals[-1+yyTop].value) == null ? -1 : yyVals[yyTop - count + 5].start, yyVals[yyTop - count + 6].end);
4243+
p.branch_if(((Node)yyVal), yyVals[yyTop - count + 2].end, ((Node)yyVals[-1+yyTop].value) == null ? -1 : yyVals[yyTop - count + 5].start, yyVals[yyTop - count + 6].end);
42444244
/*% %*/
42454245
/*% ripper: if!($2, $4, escape_Qundef($5)) %*/
42464246
return yyVal;
42474247
};
42484248
states[373] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
42494249
/*%%%*/
42504250
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[-4+yyTop].value), ((Node)yyVals[-1+yyTop].value), ((Node)yyVals[-2+yyTop].value));
4251-
p.branch_unless(((Node)yyVal), ((Node)yyVals[-4+yyTop].value), yyVals[yyTop - count + 2].end);
4251+
p.branch_unless(((Node)yyVal), yyVals[yyTop - count + 2].end);
42524252
/*% %*/
42534253
/*% ripper: unless!($2, $4, escape_Qundef($5)) %*/
42544254
return yyVal;
@@ -4564,7 +4564,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
45644564
states[424] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
45654565
/*%%%*/
45664566
yyVal = p.new_if(yyVals[yyTop - count + 1].start(), ((Node)yyVals[-3+yyTop].value), ((Node)yyVals[-1+yyTop].value), ((Node)yyVals[0+yyTop].value));
4567-
p.branch_elsif(((Node)yyVal), ((Node)yyVals[-3+yyTop].value), yyVals[yyTop - count + 2].end, ((Node)yyVals[0+yyTop].value) == null ? -1 : yyVals[yyTop - count + 5].start);
4567+
p.branch_elsif(((Node)yyVal), yyVals[yyTop - count + 2].end, ((Node)yyVals[0+yyTop].value) == null ? -1 : yyVals[yyTop - count + 5].start);
45684568
/*% %*/
45694569
/*% ripper: elsif!($2, $4, escape_Qundef($5)) %*/
45704570
return yyVal;

‎core/src/main/java/org/jruby/parser/RubyParser.y‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,15 +628,15 @@ stmt : keyword_alias fitem {
628628
/*%%%*/
629629
$$ = p.new_if(@1.start(), $3, p.remove_begin($1), null);
630630
p.fixpos($<Node>$, $3);
631-
p.branch_modifier($<Node>$, $3, $1, false, @3.end);
631+
p.branch_modifier($<Node>$, $1, false, @3.end);
632632
/*% %*/
633633
/*% ripper: if_mod!($3, $1) %*/
634634
}
635635
| stmt modifier_unless expr_value {
636636
/*%%%*/
637637
$$ = p.new_if(@1.start(), $3, null, p.remove_begin($1));
638638
p.fixpos($<Node>$, $3);
639-
p.branch_modifier($<Node>$, $3, $1, true, @3.end);
639+
p.branch_modifier($<Node>$, $1, true, @3.end);
640640
/*% %*/
641641
/*% ripper: unless_mod!($3, $1) %*/
642642
}
@@ -1956,7 +1956,7 @@ arg : lhs '=' lex_ctxt arg_rhs {
19561956
p.value_expr($1);
19571957
// Each arm of a ternary is a statement of its own for line events, as in MRI.
19581958
$$ = p.new_if(@1.start(), $1, p.newline_node($3, @3.start()), p.newline_node($6, @6.start()));
1959-
p.branch_ternary($<Node>$, $1, @1.end);
1959+
p.branch_ternary($<Node>$, @1.end);
19601960
/*% %*/
19611961
/*% ripper: ifop!($1, $3, $6) %*/
19621962
}
@@ -2428,14 +2428,14 @@ primary : literal
24282428
| k_if expr_value then compstmt if_tail k_end {
24292429
/*%%%*/
24302430
$$ = p.new_if(@1.start(), $2, $4, $5);
2431-
p.branch_if($<Node>$, $2, @2.end, $5 == null ? -1 : @5.start, @6.end);
2431+
p.branch_if($<Node>$, @2.end, $5 == null ? -1 : @5.start, @6.end);
24322432
/*% %*/
24332433
/*% ripper: if!($2, $4, escape_Qundef($5)) %*/
24342434
}
24352435
| k_unless expr_value then compstmt opt_else k_end {
24362436
/*%%%*/
24372437
$$ = p.new_if(@1.start(), $2, $5, $4);
2438-
p.branch_unless($<Node>$, $2, @2.end);
2438+
p.branch_unless($<Node>$, @2.end);
24392439
/*% %*/
24402440
/*% ripper: unless!($2, $4, escape_Qundef($5)) %*/
24412441
}
@@ -2724,7 +2724,7 @@ if_tail : opt_else
27242724
| k_elsif expr_value then compstmt if_tail {
27252725
/*%%%*/
27262726
$$ = p.new_if(@1.start(), $2, $4, $5);
2727-
p.branch_elsif($<Node>$, $2, @2.end, $5 == null ? -1 : @5.start);
2727+
p.branch_elsif($<Node>$, @2.end, $5 == null ? -1 : @5.start);
27282728
/*% %*/
27292729
/*% ripper: elsif!($2, $4, escape_Qundef($5)) %*/
27302730
};

‎core/src/main/java/org/jruby/parser/RubyParserBase.java‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -618,16 +618,26 @@ private static boolean staticArrayElement(Node node) {
618618
}
619619
}
620620

621-
private static void markBranch(Node node, Node predicate, boolean unless, boolean elsif, long predicateEnd, long elseStart) {
622-
if (node instanceof IfNode ifNode) {
623-
ifNode.markBranch(unless, elsif, predicateEnd, elseStart);
624-
ifNode.setConstantPredicate(constantPredicate(predicate));
621+
/**
622+
* Whether MRI compiles a constant predicate to jumps alone: it is made only of literals, joined by and/or.
623+
*/
624+
private static boolean compilesToJumpOnly(Node node) {
625+
if (node instanceof AndNode and && and.getParenSpan() == null) {
626+
return compilesToJumpOnly(and.getFirstNode()) && compilesToJumpOnly(and.getSecondNode());
627+
} else if (node instanceof OrNode or && or.getParenSpan() == null) {
628+
return compilesToJumpOnly(or.getFirstNode()) && compilesToJumpOnly(or.getSecondNode());
625629
}
630+
631+
return jumpTargets(node) != (TO_THEN | TO_ELSE);
632+
}
633+
634+
private static void markBranch(Node node, boolean unless, boolean elsif, long predicateEnd, long elseStart) {
635+
if (node instanceof IfNode ifNode) ifNode.markBranch(unless, elsif, predicateEnd, elseStart);
626636
}
627637

628638
/** if ... [elsif ...] [else ...] end */
629-
public Node branch_if(Node node, Node predicate, long predicateEnd, long elseStart, long end) {
630-
markBranch(node, predicate, false, false, predicateEnd, elseStart);
639+
public Node branch_if(Node node, long predicateEnd, long elseStart, long end) {
640+
markBranch(node, false, false, predicateEnd, elseStart);
631641
if (node instanceof IfNode ifNode) {
632642
// MRI reports every elsif clause as reaching the shared 'end'
633643
for (Node tail = ifNode.getElseBody(); tail instanceof IfNode elsif && elsif.isElsif(); tail = elsif.getElseBody()) {
@@ -638,26 +648,26 @@ public Node branch_if(Node node, Node predicate, long predicateEnd, long elseSta
638648
}
639649

640650
/** unless ... [else ...] end (then/else are swapped in the IfNode) */
641-
public Node branch_unless(Node node, Node predicate, long predicateEnd) {
642-
markBranch(node, predicate, true, false, predicateEnd, -1);
651+
public Node branch_unless(Node node, long predicateEnd) {
652+
markBranch(node, true, false, predicateEnd, -1);
643653
return node;
644654
}
645655

646656
/** elsif ... (nested as the else body of the enclosing if) */
647-
public Node branch_elsif(Node node, Node predicate, long predicateEnd, long elseStart) {
648-
markBranch(node, predicate, false, true, predicateEnd, elseStart);
657+
public Node branch_elsif(Node node, long predicateEnd, long elseStart) {
658+
markBranch(node, false, true, predicateEnd, elseStart);
649659
return node;
650660
}
651661

652662
/** cond ? a : b */
653-
public Node branch_ternary(Node node, Node predicate, long predicateEnd) {
654-
markBranch(node, predicate, false, false, predicateEnd, -1);
663+
public Node branch_ternary(Node node, long predicateEnd) {
664+
markBranch(node, false, false, predicateEnd, -1);
655665
return node;
656666
}
657667

658668
/** stmt if cond / stmt unless cond */
659-
public Node branch_modifier(Node node, Node predicate, Node statement, boolean unless, long predicateEnd) {
660-
markBranch(node, predicate, unless, false, predicateEnd, -1);
669+
public Node branch_modifier(Node node, Node statement, boolean unless, long predicateEnd) {
670+
markBranch(node, unless, false, predicateEnd, -1);
661671
if (node instanceof IfNode ifNode) ifNode.setSourceBody(statement);
662672
return node;
663673
}
@@ -1274,7 +1284,7 @@ public Node void_stmts(Node node) {
12741284
/**
12751285
* Whether MRI compiles nothing for this statement when its value is not used: a statement
12761286
* {@link #eliminatedWhenUnused} finds, a local variable assigned to itself (which MRI's peephole optimizer
1277-
* removes), or a list of them.
1287+
* removes), a conditional on a literal whose arm that can run is one, or a list of them.
12781288
*/
12791289
private static boolean compilesToNothingWhenUnused(Node node) {
12801290
return switch (node) {
@@ -1286,6 +1296,8 @@ private static boolean compilesToNothingWhenUnused(Node node) {
12861296
}
12871297
yield true;
12881298
}
1299+
case IfNode ifNode -> ifNode.hasFoldedPredicate() &&
1300+
compilesToNothingWhenUnused(ifNode.isConstantlyTrue() ? ifNode.getThenBody() : ifNode.getElseBody());
12891301
case LocalAsgnNode asgn -> isSameVariable(asgn, asgn.getValueNode());
12901302
case DAsgnNode asgn -> isSameVariable(asgn, asgn.getValueNode());
12911303
default -> eliminatedWhenUnused(node);
@@ -1433,13 +1445,26 @@ public Node method_cond(Node node) {
14331445
public Node new_if(int line, Node condition, Node thenNode, Node elseNode) {
14341446
if (condition == null) return elseNode;
14351447

1448+
Node rawCondition = condition;
1449+
int constantPredicate = constantPredicate(condition);
14361450
condition = cond0(condition, ConditionType.IN_COND);
14371451

14381452
IfNode ifNode = new IfNode(line, condition, thenNode, elseNode);
1453+
ifNode.setConstantPredicate(constantPredicate);
1454+
ifNode.setFoldedPredicate(constantPredicate != 0 && compilesToJumpOnly(rawCondition));
1455+
1456+
// MRI folds a conditional on a literal away: the arm that cannot run is not compiled, so it has no line
1457+
// events.
1458+
if (constantPredicate != 0) uncoverAll(constantPredicate > 0 ? elseNode : thenNode);
14391459

1440-
// As in MRI, a conditional's predicate is a line event wherever the conditional is (an IfNode is a newline
1441-
// node), so its line is coverable.
1442-
cover(ifNode);
1460+
if (ifNode.hasFoldedPredicate()) {
1461+
// Nor is a predicate of literals, so there is no line event for it either
1462+
ifNode.unsetNewline();
1463+
} else {
1464+
// As in MRI, a conditional's predicate is a line event wherever the conditional is (an IfNode is a
1465+
// newline node), so its line is coverable.
1466+
cover(ifNode);
1467+
}
14431468

14441469
return ifNode;
14451470
}

‎test/jruby/test_coverage.rb‎

Lines changed: 26 additions & 0 deletions

0 commit comments

Comments
 (0)