Skip to content

Commit e93c4d7

Browse files
andrykonchinandrykonchin
committed
Optimise safe navigation operator handling for assign operators and set isSafeNavigation = false
1 parent f149a59 commit e93c4d7

File tree

4 files changed

+21
-13
lines changed

4 files changed

+21
-13
lines changed

‎spec/truffle/parsing/fixtures/operators/&&=/attribute_assignment_with_safe_navigation_operator.yaml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ notes: >
1717
(IsNilNode
1818
(ReadLocalVariableNode)) # %value_0
1919
(AndNodeGen ...))
20-
yarp_specific: true # assign true value (instead of false) to isSafeNavigation attribute for a.foo and a.foo= calls
2120
focused_on_node: "org.truffleruby.language.defined.DefinedWrapperNode"
2221
ruby: |
2322
a&.foo &&= 42
@@ -86,7 +85,7 @@ ast: |
8685
emptyKeywordsProfile = false
8786
flags = 0
8887
isAttrAssign = false
89-
isSafeNavigation = true
88+
isSafeNavigation = false
9089
isSplatted = false
9190
isVCall = false
9291
lastArgIsNotHashProfile = false
@@ -108,7 +107,7 @@ ast: |
108107
emptyKeywordsProfile = false
109108
flags = 0
110109
isAttrAssign = true
111-
isSafeNavigation = true
110+
isSafeNavigation = false
112111
isSplatted = false
113112
isVCall = false
114113
lastArgIsNotHashProfile = false

‎spec/truffle/parsing/fixtures/operators/+=/attribute_assignment_with_safe_navigation_operator.yaml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ notes: >
1818
(ReadLocalVariableNode)) # %value_0
1919
(RubyCallNode ...))
2020
yarp_specific: true # fixed assigning method call node and set `isAttrAssign = true`
21-
# assign true value (instead of false) to isSafeNavigation attribute for a.foo and a.foo= calls
2221
focused_on_node: "org.truffleruby.language.control.SequenceNode"
2322
ruby: |
2423
a&.foo += 42
@@ -90,7 +89,7 @@ ast: |
9089
emptyKeywordsProfile = false
9190
flags = 0
9291
isAttrAssign = true
93-
isSafeNavigation = true
92+
isSafeNavigation = false
9493
isSplatted = false
9594
isVCall = false
9695
lastArgIsNotHashProfile = false
@@ -113,7 +112,7 @@ ast: |
113112
emptyKeywordsProfile = false
114113
flags = 0
115114
isAttrAssign = false
116-
isSafeNavigation = true
115+
isSafeNavigation = false
117116
isSplatted = false
118117
isVCall = false
119118
lastArgIsNotHashProfile = false

‎spec/truffle/parsing/fixtures/operators/||=/attribute_assignment_with_safe_navigation_operator.yaml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ notes: >
1717
(IsNilNode
1818
(ReadLocalVariableNode)) # %value_0
1919
(OrNodeGen ...))
20-
yarp_specific: true # assign true value (instead of false) to isSafeNavigation attribute for a.foo and a.foo= calls
2120
focused_on_node: "org.truffleruby.language.defined.DefinedWrapperNode"
2221
ruby: |
2322
a&.foo ||= 42
@@ -86,7 +85,7 @@ ast: |
8685
emptyKeywordsProfile = false
8786
flags = 0
8887
isAttrAssign = false
89-
isSafeNavigation = true
88+
isSafeNavigation = false
9089
isSplatted = false
9190
isVCall = false
9291
lastArgIsNotHashProfile = false
@@ -108,7 +107,7 @@ ast: |
108107
emptyKeywordsProfile = false
109108
flags = 0
110109
isAttrAssign = true
111-
isSafeNavigation = true
110+
isSafeNavigation = false
112111
isSplatted = false
113112
isVCall = false
114113
lastArgIsNotHashProfile = false

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,13 @@ public RubyNode visitCallAndWriteNode(Nodes.CallAndWriteNode node) {
599599
final var writeReceiverNode = receiverExpression.getWriteNode();
600600
final var readReceiver = receiverExpression.getReadYARPNode();
601601

602-
// Use Prism nodes and rely on CallNode translation to automatically set RubyCallNode attributes
602+
// Use Prism nodes and rely on CallNode translation to automatically set RubyCallNode attributes.
603+
// Prism doesn't set ATTRIBUTE_WRITE flag, so we should add it manually
604+
// safe navigation flag is handled separately, so as optimisation remove it from the flags
603605
short writeFlags = (short) (node.flags | Nodes.CallNodeFlags.ATTRIBUTE_WRITE);
604-
final RubyNode readNode = callNode(node, node.flags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY)
606+
writeFlags = (short) (writeFlags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
607+
short readFlags = (short) (node.flags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
608+
final RubyNode readNode = callNode(node, readFlags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY)
605609
.accept(this);
606610
final RubyNode writeNode = callNode(node, writeFlags, readReceiver, node.write_name,
607611
node.value).accept(this);
@@ -787,8 +791,11 @@ public RubyNode visitCallOperatorWriteNode(Nodes.CallOperatorWriteNode node) {
787791
final var readReceiver = receiverExpression.getReadYARPNode();
788792

789793
// Use Prism nodes and rely on CallNode translation to automatically set CallNode flags
794+
// safe navigation flag is handled separately, so as optimisation remove it from the flags
790795
short writeFlags = (short) (node.flags | Nodes.CallNodeFlags.ATTRIBUTE_WRITE);
791-
final Nodes.Node read = callNode(node, node.flags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY);
796+
writeFlags = (short) (writeFlags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
797+
short readFlags = (short) (node.flags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
798+
final Nodes.Node read = callNode(node, readFlags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY);
792799
final Nodes.Node executeOperator = callNode(node, read, node.operator, node.value);
793800
final Nodes.Node write = callNode(node, writeFlags, readReceiver, node.write_name,
794801
executeOperator);
@@ -821,8 +828,12 @@ public RubyNode visitCallOrWriteNode(Nodes.CallOrWriteNode node) {
821828
final var readReceiver = receiverExpression.getReadYARPNode();
822829

823830
// Use Prism nodes and rely on CallNode translation to automatically set CallNode flags
831+
// Prism doesn't set ATTRIBUTE_WRITE flag, so we should add it manually
832+
// safe navigation flag is handled separately, so as optimisation remove it from the flags
824833
short writeFlags = (short) (node.flags | Nodes.CallNodeFlags.ATTRIBUTE_WRITE);
825-
final RubyNode readNode = callNode(node, node.flags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY)
834+
writeFlags = (short) (writeFlags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
835+
short readFlags = (short) (node.flags & ~Nodes.CallNodeFlags.SAFE_NAVIGATION);
836+
final RubyNode readNode = callNode(node, readFlags, readReceiver, node.read_name, Nodes.Node.EMPTY_ARRAY)
826837
.accept(this);
827838
final RubyNode writeNode = callNode(node, writeFlags, readReceiver, node.write_name,
828839
node.value).accept(this);

0 commit comments

Comments
 (0)