openEHR / adl-antlr

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

assumed value tree-walking is annoying in the current code #8

Closed pieterbos closed 8 years ago

pieterbos commented 8 years ago

The current syntax for c_integer is:

c_integer: ( integer_value | integer_list_value | integer_interval_value | integer_interval_list_value ) ( ';' integer_value )? ;

This makes it rather annoying to parse in code, because you get just an array of integer values and you have to check the semicolon to see how to tree it.

it could be changed in:

c_integer: ( integer_value | integer_list_value | integer_interval_value | integer_interval_list_value ) ( ';' assumed_integer_value )? ;
assumed_integer_value: integer_value;

Which would make walking this tree much easier.

wolandscat commented 8 years ago

I think in that case it would be clearer to make the ';' integer_value part its own rule, i.e.

c_integer: ( integer_value | integer_list_value | integer_interval_value | integer_interval_list_value ) assumed_integer_value? ;
assumed_integer_value: ';' integer_value;

but I am not sure what this looks like in the output tree.

pieterbos commented 8 years ago

I agree that that is better. Both work fine in the output tree. You can in both cases just call:

c_integerContext.assumed_integer_value().integer_value();

and you get the assumed integer value. Same if you do it with a listener instead of walking the tree manually.

pieterbos commented 8 years ago

You fixed this - thanks :)