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

Uncaught SyntaxError: Identifier '_alt' has already been declared #2980

Closed
yulingtianxia opened this issue Nov 29, 2020 · 19 comments
Closed

Comments

@yulingtianxia
Copy link

This issue happends on JS runtime target with antlr 4.9, for example:

antlr4 -Dlanguage=JavaScript Java9.g4

The generated file Java9Parser.js run with this error.

@ericvergnaud
Copy link
Contributor

Hi,
does not happen with all grammars.
Can you produce the smallest grammar that triggers the issue?

@yulingtianxia
Copy link
Author

Hi,
does not happen with all grammars.
Can you produce the smallest grammar that triggers the issue?

Hello, I can produce this issue by Java9.g4.

@ericvergnaud
Copy link
Contributor

That's not a small grammar.

@XStepien
Copy link

XStepien commented Dec 3, 2020

Hello, i can reproduce this issue with a "medium sized" grammar when trying to update from Antlr 4.8 to 4.9 (Javascript Runtime).

Also, it would be great if the documentation could give a concrete example of migration from 4.8 to 4.9 Javascript parser (visitor/listener).

@ericvergnaud
Copy link
Contributor

@XStepien re docs feel free to submit a PR
re the issue, I'm looking for a 10 rules grammar where the issue can be reproduced.

@ericvergnaud
Copy link
Contributor

could relate to a * and + in the same rule

@ericvergnaud
Copy link
Contributor

alternately please post here the generated code fragment where this happens and the corresponding grammar fragment

@XStepien
Copy link

XStepien commented Dec 3, 2020

I have this kind of rule:

expr : expr WS? Calc_ope WS? expr
    | Parenthesis WS? expr WS? ParenthesisEnd
    | operand
    ;

Where "Calc_ope" is :

Calc_ope: Mult | Div | Add | Sub;

Mult: '*';
Div: '/';
Add: '+';
Sub: '-';

Hope this help

@ericvergnaud
Copy link
Contributor

and that rule generates invalid javascript?

@XStepien
Copy link

XStepien commented Dec 3, 2020

I don't know if it's this rule, but it's related to your comment => "could relate to a * and + in the same rule"

I have to do some tests before being able to say which rule generate this issue ( Identifier '_alt' has already been declared )

@ericvergnaud
Copy link
Contributor

there is no * or + in your rule (I'm referring to ANTLR operators * and +, not characters '*' and '+')

@XStepien
Copy link

XStepien commented Dec 3, 2020

Ok, sorry for the misunderstanding,I will try to find more information soon

@dbemiller
Copy link

dbemiller commented Dec 4, 2020

I'm also seeing this. I can't post the whole grammar for IP reasons, but I do have a few tricky rules with + and * in them:

nestedPredicate :
    comparison
  | L_PAREN nestedPredicate (SPACE (OR_UPPER | OR_LOWER) SPACE nestedPredicate)+ R_PAREN
  | L_PAREN nestedPredicate (SPACE (AND_UPPER | AND_LOWER) SPACE nestedPredicate)+ R_PAREN
  ;

nestedMathable :
    integer
  | decimal
  | column
  | L_PAREN nestedMathable SPACE DIVIDE SPACE nestedMathable R_PAREN
  | L_PAREN nestedMathable SPACE MINUS SPACE nestedMathable R_PAREN
  | L_PAREN nestedMathable SPACE PLUS SPACE nestedMathable (SPACE PLUS SPACE nestedMathable)* R_PAREN
  | L_PAREN nestedMathable SPACE TIMES SPACE nestedMathable (SPACE TIMES SPACE nestedMathable)* R_PAREN
  ;

Hope this helps. It seems plausible since the error is a re-declared variable, and these rules are also recursive.

If not I can start deleting/anonymizing names to try to get a minimum reproducible grammar that I can share without exposing proprietary stuff though.

@ericvergnaud
Copy link
Contributor

can you paste the generated fragment causing the trouble?

ericvergnaud added a commit that referenced this issue Dec 6, 2020
@dbemiller
Copy link

dbemiller commented Dec 7, 2020

ahh... yeah now that I'm looking at the generated code, it's much clearer. The nestedPredicate() and nestedMathable() functions do not have the error, so ignore my last comment.

My plain old mathable() rule does, though:

mathable() {
  ...
  try {
    ...
    case 6:
      ...
      let _alt = this._interp.adaptivePredict(this._input,19,this._ctx)
      ...
    case 7:
      ...
      let _alt = this._interp.adaptivePredict(this._input,20,this._ctx)
      ...
}

Issue being that let declares the same var twice within the same block scope. The rule in my g4 file looks like:

mathable :
    integer
  | decimal
  | column
  | nestedMathable SPACE DIVIDE SPACE nestedMathable
  | nestedMathable SPACE MINUS SPACE nestedMathable
  | nestedMathable SPACE PLUS SPACE nestedMathable (SPACE PLUS SPACE nestedMathable)*
  | nestedMathable SPACE TIMES SPACE nestedMathable (SPACE TIMES SPACE nestedMathable)*
  ;

So... it looks like the issue is with non-recursive * rules. Recursive ones are ok.

I was able to fix it by tweaking the generated code to wrap { } braces around the missing offending case blocks, like:

case 6: {
  ...
}
case 7: {
  ...
}

@ericvergnaud
Copy link
Contributor

In c87e55b I've switched let _alt... to var ...alt
It would help if you could try it out and tell if that fixes the issue

@dbemiller
Copy link

dbemiller commented Dec 8, 2020

That should also be fine. let is spec'd to throw a SyntaxError when you re-declare it (see the "Redeclarations" section).

var is not

Duplicate variable declarations using var will not trigger an error, even in strict mode, and the variable will not lose its value, unless another assignment is performed.

@dbemiller
Copy link

dbemiller commented Dec 8, 2020

followup: just tested by following the instructions here on your commit. It builds & runs fine again for me. Thanks for the quick response & fix!

Not sure what antlr's release schedule is... do you have any estimate on how long it'll be until a patch version gets released with the bug fix?

@parrt parrt added this to the 4.9 milestone Jan 3, 2021
@parrt parrt removed this from the 4.9 milestone Jan 3, 2021
@parrt parrt added this to the 4.9.1 milestone Jan 3, 2021
@XStepien
Copy link

Hello,

I just upgraded to version 4.9.1 and everything works fine now.

Thanks a lot!

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

No branches or pull requests

5 participants