congo-cc / congo-parser-generator

The CongoCC Parser Generator, the Next Generation of JavaCC 21, which in turn was the next generation of JavaCC
https://discuss.congocc.org/
Other
36 stars 11 forks source link

Consider removing support for the postfix form of named child/children #72

Closed adMartem closed 10 months ago

adMartem commented 10 months ago

In the addition of universal LHS assignment, and the inclusion of LHS assignment of named children (e.g., /myNamedNode/ = ... and /myNamedNodes/ += ...) I've left the support for the postfix form (e.g., ... /myNamedNode/and ... /[myNamedNodes]/) intact. It might be that we could remove the postfix form as, I think, it is now redundant (almost). The "almost" qualifier is because the postfix form uses the CURRENT_NODE that is at the top of the stack (being built) as the implementation of the setNamedChild and addToNamedChildList methods it uses. This is often the production node that is being built (i.e., thisProduction), but not always. If the postfix form is used with a node created by a tree node annotation (e.g., #myNode) or when the currently experimental option X_SYNTHETIC_NODES_ENABLED is used, it is possible for the CURRENT_NODE to be a different indefinite node, not the production node, in which case the named child/children will be added to that node, not the one being built for the production. For example, the expansion /aComma/ = <COMMA> /anotherComma/ #ACommaNode would set a named child aComma of the production node in which this occurred to the <COMMA> Node.Terminal and it would set a named child anotherComma of the ACommaNode to the same <COMMA> token. I'm not sure if this is an intended result, but if it is then perhaps it is a capability worth keeping. If not, then maybe it would be best to remove the postfix form and state that the named child or child list always refers to the ones in the current production node and is specified using the LHS assignment syntax.

adMartem commented 10 months ago

I would be happy to do the actual work if this is the consensus.

vsajip commented 10 months ago

I'd be fine for you to remove the postfix form, but note that named child nodes are used (IIRC) here and there in the transpiler, and the proposed changes would need corresponding changes there. The same applies to #73; ideally, they would be in their own PR and pass all tests there, and be squashed and merged into main as one commit when ready.

adMartem commented 10 months ago

Yes, I've run across most (maybe all) of them and while I've left the postfix in place, I changed a couple of them to use the assignment infix form as a test. If we decide to remove the postfix form I would plan to squash it all into a single-purpose commit as you suggest.