openEHR / adl-antlr

Antrl4 grammars for ADL
Apache License 2.0
1 stars 4 forks source link

Sibling order parsing hard to do with grammar changes #25

Closed pieterbos closed 8 years ago

pieterbos commented 8 years ago

You changed:

c_objects: c_non_primitive_object_ordered+ | c_primitive_object ;
c_non_primitive_object_ordered: sibling_order? c_non_primitive_object ;

into

c_objects: ( sibling_order? c_non_primitive_object )+ | c_primitive_object ;

Which looks much better in the grammar. However, when parsing, it's a problem, because ANTLRgenerates the following methods in c_object

sibling_order() - returns a list of sibling orders
c_non_primitive_object() - returns a list of non primitive objets

Without any hint about which sibling order corresponds to which non primitive object. Which means you lose the ability to check which sibling order belongs to which c_non_primitive_object unless you perform some nasty tricks, which means this is now rather hard to use.

wolandscat commented 8 years ago

This seems to me like an error in the Antlr generator, since the following construction

( sibling_order? c_non_primitive_object )+

indicates that there is only zero or one sibling_order per c_non_primitive_object, due to the (). It seems to me the newer version is easier to read and has the same semantics, but I have no problem changing it back and commenting it - after all, the grammar isn't useful if it doesn't work. Maybe it's because no rule name is associated with the sibling_order / c_non_primitive_object pair. I think that must be it.

pieterbos commented 8 years ago

Thanks!

The problem is certainly not that the parser does not work - it does!

It's just that because the node is not named, walking the AST is harder. You can no longer just lookup the sibling orders and primitive objects, you need to iterator over the nodes in the tree and see in which order they appear, then handle that in code instead of the parser.