google / polymorphicDSL

Apache License 2.0
11 stars 7 forks source link

Automatically generate 'polymorphicDslAllRules' grammar from interpreter file #12

Closed incident-recipient closed 1 year ago

incident-recipient commented 3 years ago

The DefaultPolymorphicDslPhraseFilter (mostly) requires creating another grammar on top of the grammar we actually care about. It also has a verbose constructor.

If possible we could compile the grammars at run time and generate the "meta" parser to remove this overhead. This way we could just write visitors/listeners for the parser we care about at less cost.

Se currently have the beginning of this in the InterpreterBasedPhraseFilter, however we haven't solved the problem of how to get the new context objects to work with the listeners generated from the original grammar (they are incompatible)

This problem may be large enough that we may need to abandon the effort.

One potentials way to solve this would be by generating code that used some form of method forwarding to invoke the listeners properly. This would require the ability to convert the "meta" context to the original parser context

Another might be to generate code that just has a walker call the corresponding rule with the encountered parse tree.

In either case it seems will need to find a way to wrap the original parser in the meta-parser some way if it is ever going to work.

incident-recipient commented 1 year ago

Having developed several projects in cross team initiatives I'm not convinced this would be a good thing anymore.

We found ourselves making many subgrammars to follow the interface segregation principle. As a result we needed wrapper grammars for each team to define which parsers they actually cared to use anyway.

There were some other interesting patterns we discovered. We would have each grammar define all of it's rules that we called AllRules. We could then easily pull in all the rules we wanted into some parent grammar and assign them to polymorphicDslAllRules.

I think there are advantages we might actually lose if we implemented this feature, so I'm closing it for now.