-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[go target] Build errors with PartiQL parser #4632
Comments
I am reworking the Go visitor code as there are some other issues. Though it generally works, there are some issues that need manual intervention, which is not ideal. It may take a week or so to get this ready for release. though. Looks like you are using the Listener here though, which does work, so I woudl need to see your code to work out what is wrong here. Please don't use images for text errors as they are hard to read. |
I have several comments on this grammar. There are several problems.
Correcting all these errors, we see the Go target work with this grammar. Input:
|
Thank for your reply! I remove the image from issue description and put text instead. |
Incorrect. The option works in a combined grammar (for lexer rules). See testCaseInsensitiveInCombinedGrammar test for example. Also, it could be actual in parser grammar if Token value comparators feature is implemented (not now). |
Sorry, what I meant is for this grammar. Even though PartiQL.g4 has a grammarType without "parser", it is essentially a parser grammar because Rather than be exposed to this rabbit hole, a parser grammar should be declared as |
First, I am curious about this: Second, I found a related issue mentioned symbol conflict. However, why don't ANTLR tools raise any warning or error in this case? BTW, I can get the correct go target after renamming |
To see what is happening, let's rename all but the first of the attribute "parent=" to "parent_=" (in parser rule
For attribute
For the renamed attribute
However, the Antlr Go Runtime also defines functions antlr4/runtime/Go/antlr/v4/parser_rule_context.go Lines 391 to 399 in 6197d6c
antlr4/runtime/Go/antlr/v4/parser_rule_context.go Lines 357 to 363 in 6197d6c
The Antlr Tool cannot apriori warn there is a conflict because it doesn't do a build. It only "knows" when the symbol is in a list of symbol names that are known to cause build problems. That table is here: antlr4/tool/src/org/antlr/v4/codegen/target/GoTarget.java Lines 23 to 54 in 6197d6c
That table does not contain "parent". It does contain "GetParent" however. The Antlr tool does not warn because it just internally renames symbol conflicts before code generation. Other targets may still work with the original attribute name. The requirement for this solution was to not break existing parsers that use Antlr. We could have implemented a different solution to the problem by adding an option to generate code with a unique string appended to symbol names for other targets, which would have not broken old, working code for the original target. But that was not done here. I have a script (https://github.com/kaby76/g4-scripts/blob/e5ca411ddbf8eb9fb97e986c99d124991e839899/find-known-conflicts.sh) that parses the Antlr4 grammar and checks all symbols against all known conflicts, but it's hard to keep it up to date. We can't predict when there is a conflict. |
I get it. If we name it |
The ANTLR4 version I used is 4.13.1.
Use the PartiQL grammar in PartiQL site, https://partiql.org/syntax/antlr.html. And generate go target with command
antlr -Dlanguage=Go -package parser -long-messages -listener *.g4
, the target has build errors, for examples:The text was updated successfully, but these errors were encountered: