Should we have target nodes?

[kddnewton]

Right now we have *ReadNode nodes and *WriteNode nodes, where * is one of:

• InstanceVariable
• ClassVariable
• GlobalVariable
• LocalVariable
• Constant

When we encounter an = after one of these has been parsed, we translate them into *WriteNode nodes, which all have a target, an operator location (the =), and a value.

However, there are a lot of places in the AST where we use these *WriteNode nodes to represent when a value will be written to the target, but there is no explicit value for them. This includes multi write, for loops, pattern matching, and rescue clauses. For example:

@foo, @bar = baz

In the above example it will create a multi write node that encapsulates two InstanceVariableWriteNode nodes that have a not provided token for the = and a NULL value.

begin
rescue => @foo
end

This will also create an InstanceVariableWriteNode with those values being NULL.

case foo
in @bar
end

This also creates an InstanceVariableWriteNode with those values being NULL.

Because we have 4 places in the AST that require this functionality, I'm wondering if we should introduce one more variant of these kinds of nodes that is a *TargetNode. The target nodes would only contain the names and not the operators or values. This would save a fair amount of space in multiple assignment, and a regular amount of space in other places. It complicates the AST a bit, but not too much.

I would love your thoughts @enebo, @eregon, @jemmaissroff.

[enebo]

@kddnewton I think the more specialized nodes the better until you get past 256 total nodes (1 byte type indicator), It will save space and on the processing side it will also lead to more small methods with less conditionals. At a small tradeoff of having to understand more node types (although the comment: field in config.yml is great at mitigating that).

Historically, we have all used null values (or in some cases an implicit non-syntactical nil marker) and most crashes in parsing have been from unexpected null values or poor array access logic (this being unimportant to this issue). The less nulls the better. They also muddle meaning. Is it semantic or just a mistake during parsing?

Tangential note: Call with null receiver is an extra field vs breaking it into CallNode and FunctionCallNode. No receiver and no args is a VCallNode (Variable? not sure on what V is meant in Ruby lore). Same discussion different node(s).

[eregon]

VCallNode is IMO a good example of a counter productive node type split. All usages need to handle call nodes similarly and share most of the logic. That should just be a boolean I think (or through optional receiver like currently, even better). vcall is not even relevant for execution, it's the same as fcall, i.e. all cryptic names to mean "implicit receiver"

[eregon]

I think it's simpler to keep those as WriteNode with optional value. It's only 2 bytes overhead for optional token and value, no?

For most of these I would actually want to give them a proper value node or process them and use "no value node" as a way to know I should pass the live value from somewhere else. On TruffleRuby's case both of these cases do translate to the same Truffle AST Node, just one uses execute() and the other assign(value)

[eregon]

BTW regarding avoiding overhead for tokens in serialized size, we could split token fields into syntactic tokens and semantics tokens, and e.g. have a way to only serialize semantics tokens. Or maybe there are so few semantics tokens that data could be encoded as boolean or flags or some field (e.g. lonely call operator could be a boolean on CallNode).

Another way could be to not serialize token offsets in the "semantics mode" since that's likely never needed for semantics/execution purposes.