[ruby/prism] Split up CallNode in target position

In this commit we're splitting up the call nodes that were in target
positions (that is, for loop indices, rescue error captures, and
multi assign targets).

Previously, we would simply leave the call nodes in place. This had
the benefit of keeping the AST relatively simple, but had the
downside of not being very explicit. If a static analysis tool wanted
to only look at call nodes, it could easily be confused because the
method would have 1 fewer argument than it would actually be called
with.

This also brings some consistency to the AST. All of the nodes in
a target position are now *TargetNode nodes. These should all be
treated the same, and the call nodes can now be treated the same.

Finally, there is benefit to memory. Because being in a target
position ensures we don't have some fields, we can strip down the
number of fields on these nodes.

So this commit introduces two new nodes: CallTargetNode and
IndexTargetNode. For CallTargetNode we get to drop the opening_loc,
closing_loc, arguments, and block. Those can never be present. We
also get to mark their fields as non-null, so they will always be
seen as present.

The IndexTargetNode keeps around most of its fields but gets to
drop both the name (because it will always be []=) and the
message_loc (which was always super confusing because it included
the arguments by virtue of being inside the []).

Overall, this adds complexity to the AST at the expense of memory
savings and explicitness. I believe this tradeoff is worth it in
this case, especially because these are very much not common nodes
in the first place.

https://github.com/ruby/prism/commit/3ef71cdb45
This commit is contained in:
Kevin Newton 2023-12-08 09:57:31 -05:00 коммит произвёл git
Родитель c69d1367a7
Коммит b673b5b432
9 изменённых файлов: 156 добавлений и 107 удалений

Просмотреть файл

@ -790,6 +790,32 @@ nodes:
foo.bar ||= value
^^^^^^^^^^^^^^^^^
- name: CallTargetNode
fields:
- name: flags
type: flags
kind: CallNodeFlags
- name: receiver
type: node
- name: call_operator_loc
type: location
- name: name
type: constant
- name: message_loc
type: location
comment: |
Represents assigning to a method call.
foo.bar, = 1
^^^^^^^
begin
rescue => foo.bar
^^^^^^^
end
for foo.bar in baz do end
^^^^^^^
- name: CapturePatternNode
fields:
- name: value
@ -1596,6 +1622,35 @@ nodes:
foo.bar[baz] ||= value
^^^^^^^^^^^^^^^^^^^^^^
- name: IndexTargetNode
fields:
- name: flags
type: flags
kind: CallNodeFlags
- name: receiver
type: node
- name: opening_loc
type: location
- name: arguments
type: node?
kind: ArgumentsNode
- name: closing_loc
type: location
- name: block
type: node?
comment: |
Represents assigning to an index.
foo[bar], = 1
^^^^^^^^
begin
rescue => foo[bar]
^^^^^^^^
end
for foo[bar] in baz do end
^^^^^^^^
- name: InstanceVariableAndWriteNode
fields:
- name: name

Просмотреть файл

@ -2111,6 +2111,63 @@ pm_index_or_write_node_create(pm_parser_t *parser, pm_call_node_t *target, const
return node;
}
/**
* Allocate and initialize a new CallTargetNode node from an existing call
* node.
*/
static pm_call_target_node_t *
pm_call_target_node_create(pm_parser_t *parser, pm_call_node_t *target) {
pm_call_target_node_t *node = PM_ALLOC_NODE(parser, pm_call_target_node_t);
*node = (pm_call_target_node_t) {
{
.type = PM_CALL_TARGET_NODE,
.flags = target->base.flags,
.location = target->base.location
},
.receiver = target->receiver,
.call_operator_loc = target->call_operator_loc,
.name = target->name,
.message_loc = target->message_loc
};
// Here we're going to free the target, since it is no longer necessary.
// However, we don't want to call `pm_node_destroy` because we want to keep
// around all of its children since we just reused them.
free(target);
return node;
}
/**
* Allocate and initialize a new IndexTargetNode node from an existing call
* node.
*/
static pm_index_target_node_t *
pm_index_target_node_create(pm_parser_t *parser, pm_call_node_t *target) {
pm_index_target_node_t *node = PM_ALLOC_NODE(parser, pm_index_target_node_t);
*node = (pm_index_target_node_t) {
{
.type = PM_INDEX_TARGET_NODE,
.flags = target->base.flags,
.location = target->base.location
},
.receiver = target->receiver,
.opening_loc = target->opening_loc,
.arguments = target->arguments,
.closing_loc = target->closing_loc,
.block = target->block
};
// Here we're going to free the target, since it is no longer necessary.
// However, we don't want to call `pm_node_destroy` because we want to keep
// around all of its children since we just reused them.
free(target);
return node;
}
/**
* Allocate and initialize a new CapturePatternNode node.
*/
@ -10618,23 +10675,15 @@ parse_target(pm_parser_t *parser, pm_node_t *target) {
if (*call->message_loc.start == '_' || parser->encoding->alnum_char(call->message_loc.start, call->message_loc.end - call->message_loc.start)) {
parse_write_name(parser, &call->name);
return (pm_node_t *) call;
return (pm_node_t *) pm_call_target_node_create(parser, call);
}
}
// If there is no call operator and the message is "[]" then this is
// an aref expression, and we can transform it into an aset
// expression.
if (
(call->call_operator_loc.start == NULL) &&
(call->message_loc.start != NULL) &&
(call->message_loc.start[0] == '[') &&
(call->message_loc.end[-1] == ']') &&
(call->block == NULL)
) {
// Replace the name with "[]=".
call->name = pm_parser_constant_id_constant(parser, "[]=", 3);
return target;
if (pm_call_node_index_p(call)) {
return (pm_node_t *) pm_index_target_node_create(parser, call);
}
}
/* fallthrough */

