Skip to content

Commit 88b315f

Browse files
andrykonchinandrykonchin
committed
Don't duplicate when clause body if when clause contains multiple conditions
1 parent 285cb98 commit 88b315f

File tree

3 files changed

+97
-92
lines changed

3 files changed

+97
-92
lines changed

‎spec/truffle/parsing/fixtures/case/with_expression_and_when/with_multiple_values.yaml

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
subject: "case expression"
22
description: "with expression to match / and multiple values in a `when` branch (case exp when a, b ... end)"
33
notes: >
4-
A list of values in a `when` expression is implemented as multiple nested `if` statements
4+
A list of values in a `when` expression is implemented as `if` operator
55
to compare each value in a list.
66
77
So the example below is translated to the following expression:
88
99
%case_1 = 42
10-
if 42 === %case_1
11-
"forty two"
12-
if 42.0 === %case_1
10+
if 42 === %case_1 || 42.0 === %case_1
1311
"forty two"
1412
else
1513
nil
1614
end
15+
yarp_specific: true # Simplified AST and removed duplication of `when`'s body for each `when`'s condition
1716
focused_on_node: "org.truffleruby.language.control.SequenceNode"
1817
ruby: |
1918
case 42
@@ -55,30 +54,30 @@ ast: |
5554
flags = 0
5655
children:
5756
condition =
58-
InlinedCaseEqualNodeGen
57+
OrNodeGen
5958
attributes:
60-
assumptions = [Assumption(valid, name=set_trace_func is not used)]
6159
flags = 0
62-
integerCaseEqualAssumption = Assumption(valid, name=inlined Integer#===)
63-
parameters = org.truffleruby.language.dispatch.RubyCallNodeParameters@...
6460
children:
65-
leftNode_ =
66-
IntegerFixnumLiteralNode
67-
attributes:
68-
flags = 0
69-
value = 42
70-
rightNode_ =
71-
ReadLocalVariableNode
61+
left =
62+
InlinedCaseEqualNodeGen
7263
attributes:
64+
assumptions = [Assumption(valid, name=set_trace_func is not used)]
7365
flags = 0
74-
frameSlot = 2
75-
type = FRAME_LOCAL
76-
elseBody =
77-
IfElseNodeGen
78-
attributes:
79-
flags = 0
80-
children:
81-
condition =
66+
integerCaseEqualAssumption = Assumption(valid, name=inlined Integer#===)
67+
parameters = org.truffleruby.language.dispatch.RubyCallNodeParameters@...
68+
children:
69+
leftNode_ =
70+
IntegerFixnumLiteralNode
71+
attributes:
72+
flags = 0
73+
value = 42
74+
rightNode_ =
75+
ReadLocalVariableNode
76+
attributes:
77+
flags = 0
78+
frameSlot = 2
79+
type = FRAME_LOCAL
80+
right =
8281
InlinedCaseEqualNodeGen
8382
attributes:
8483
assumptions = [Assumption(valid, name=set_trace_func is not used)]
@@ -97,17 +96,11 @@ ast: |
9796
flags = 0
9897
frameSlot = 2
9998
type = FRAME_LOCAL
100-
elseBody =
101-
NilLiteralNode
102-
attributes:
103-
flags = 0
104-
isImplicit = false
105-
thenBody =
106-
StringLiteralNode
107-
attributes:
108-
encoding = UTF-8
109-
flags = 1
110-
tstring = forty two
99+
elseBody =
100+
NilLiteralNode
101+
attributes:
102+
flags = 0
103+
isImplicit = false
111104
thenBody =
112105
StringLiteralNode
113106
attributes:

‎spec/truffle/parsing/fixtures/case/without_expression_and_when/with_multiple_values.yaml

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
subject: "case expression"
22
description: "without expression to match / with multiple values in a `when` branch (case when a, b ... end)"
33
notes: >
4-
A list of conditions in a `when` expression is represented as multiple nested `if` statements
4+
A list of conditions in a `when` expression is represented as a single `if` operator
55
to evaluate each of conditions.
66
77
So the example below is translated to the following expression:
88
9-
if (true)
10-
"1st or 2d condition"
11-
else (false)
9+
if true || false
1210
"1st or 2d condition"
1311
else
1412
nil
1513
end
14+
yarp_specific: true # Simplified AST and removed duplication of `when`'s body for each `when`'s condition
1615
focused_on_node: "org.truffleruby.language.control.IfElseNode"
1716
ruby: |
1817
case
@@ -25,31 +24,25 @@ ast: |
2524
flags = 1
2625
children:
2726
condition =
28-
BooleanLiteralNode
29-
attributes:
30-
flags = 0
31-
value = true
32-
elseBody =
33-
IfElseNodeGen
27+
OrNodeGen
3428
attributes:
3529
flags = 0
3630
children:
37-
condition =
31+
left =
3832
BooleanLiteralNode
3933
attributes:
4034
flags = 0
41-
value = false
42-
elseBody =
43-
NilLiteralNode
35+
value = true
36+
right =
37+
BooleanLiteralNode
4438
attributes:
4539
flags = 0
46-
isImplicit = false
47-
thenBody =
48-
StringLiteralNode
49-
attributes:
50-
encoding = UTF-8
51-
flags = 1
52-
tstring = 1st or 2d condition
40+
value = false
41+
elseBody =
42+
NilLiteralNode
43+
attributes:
44+
flags = 0
45+
isImplicit = false
5346
thenBody =
5447
StringLiteralNode
5548
attributes:

‎src/main/java/org/truffleruby/parser/YARPTranslator.java

Lines changed: 56 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -541,13 +541,7 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
541541

542542
final Nodes.WhenNode when = (Nodes.WhenNode) conditions[n];
543543
final Nodes.Node[] whenConditions = when.conditions;
544-
545-
boolean containSplatOperator = false;
546-
for (Nodes.Node value : whenConditions) {
547-
if (value instanceof Nodes.SplatNode) {
548-
containSplatOperator = true;
549-
}
550-
}
544+
boolean containSplatOperator = containYARPSplatNode(whenConditions);
551545

552546
if (containSplatOperator) {
553547
final RubyNode receiver = new TruffleInternalModuleLiteralNode();
@@ -562,20 +556,32 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
562556
// this `if` becomes `else` branch of the outer `if`
563557
elseNode = ifNode;
564558
} else {
565-
// TODO: we duplicate `then` for each when' condition, does it make sense to avoid it?
566-
for (int k = whenConditions.length - 1; k >= 0; k--) {
567-
final var whenCondition = whenConditions[k];
559+
// translate `when` with multiple expressions into a single `if` operator, e.g.
560+
// case x
561+
// when a, b, c
562+
// is translated into
563+
// if x === a || x === b || x === c
564+
565+
RubyNode predicateNode = null;
566+
567+
for (var whenCondition : whenConditions) {
568568
final RubyNode receiver = whenCondition.accept(this);
569569
final RubyNode[] arguments = new RubyNode[]{ NodeUtil.cloneNode(readTemp) };
570-
final RubyNode predicateNode = createCallNode(receiver, "===", arguments);
570+
final RubyNode nextPredicateNode = createCallNode(receiver, "===", arguments);
571571

572-
// create `if` node
573-
final RubyNode thenNode = translateNodeOrNil(when.statements);
574-
final IfElseNode ifNode = IfElseNodeGen.create(predicateNode, thenNode, elseNode);
575-
576-
// this `if` becomes `else` branch of the outer `if`
577-
elseNode = ifNode;
572+
if (predicateNode == null) {
573+
predicateNode = nextPredicateNode;
574+
} else {
575+
predicateNode = OrNodeGen.create(predicateNode, nextPredicateNode);
576+
}
578577
}
578+
579+
// create `if` node
580+
final RubyNode thenNode = translateNodeOrNil(when.statements);
581+
final IfElseNode ifNode = IfElseNodeGen.create(predicateNode, thenNode, elseNode);
582+
583+
// this `if` becomes `else` branch of the outer `if`
584+
elseNode = ifNode;
579585
}
580586
}
581587

@@ -596,27 +602,33 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
596602

597603
final Nodes.WhenNode when = (Nodes.WhenNode) conditions[n];
598604
final Nodes.Node[] whenConditions = when.conditions;
599-
600-
boolean containSplatOperator = false;
601-
for (Nodes.Node value : whenConditions) {
602-
if (value instanceof Nodes.SplatNode) {
603-
containSplatOperator = true;
604-
}
605-
}
605+
boolean containSplatOperator = containYARPSplatNode(whenConditions);
606606

607607
if (!containSplatOperator) {
608-
for (int k = whenConditions.length - 1; k >= 0; k--) {
609-
final var whenCondition = whenConditions[k];
608+
// translate `when` with multiple expressions into a single `if` operator, e.g.
609+
// case
610+
// when a, b, c
611+
// is translated into
612+
// if a || b || c
613+
614+
RubyNode predicateNode = null;
610615

611-
// create `if` node
612-
final RubyNode predicateNode = whenCondition.accept(this);
613-
// TODO: we duplicate `then` for each when' condition, does it make sense to avoid it?
614-
final RubyNode thenNode = translateNodeOrNil(when.statements);
615-
final IfElseNode ifNode = IfElseNodeGen.create(predicateNode, thenNode, elseNode);
616+
for (var whenCondition : whenConditions) {
617+
final RubyNode nextPredicateNode = whenCondition.accept(this);
616618

617-
// this `if` becomes `else` branch of the outer `if`
618-
elseNode = ifNode;
619+
if (predicateNode == null) {
620+
predicateNode = nextPredicateNode;
621+
} else {
622+
predicateNode = OrNodeGen.create(predicateNode, nextPredicateNode);
623+
}
619624
}
625+
626+
// create `if` node
627+
final RubyNode thenNode = translateNodeOrNil(when.statements);
628+
final IfElseNode ifNode = IfElseNodeGen.create(predicateNode, thenNode, elseNode);
629+
630+
// this `if` becomes `else` branch of the outer `if`
631+
elseNode = ifNode;
620632
} else {
621633
// use Array#any? to check whether there is any truthy value
622634
// whenConditions are translated into an array-producing node
@@ -632,8 +644,6 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
632644
// this `if` becomes `else` branch of the outer `if`
633645
elseNode = ifNode;
634646
}
635-
636-
637647
}
638648

639649
rubyNode = elseNode;
@@ -1533,11 +1543,11 @@ private RubyNode translateExpressionsList(Nodes.Node[] nodes) {
15331543
assert nodes != null;
15341544
assert nodes.length > 0;
15351545

1536-
boolean isSplatNodePresent = Arrays.stream(nodes).anyMatch(n -> n instanceof Nodes.SplatNode);
1546+
boolean containSplatOperator = containYARPSplatNode(nodes);
15371547

15381548
// fast path (no SplatNode)
15391549

1540-
if (!isSplatNodePresent) {
1550+
if (!containSplatOperator) {
15411551
RubyNode[] rubyNodes = new RubyNode[nodes.length];
15421552

15431553
for (int i = 0; i < nodes.length; i++) {
@@ -1961,4 +1971,13 @@ private void assignNodePositionInSource(Nodes.Node[] nodes, RubyNode rubyNode) {
19611971
rubyNode.unsafeSetSourceSection(first.startOffset, length);
19621972
}
19631973

1974+
private boolean containYARPSplatNode(Nodes.Node[] nodes) {
1975+
for (var n : nodes) {
1976+
if (n instanceof Nodes.SplatNode) {
1977+
return true;
1978+
}
1979+
}
1980+
1981+
return false;
1982+
}
19641983
}

0 commit comments

Comments
 (0)