joelspadin / tree-sitter-devicetree

Tree-sitter grammar for Devicetree files
MIT License
28 stars 5 forks source link

Can't parse a binary expression as an integer cell item #6

Closed nickcoutsos closed 1 year ago

nickcoutsos commented 1 year ago

I'm looking into handling ZMK mods, noticing that in some places the documentation uses both <MOD_LSFT | MOD_LALT> and <(MOD_LSFT | MOD_LALT)>, though only the latter parses correctly with the current grammar.

It appears to result in a binary_expression but looking at _integer_cell_items:

        _integer_cell_items: ($) =>
            choice(
                $.integer_literal,
                $.reference,
                $.parenthesized_expression,
                $.identifier,
                $.call_expression
            ),

it seems like it's matching $.parenthesized_expression specifically. If I try adding $.binary_expression to that list it results in a conflict

Unresolved conflict for symbol sequence:

  '_label_name'  '{'  '_label_name'  '='  '<'  integer_literal  •  '&'  …

Possible interpretations:

  1:  '_label_name'  '{'  '_label_name'  '='  '<'  (_expression  integer_literal)  •  '&'  …
  2:  '_label_name'  '{'  '_label_name'  '='  '<'  (_integer_cell_items  integer_literal)  •  '&'  …

Possible resolutions:

  1:  Specify a higher precedence in `_integer_cell_items` than in the other rules.
  2:  Specify a higher precedence in `_expression` than in the other rules.
  3:  Add a conflict for these rules: `_integer_cell_items`, `_expression`

Would using $._expression in place of most of the choices be appropriate? It seems that once the choices are pared down to:

        _integer_cell_items: ($) =>
            choice(
                $._expression,
                $.reference
            ),

the build succeeds, but I don't know what other consequences this may have.

joelspadin commented 1 year ago

My understanding of devicetree syntax is that parenthesis are required for integer cells, because spaces are used as the item delimiter. Is it valid to leave out the parenthesis for a field that takes a single integer and not integer cells? If so, then changing the definition of _integer_cell_items would not be the correct place to add that.

nickcoutsos commented 1 year ago

Ok, so I guess the difference here would be <FOO | BAR> being an integer whereas <FOO | BAR BAZ> would be integer-cells?

joelspadin commented 1 year ago

Yeah. And I am not sure about whether the DT language allows the former, but the latter would need to be <(FOO | BAR) BAZ> to be valid.

nickcoutsos commented 1 year ago

I'm looking at the source for the devicetree spec and I'm thinking that maybe the parentheses ought to be mandatory after all:

https://github.com/devicetree-org/devicetree-specification/blob/639f3e9a97040b6a60ee8716748b558bf883aae1/source/chapter6-source-language.rst#node-and-property-definitions (hard to link to the exact line with GitHub rendering RST files)

values may be represented as arithmetic, bitwise, or logical expressions within parenthesis.

I'll bring it up in the ZMK discord server but I feel like, correct or not, people are accepting <FOO | BAR> and I might need to be add my own special handling of that specific property value.

nickcoutsos commented 1 year ago

Ok, apparently the other syntax actually doesn't compile so this was all moot. Sorry to take up your time!