antlr-kotlin icon indicating copy to clipboard operation
antlr-kotlin copied to clipboard

Wrong precedence in left-recursive rules

Open mstrohbach opened this issue 5 years ago • 10 comments

It seems that antlr-kotlin does not properly handle precedence in left-recursive rules. Consider for instance the expression rule in the MiniCalc example:

expression : left=expression operator=(DIVISION|ASTERISK) right=expression # binaryOperation
           | left=expression operator=(PLUS|MINUS) right=expression        # binaryOperation
           | value=expression AS targetType=type                           # typeConversion
           | LPAREN expression RPAREN                                      # parenExpression
           | ID                                                            # valueReference
           | MINUS expression                                              # minusExpression
           | STRING_OPEN (parts+=stringLiteralContent)* STRING_CLOSE       # stringLiteral
           | INTLIT                                                        # intLiteral
           | DECLIT                                                        # decimalLiteral ;

The expression b/3+2 is parsed as expected by the Intellij ANTLR v4 grammar plugin (v1.10): plugin-tree But antlr-kotlin groups 3+2 as a child to the / operator: antlr-kotlin-tree

I get the same behaviour in my own grammars.

I am using the antlr-kotlin plugin with the default settings antlrKotlinVersion = "0.0.4" and antlrVersion = "4.7.1"

mstrohbach avatar Aug 06 '19 13:08 mstrohbach

Hi Martin, thank you for reporting this.

I am not sure when I will be able to work on this but a PR would be appreciated at any time :)

This issue surprise me a bit because ANTLR Kotlin is basically a translation of the original ANTLR implementation written in Java, so it is very curious we introduced this bug while translating.

ftomassetti avatar Aug 12 '19 09:08 ftomassetti

Hi Federico,

Thanks for your reply. I looked into the issue and the problem seems to be that a method in Parser.kt has been commented out.

Regards

Martin

mstrohbach avatar Aug 12 '19 11:08 mstrohbach

Thank you Martin for your investigation!

ftomassetti avatar Aug 13 '19 10:08 ftomassetti

Has this ever been resolved? I seem to still run into the issue using 94f8764bcf

edwinRNDR avatar Mar 29 '20 17:03 edwinRNDR

I guess not. I tried the same grammar (something very similar to MiniCalc) with the Java based Antlr tooling and there the operator precedence works as expected.

edwinRNDR avatar Apr 04 '20 09:04 edwinRNDR

I had been similarly stuck at false-positive ambiguity detection failure that IDEA ANTLR v4 plugin never complains. I tried to fix it by actually uncommenting various code bits around Parser.kt, but it did not make it any better for me.

I leave my patch here so in case anyone finds it useful feel free to take it. https://gist.github.com/atsushieno/e56e451eb3fd42270ed7af5c21e55b86

atsushieno avatar Mar 15 '21 13:03 atsushieno

Also experiencing this issue. Antlr-kotlin parsing incorrectly, but getting the correct precedence using antlr4 generated parsers.

sergioazua-pc avatar Nov 11 '21 15:11 sergioazua-pc

I had been similarly stuck at false-positive ambiguity detection failure that IDEA ANTLR v4 plugin never complains. I tried to fix it by actually uncommenting various code bits around Parser.kt, but it did not make it any better for me.

I leave my patch here so in case anyone finds it useful feel free to take it. https://gist.github.com/atsushieno/e56e451eb3fd42270ed7af5c21e55b86

This patch seems to fix my precedence issues. Thank you!

sergioazua-pc avatar Nov 12 '21 16:11 sergioazua-pc

Any updates on this? ~I've tried the patch with no success.~

jay-karimi avatar Dec 13 '21 21:12 jay-karimi

Thank you @atsushieno for your patch! I've applied it to my repo: https://github.com/tianyu/antlr-kotlin And you can use the maven package here: https://jitpack.io/#tianyu/antlr-kotlin/3bb1661562 And there's a Pull Request here: https://github.com/Strumenta/antlr-kotlin/pull/75

tianyu avatar Mar 14 '22 14:03 tianyu

Solved in https://github.com/Strumenta/antlr-kotlin/pull/107, closing (thank you @lppedd for pointing it out)

ftomassetti avatar Jan 15 '24 14:01 ftomassetti