search

KhronosGroup / NNEF-Tools

The NNEF Tools repository contains tools to generate and consume NNEF documents
https://www.khronos.org/nnef
222 stars 57 forks source link

tuple optional parentheses #17

Closed jnorwood closed 6 years ago

jnorwood commented 6 years ago

This is just a suggestion. The optional parenthesis in the tuple literal expression and tuple-lvalue-expr and tuple-rvalue-expr cause left recursion in the grammar

It is also inconsistent with the tuple-type-spec, where they are not optional.

I'd suggest making the parenthesis around the tuples non optional in all cases.

Parenthesis may be required for tuple rvalue-expr in any case, since these are used in the argument lists, which are also comma separated. I haven't tried that, but it appears to be a problem, just looking at it briefly

::= ["("] ("," )+ [")"] ::= | | | ::= ("," )* ::= | "="
jnorwood commented 6 years ago

wow, the attempted paste of the grammar from the spec apparently didn't like all the angle brackets.

gyenesvi commented 6 years ago

I'd say if this is the only rule that causes left-recursion then it is worth considering fixing it, but I don;t think that's the case.

I don't understand why it can cause an actual problem. An expression like

some_operation(param1 = a, b, param2 = c)

will be invalid (parse error), because expression b will be interpreted as the next parameter, but that's not a problem, it just forces the use of parentheses in that context, like

some_operation(param1 = (a, b), param2 = c)

However, in other contexts it can be valid, such as

a, b = 1, 2

True that it could be written as

(a,b) = (1,2)

which may be more readable.

Do you know a case when it actually causes some problem which prohibits writing something valid, or where a parser would accept something invalid?

gyenesvi commented 6 years ago

I close this issue here and continue discussing this in the NNEF-Docs repo:

https://github.com/KhronosGroup/NNEF-Docs/issues/3