Просмотреть файл

@ -188,6 +188,12 @@ module Prism
assert_location(CallOrWriteNode, "foo.foo ||= bar")
end
def test_CallTargetNode
assert_location(CallTargetNode, "foo.bar, = baz", 0...7) do |node|
node.lefts.first
end
end
def test_CapturePatternNode
assert_location(CapturePatternNode, "case foo; in bar => baz; end", 13...23) do |node|
node.conditions.first.pattern
@ -462,6 +468,12 @@ module Prism
assert_location(IndexOrWriteNode, "foo[foo] ||= bar")
end
def test_IndexTargetNode
assert_location(IndexTargetNode, "foo[bar, &baz], = qux", 0...14) do |node|
node.lefts.first
end
end
def test_InstanceVariableAndWriteNode
assert_location(InstanceVariableAndWriteNode, "@foo &&= bar")
end

Просмотреть файл

@ -427,7 +427,7 @@
│ └── block: ∅
├── @ MultiWriteNode (location: (41,0)-(41,21))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (41,0)-(41,6))
│ │ ├── @ IndexTargetNode (location: (41,0)-(41,6))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ CallNode (location: (41,0)-(41,3))
@ -440,9 +440,6 @@
│ │ │ │ ├── arguments: ∅
│ │ │ │ ├── closing_loc: ∅
│ │ │ │ └── block: ∅
│ │ │ ├── call_operator_loc: ∅
│ │ │ ├── name: :[]=
│ │ │ ├── message_loc: (41,3)-(41,6) = "[0]"
│ │ │ ├── opening_loc: (41,3)-(41,4) = "["
│ │ │ ├── arguments:
│ │ │ │ @ ArgumentsNode (location: (41,4)-(41,5))
@ -452,7 +449,7 @@
│ │ │ │ └── flags: decimal
│ │ │ ├── closing_loc: (41,5)-(41,6) = "]"
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (41,8)-(41,14))
│ │ └── @ IndexTargetNode (location: (41,8)-(41,14))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ CallNode (location: (41,8)-(41,11))
@ -465,9 +462,6 @@
│ │ │ ├── arguments: ∅
│ │ │ ├── closing_loc: ∅
│ │ │ └── block: ∅
│ │ ├── call_operator_loc: ∅
│ │ ├── name: :[]=
│ │ ├── message_loc: (41,11)-(41,14) = "[0]"
│ │ ├── opening_loc: (41,11)-(41,12) = "["
│ │ ├── arguments:
│ │ │ @ ArgumentsNode (location: (41,12)-(41,13))
@ -2164,7 +2158,7 @@
│ │ │ ├── exceptions: (length: 0)
│ │ │ ├── operator_loc: (140,17)-(140,19) = "=>"
│ │ │ ├── reference:
│ │ │ │ @ CallNode (location: (140,20)-(140,24))
│ │ │ │ @ IndexTargetNode (location: (140,20)-(140,24))
│ │ │ │ ├── flags: ∅
│ │ │ │ ├── receiver:
│ │ │ │ │ @ CallNode (location: (140,20)-(140,21))
@ -2177,9 +2171,6 @@
│ │ │ │ │ ├── arguments: ∅
│ │ │ │ │ ├── closing_loc: ∅
│ │ │ │ │ └── block: ∅
│ │ │ │ ├── call_operator_loc: ∅
│ │ │ │ ├── name: :[]=
│ │ │ │ ├── message_loc: (140,21)-(140,24) = "[*]"
│ │ │ │ ├── opening_loc: (140,21)-(140,22) = "["
│ │ │ │ ├── arguments:
│ │ │ │ │ @ ArgumentsNode (location: (140,22)-(140,23))
@ -2230,7 +2221,7 @@
│ │ ├── exceptions: (length: 0)
│ │ ├── operator_loc: (142,17)-(142,19) = "=>"
│ │ ├── reference:
│ │ │ @ CallNode (location: (142,20)-(142,27))
│ │ │ @ IndexTargetNode (location: (142,20)-(142,27))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ CallNode (location: (142,20)-(142,21))
@ -2243,9 +2234,6 @@
│ │ │ │ ├── arguments: ∅
│ │ │ │ ├── closing_loc: ∅
│ │ │ │ └── block: ∅
│ │ │ ├── call_operator_loc: ∅
│ │ │ ├── name: :[]=
│ │ │ ├── message_loc: (142,21)-(142,27) = "[1, *]"
│ │ │ ├── opening_loc: (142,21)-(142,22) = "["
│ │ │ ├── arguments:
│ │ │ │ @ ArgumentsNode (location: (142,22)-(142,26))

