Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support raw identifiers #70

Closed
wants to merge 1 commit into from

Conversation

GrayJack
Copy link
Contributor

This PR should fix #68

This is yet a draft cause I have some questions:

What I already pushed make type_identifier and field_identifier to stop being final nodes to be

(type_identifier
    (identifier))

(type_identifier
    (raw_identifier))

That would cause a massive test breakage

To avoid this level of breakage, a alternative would be making those like this:

    _type_identifier: $ => choice(
      alias($.identifier, $.type_identifier),
      alias($.raw_identifier, $.type_identifier)
    ),
    _field_identifier: $ => choice(
      alias($.identifier, $.field_identifier),
      alias($.raw_identifier, $.field_identifier)
    ),

But that would create a conflict:

Unresolved conflict for symbol sequence:

  identifier  •  '::'  …

Possible interpretations:

  1:  (_identifier  identifier)  •  '::'  …
  2:  (generic_type_with_turbofish  identifier  •  '::'  type_arguments)

Possible resolutions:

  1:  Specify a higher precedence in `generic_type_with_turbofish` than in the other rules.
  2:  Specify a higher precedence in `_identifier` than in the other rules.
  3:  Specify a left or right associativity in `_identifier`
  4:  Add a conflict for these rules: `generic_type_with_turbofish`, `_identifier`

So I would like to talk on what would be the best course of action here before going forward to fix the breakage on tests plus adding some more testings for raw identifiers

@maxbrunsfeld
Copy link
Contributor

To avoid this level of breakage, a alternative would be making those like this

Yeah, I think that is a good solution. I don't yet understand why it creates a new conflict. I will try to have a look soon.

@maxbrunsfeld
Copy link
Contributor

Hey, sorry for the delay here. After looking at the conflict that this introduced, I decided it was better to parse raw identifiers as part of the normal identifier token, handling them entirely in the lexer, with no grammar changes.

This does permit things like raw loop labels, which are technically not allowed, but I think that's ok. In general, I prefer to err on the side of performance/simplicity rather than making sure to disallow things that the spec disallows.

Thanks for exploring this approach, it was helpful to understand how much complexity it would add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raw identifiers are not supported
2 participants