Exclusion of location fields in serialize for execution

[enebo]

Execution does not need all info contained in Nodes defined in config.yml. In particular, they do not need any Tokens or Location fields. We should allow templates for serialization and loading to not include those in the blob or in the generated AST.

Towards this goal issues will get opened we encounter places where we DO currently need tokens (e.g. CallNode.call_operator (. vs &.) or RegularExpressionNode.closing (/ix).

[eregon]

Towards this goal issues will get opened we encounter places where we DO currently need tokens

For those I suggest we annotate them with serialize: true or keep_in_serialization: true or so in config.yml, until that info is available through other fields.

[enebo]

Here is an interesting case where I am not sure how this should be structured if you remove the token:

  - name: BlockParameterNode
    child_nodes:
      - name: name
        type: token?
      - name: operator_loc
        type: location
        java_exclude: true
    comment: |
      Represents a block parameter to a method, block, or lambda definition.

          def a(&b)
                ^^
          end

If the token is removed then it will be pointing at '&b' and not 'b'. I think the solution would be to assume first char is '&'? Today I use the name token.

[enebo]

@jemmaissroff Is this layout/strategy ok? I open up individual issues where excluding the token would lead to loss of information and mention this issue in the description? I am willing to audit all in config.yml but don't want know if this is how it should be done.

[eregon]

For BlockParameterNode it seems best to keep the token for serialization for now (to get the identifier name).
Notably there can be spacing around there and it's still valid Ruby: ruby -e 'def m( & a ); a; end; p m {}'.
But we wouldn't want a space in the variable name (would break for references to a in the method).

Such identifiers should most likely use the constant pool long term, and then they're pretty short in serialized form + avoids rereading the same identifier multiple times (and to convert from byte[] to java.lang.String or so).

So I'd say use serialize: true for that token until we have another way.

[enebo]

@eregon I am going with the assumption the goal is to replace anything which is token and is needed to what we want to read. You are correct we can add a temporary serialize: true but I am asking how should we do it. If you were to read this node from TR how would you want to see this node?

[eregon]

Yeah, agreed. I think an interned identifier in the constant pool is the way to go here (but we don't have a constant pool yet in YARP).

[kddnewton]

I'm going to close this once #997 is merged. At that point all tokens are off the tree and only locations remain.

I think going forward we can operate as if there is a future where all location fields on nodes are removed from the serialization. In that case if there is anything that y'all need for compilation that isn't serviced by just the name of the node and its fields that aren't locations, we should add flags.

The only one I know of at the moment is the range node where we need to indicate exclusivity. Other than that I think we have all the information we need? We certainly will find out pretty soon once we integrate into CRuby and start our own compilation work.

[eregon]

Let me reopen this with the original intent, i.e. the possibility to have no token and no location fields in serialization, and also no location fields in Java nodes.