Просмотреть файл

@ -500,7 +500,7 @@
│ └── block: ∅
├── @ MultiWriteNode (location: (41,0)-(41,23))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (41,0)-(41,7))
│ │ ├── @ CallTargetNode (location: (41,0)-(41,7))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ CallNode (location: (41,0)-(41,3))
@ -515,12 +515,8 @@
│ │ │ │ └── block: ∅
│ │ │ ├── call_operator_loc: (41,3)-(41,4) = "."
│ │ │ ├── name: :foo=
│ │ │ ├── message_loc: (41,4)-(41,7) = "foo"
│ │ │ ├── opening_loc: ∅
│ │ │ ├── arguments: ∅
│ │ │ ├── closing_loc: ∅
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (41,9)-(41,16))
│ │ │ └── message_loc: (41,4)-(41,7) = "foo"
│ │ └── @ CallTargetNode (location: (41,9)-(41,16))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ CallNode (location: (41,9)-(41,12))
@ -535,11 +531,7 @@
│ │ │ └── block: ∅
│ │ ├── call_operator_loc: (41,12)-(41,13) = "."
│ │ ├── name: :bar=
│ │ ├── message_loc: (41,13)-(41,16) = "bar"
│ │ ├── opening_loc: ∅
│ │ ├── arguments: ∅
│ │ ├── closing_loc: ∅
│ │ └── block: ∅
│ │ └── message_loc: (41,13)-(41,16) = "bar"
│ ├── rest: ∅
│ ├── rights: (length: 0)
│ ├── lparen_loc: ∅

Просмотреть файл

@ -8,7 +8,7 @@
│ ├── @ LocalVariableTargetNode (location: (1,0)-(1,1))
│ │ ├── name: :a
│ │ └── depth: 0
│ └── @ CallNode (location: (1,3)-(1,7))
│ └── @ CallTargetNode (location: (1,3)-(1,7))
│ ├── flags: ∅
│ ├── receiver:
│ │ @ CallNode (location: (1,3)-(1,4))
@ -23,11 +23,7 @@
│ │ └── block: ∅
│ ├── call_operator_loc: (1,4)-(1,6) = "::"
│ ├── name: :c=
│ ├── message_loc: (1,6)-(1,7) = "c"
│ ├── opening_loc: ∅
│ ├── arguments: ∅
│ ├── closing_loc: ∅
│ └── block: ∅
│ └── message_loc: (1,6)-(1,7) = "c"
├── rest: ∅
├── rights: (length: 0)
├── lparen_loc: ∅

Просмотреть файл

@ -8,7 +8,7 @@
│ ├── @ LocalVariableTargetNode (location: (1,0)-(1,1))
│ │ ├── name: :a
│ │ └── depth: 0
│ └── @ CallNode (location: (1,3)-(1,6))
│ └── @ CallTargetNode (location: (1,3)-(1,6))
│ ├── flags: ∅
│ ├── receiver:
│ │ @ CallNode (location: (1,3)-(1,4))
@ -23,11 +23,7 @@
│ │ └── block: ∅
│ ├── call_operator_loc: (1,4)-(1,5) = "."
│ ├── name: :C=
│ ├── message_loc: (1,5)-(1,6) = "C"
│ ├── opening_loc: ∅
│ ├── arguments: ∅
│ ├── closing_loc: ∅
│ └── block: ∅
│ └── message_loc: (1,5)-(1,6) = "C"
├── rest: ∅
├── rights: (length: 0)
├── lparen_loc: ∅

