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

feat(tablegen): add MLIR TableGen grammar #4107

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RIvance
Copy link

@RIvance RIvance commented May 24, 2024


// ArgValueList ::= PostionalArgValueList [","] NamedArgValueList
argValueList
: positionalArgValueList (',')? namedArgValueList
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could remove the extra parentheses here and in a few other places?

$ bash /c/Users/Kenne/Documents/GitHub/g4-scripts/find-useless-parentheses.sh
Using built-in parser.
CSharp 0 TableGen.g4 success 0.0477444
Computing useless parentheses in TableGen.g4
Using built-in parser.
CSharp 0 TableGen.g4 success 0.0477226
Found useless parentheses in grammars...
TableGen.g4:L100:     :   positionalArgValueList (',')? namedArgValueList
                                                 ^
TableGen.g4:L333:     :   '(' dagArg (dagArgList)? ')'
                                     ^
TableGen.g4:L460: DecimalInteger  :  ('+' | '-')? (UDigit)+;
                                                  ^
TableGen.g4:L464: TokIdentifier   :  (UDigit)* UAlpha (UAlpha | UDigit)*;
                                     ^
TableGen.g4:L455: TokString       : '"' (~('"') | Escape)* '"';
                                          ^
TableGen.g4:L457: TokCode         : '[{' (~(']') | ']' ~('}') | Escape)*? '}]';
                                           ^
TableGen.g4:L457: TokCode         : '[{' (~(']') | ']' ~('}') | Escape)*? '}]';
                                                        ^
05/25-06:29:01 ~/issues/g4-4107/tablegen/Generated-CSharp

Defset : 'defset';
Defvar : 'defvar';
Deftype : 'deftype';
Field : 'field';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field (and fragment EOL below) are unused. Is something missing?

$ bash /c/Users/Kenne/Documents/GitHub/g4-scripts/find-unused-lexer-symbols.sh
No arguments were provided.
Finding unused lexer symbols in grammars...
./desc.xml
1st pass: fold string literals in parser rules to make explicit lexer rule used.
2nd pass: find unreferenced lexer rule symbols.
Field
EOL
05/25-06:32:18 ~/issues/g4-4107/tablegen/Generated-CSharp

@@ -0,0 +1,454 @@
//===- ToyOps.td - Toy dialect operation definitions -------*- tablegen -*-===//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coverage for this one test is ~50%, but perhaps you could add additional tests to cover more of the grammar?

$ trcover.exe ../examples/ToyOps.td
Parsing grammar C:/msys64/home/Kenne/issues/g4-4107/tablegen/Generated-CSharp/TableGen.g4
CSharp 0 ../examples/ToyOps.td success 0.0336701
Analyzing...
Analysis 0 ../examples/ToyOps.td 0.0115427
05/25-06:39:13 ~/issues/g4-4107/tablegen/Generated-CSharp

Output: cover.html.txt

@teverett teverett added new-grammar New grammar issue or pull request example New example of file(s) parsed by grammar-generated parser tablegen labels May 25, 2024
@teverett
Copy link
Member

Looking at the build results, @kaby76 , is this an bug in the Antlr Python generator?

Traceback (most recent call last):
  File "/Users/runner/work/grammars-v4/grammars-v4/tablegen/Generated-Python3/Test.py", line 8, in <module>
    from TableGenParser import TableGenParser;
  File "/Users/runner/work/grammars-v4/grammars-v4/tablegen/Generated-Python3/TableGenParser.py", line 3[80](https://github.com/antlr/grammars-v4/actions/runs/9233773832/job/25415776942?pr=4107#step:24:81)6
    return self.getToken(TableGenParser.True, 0)
                                        ^^^^

@kaby76
Copy link
Contributor

kaby76 commented Jul 22, 2024

The problem is rule True. It's a symbol conflict. 'True' needs to be renamed with a trailing '_', or with some other naming scheme, e.g. 'KW_' Probably False has a problem as well.

@teverett
Copy link
Member

@kaby76 thanks! Hey @RIvance could you make the fix in your PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example New example of file(s) parsed by grammar-generated parser new-grammar New grammar issue or pull request tablegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants