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

fix: fix mismatched parenthesis when && is used #3274

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

slackner
Copy link
Contributor

@amaanq This fixes an issue introduced by the recent changes in #3070

Tree-sitter 0.22.2 generates the following incorrect code:

      if (lookahead == '-' ||
          ('A' <= lookahead && lookahead <= 'Z') ||
          lookahead == '_' ||
          ('a' <= lookahead && lookahead <= 'z')) ADVANCE(244);
      if (!eof && (0 <= lookahead && lookahead <= 8) ||
          (14 <= lookahead && lookahead <= 31) ||
          (lookahead == '"' ||
          lookahead == '\'' ||
          lookahead == '\\' ||
          (127 <= lookahead && lookahead <= 159)) ADVANCE(381);
      if (lookahead != 0 &&
          (lookahead < '\t' || ' ' < lookahead) &&
          lookahead != '(' &&
          lookahead != ')') ADVANCE(245);

Note that for the middle if, the parenthesis are not balanced and compilation fails.
With this change, it looks good again:

      if (lookahead == '-' ||
          ('A' <= lookahead && lookahead <= 'Z') ||
          lookahead == '_' ||
          ('a' <= lookahead && lookahead <= 'z')) ADVANCE(244);
      if ((!eof && (0 <= lookahead && lookahead <= 8)) ||
          (14 <= lookahead && lookahead <= 31) ||
          lookahead == '"' ||
          lookahead == '\'' ||
          lookahead == '\\' ||
          (127 <= lookahead && lookahead <= 159)) ADVANCE(381);
      if (lookahead != 0 &&
          (lookahead < '\t' || ' ' < lookahead) &&
          lookahead != '(' &&
          lookahead != ')') ADVANCE(245);

Feel free to adjust the PR / create your own one, if a different approach is preferred.

@slackner
Copy link
Contributor Author

Note that it would probably also be valid to generate code like this, if preferred: !eof && (... all conditions ...). !eof is only relevant for the check lookahead == 0 if I understand the code correctly, but it also doesn't hurt for the other range checks.

@amaanq
Copy link
Member

amaanq commented Apr 11, 2024

yeah my initial intention was to wrap every condition, but you're right that only the condition that's checking if the lookahead is 0 is needed anyways, thanks

@amaanq amaanq merged commit 5dc62cc into tree-sitter:master Apr 11, 2024
12 checks passed
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.

None yet

2 participants