Просмотреть файл

@ -283,7 +283,7 @@
│ └── depth: 0
├── @ MultiWriteNode (location: (14,0)-(14,23))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (14,1)-(14,6))
│ │ ├── @ CallTargetNode (location: (14,1)-(14,6))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ LocalVariableReadNode (location: (14,1)-(14,2))
@ -291,12 +291,8 @@
│ │ │ │ └── depth: 0
│ │ │ ├── call_operator_loc: (14,2)-(14,3) = "."
│ │ │ ├── name: :foo=
│ │ │ ├── message_loc: (14,3)-(14,6) = "foo"
│ │ │ ├── opening_loc: ∅
│ │ │ ├── arguments: ∅
│ │ │ ├── closing_loc: ∅
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (14,8)-(14,13))
│ │ │ └── message_loc: (14,3)-(14,6) = "foo"
│ │ └── @ CallTargetNode (location: (14,8)-(14,13))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ LocalVariableReadNode (location: (14,8)-(14,9))
@ -304,11 +300,7 @@
│ │ │ └── depth: 0
│ │ ├── call_operator_loc: (14,9)-(14,10) = "."
│ │ ├── name: :bar=
│ │ ├── message_loc: (14,10)-(14,13) = "bar"
│ │ ├── opening_loc: ∅
│ │ ├── arguments: ∅
│ │ ├── closing_loc: ∅
│ │ └── block: ∅
│ │ └── message_loc: (14,10)-(14,13) = "bar"
│ ├── rest: ∅
│ ├── rights: (length: 0)
│ ├── lparen_loc: (14,0)-(14,1) = "("
@ -326,15 +318,12 @@
│ └── closing_loc: (14,22)-(14,23) = "]"
├── @ MultiWriteNode (location: (15,0)-(15,24))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (15,1)-(15,8))
│ │ ├── @ IndexTargetNode (location: (15,1)-(15,8))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ LocalVariableReadNode (location: (15,1)-(15,2))
│ │ │ │ ├── name: :a
│ │ │ │ └── depth: 0
│ │ │ ├── call_operator_loc: ∅
│ │ │ ├── name: :[]=
│ │ │ ├── message_loc: (15,2)-(15,8) = "[*foo]"
│ │ │ ├── opening_loc: (15,2)-(15,3) = "["
│ │ │ ├── arguments:
│ │ │ │ @ ArgumentsNode (location: (15,3)-(15,7))
@ -348,15 +337,12 @@
│ │ │ │ └── depth: 0
│ │ │ ├── closing_loc: (15,7)-(15,8) = "]"
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (15,10)-(15,14))
│ │ └── @ IndexTargetNode (location: (15,10)-(15,14))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ LocalVariableReadNode (location: (15,10)-(15,11))
│ │ │ ├── name: :a
│ │ │ └── depth: 0
│ │ ├── call_operator_loc: ∅
│ │ ├── name: :[]=
│ │ ├── message_loc: (15,11)-(15,14) = "[1]"
│ │ ├── opening_loc: (15,11)-(15,12) = "["
│ │ ├── arguments:
│ │ │ @ ArgumentsNode (location: (15,12)-(15,13))
@ -383,15 +369,12 @@
│ └── closing_loc: (15,23)-(15,24) = "]"
├── @ MultiWriteNode (location: (16,0)-(16,21))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (16,1)-(16,5))
│ │ ├── @ IndexTargetNode (location: (16,1)-(16,5))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ LocalVariableReadNode (location: (16,1)-(16,2))
│ │ │ │ ├── name: :a
│ │ │ │ └── depth: 0
│ │ │ ├── call_operator_loc: ∅
│ │ │ ├── name: :[]=
│ │ │ ├── message_loc: (16,2)-(16,5) = "[0]"
│ │ │ ├── opening_loc: (16,2)-(16,3) = "["
│ │ │ ├── arguments:
│ │ │ │ @ ArgumentsNode (location: (16,3)-(16,4))
@ -401,15 +384,12 @@
│ │ │ │ └── flags: decimal
│ │ │ ├── closing_loc: (16,4)-(16,5) = "]"
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (16,7)-(16,11))
│ │ └── @ IndexTargetNode (location: (16,7)-(16,11))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ LocalVariableReadNode (location: (16,7)-(16,8))
│ │ │ ├── name: :a
│ │ │ └── depth: 0
│ │ ├── call_operator_loc: ∅
│ │ ├── name: :[]=
│ │ ├── message_loc: (16,8)-(16,11) = "[1]"
│ │ ├── opening_loc: (16,8)-(16,9) = "["
│ │ ├── arguments:
│ │ │ @ ArgumentsNode (location: (16,9)-(16,10))
@ -440,7 +420,7 @@
│ │ @ SplatNode (location: (17,1)-(17,7))
│ │ ├── operator_loc: (17,1)-(17,2) = "*"
│ │ └── expression:
│ │ @ CallNode (location: (17,2)-(17,7))
│ │ @ CallTargetNode (location: (17,2)-(17,7))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ LocalVariableReadNode (location: (17,2)-(17,3))
@ -448,11 +428,7 @@
│ │ │ └── depth: 0
│ │ ├── call_operator_loc: (17,3)-(17,4) = "."
│ │ ├── name: :foo=
│ │ ├── message_loc: (17,4)-(17,7) = "foo"
│ │ ├── opening_loc: ∅
│ │ ├── arguments: ∅
│ │ ├── closing_loc: ∅
│ │ └── block: ∅
│ │ └── message_loc: (17,4)-(17,7) = "foo"
│ ├── rights: (length: 0)
│ ├── lparen_loc: (17,0)-(17,1) = "("
│ ├── rparen_loc: (17,7)-(17,8) = ")"

Просмотреть файл

@ -5,17 +5,13 @@
└── body: (length: 3)
├── @ MultiWriteNode (location: (1,0)-(1,17))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (1,0)-(1,6))
│ │ ├── @ CallTargetNode (location: (1,0)-(1,6))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ SelfNode (location: (1,0)-(1,4))
│ │ │ ├── call_operator_loc: (1,4)-(1,5) = "."
│ │ │ ├── name: :A=
│ │ │ ├── message_loc: (1,5)-(1,6) = "A"
│ │ │ ├── opening_loc: ∅
│ │ │ ├── arguments: ∅
│ │ │ ├── closing_loc: ∅
│ │ │ └── block: ∅
│ │ │ └── message_loc: (1,5)-(1,6) = "A"
│ │ └── @ LocalVariableTargetNode (location: (1,8)-(1,11))
│ │ ├── name: :foo
│ │ └── depth: 0
@ -30,24 +26,17 @@
│ └── depth: 0
├── @ MultiWriteNode (location: (3,0)-(3,24))
│ ├── lefts: (length: 2)
│ │ ├── @ CallNode (location: (3,0)-(3,6))
│ │ ├── @ CallTargetNode (location: (3,0)-(3,6))
│ │ │ ├── flags: ∅
│ │ │ ├── receiver:
│ │ │ │ @ SelfNode (location: (3,0)-(3,4))
│ │ │ ├── call_operator_loc: (3,4)-(3,5) = "."
│ │ │ ├── name: :a=
│ │ │ ├── message_loc: (3,5)-(3,6) = "a"
│ │ │ ├── opening_loc: ∅
│ │ │ ├── arguments: ∅
│ │ │ ├── closing_loc: ∅
│ │ │ └── block: ∅
│ │ └── @ CallNode (location: (3,8)-(3,18))
│ │ │ └── message_loc: (3,5)-(3,6) = "a"
│ │ └── @ IndexTargetNode (location: (3,8)-(3,18))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ SelfNode (location: (3,8)-(3,12))
│ │ ├── call_operator_loc: ∅
│ │ ├── name: :[]=
│ │ ├── message_loc: (3,12)-(3,18) = "[1, 2]"
│ │ ├── opening_loc: (3,12)-(3,13) = "["
│ │ ├── arguments:
│ │ │ @ ArgumentsNode (location: (3,13)-(3,17))
@ -70,17 +59,13 @@
│ └── depth: 0
└── @ MultiWriteNode (location: (5,0)-(5,18))
├── lefts: (length: 2)
│ ├── @ CallNode (location: (5,0)-(5,7))
│ ├── @ CallTargetNode (location: (5,0)-(5,7))
│ │ ├── flags: ∅
│ │ ├── receiver:
│ │ │ @ SelfNode (location: (5,0)-(5,4))
│ │ ├── call_operator_loc: (5,4)-(5,6) = "::"
│ │ ├── name: :a=
│ │ ├── message_loc: (5,6)-(5,7) = "a"
│ │ ├── opening_loc: ∅
│ │ ├── arguments: ∅
│ │ ├── closing_loc: ∅
│ │ └── block: ∅
│ │ └── message_loc: (5,6)-(5,7) = "a"
│ └── @ LocalVariableTargetNode (location: (5,9)-(5,12))
│ ├── name: :foo
│ └── depth: 0