Fix 2016 round 2 #3841
Fix 2016 round 2 #3841
Conversation
parrt
added
target:java
target:python2
target:python3
target:cpp
target:go
target:javascript
target:swift
and removed
code-gen
labels
|
@lingyv-li there is a new test and it fails for the dart target could you take a look?? |
parrt
force-pushed
the
fix-2016-round-2
branch
from
7be13e4
to
3945988
Compare
|
Taking a look |
...resources/org/antlr/v4/test/runtime/descriptors/ParserExec/ListLabelsOnRuleRefStartOfAlt.txt
Show resolved
Hide resolved
Signed-off-by: Terence Parr <parrt@antlr.org>
…e causing the problem. Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
… AssertIsList so I'm dropping that test from the Go test suite. How did a comment to the C++ runnerFor my future reference as to how to build things from the command line. Signed-off-by: Terence Parr <parrt@antlr.org>
parrt
force-pushed
the
fix-2016-round-2
branch
from
c680c59
to
2bcfe15
Compare
…#3845 Signed-off-by: Terence Parr <parrt@antlr.org>
|
Go is all good on this now. PR is waiting. |
| grammar Test; | ||
|
|
||
| expression | ||
| @after { | ||
| <AssertIsList("$args")> | ||
| } | ||
| : op=NOT args+=expression | ||
| | args+=expression (op=AND args+=expression)+ | ||
| | args+=expression (op=OR args+=expression)+ | ||
| | IDENTIFIER | ||
| ; | ||
|
|
||
| AND : 'and' ; | ||
| OR : 'or' ; | ||
| NOT : 'not' ; | ||
| IDENTIFIER : [a-zA-Z_][a-zA-Z0-9_]* ; | ||
| WS : [ \t\r\n]+ -> skip ; | ||
|
|
There was a problem hiding this comment.
By the way, my sample from #3837 looks more succinctly:
grammar Test;
r
: args+=r (op=PLUS args+=r)+
| IDENTIFIER
;
PLUS : '+';
IDENTIFIER : [a-zA-Z_][a-zA-Z0-9_]*;There was a problem hiding this comment.
Ah. I didn't see that one there. I believe I cut and paste something from code you or someone else was proposing earlier to make that one. Are you suggesting we drop ListLabelsOnRuleRefStartOfAlt.txt because it is redundant? I think it is testing the same thing so maybe we should. Well I guess my checks to type and yours doesn't check the output :)
There was a problem hiding this comment.
Yes, it's the same, but in more short form.
There was a problem hiding this comment.
OK but maybe we should test the output on yours as I think you asked me to do on this test ha ha I had trouble with the output templates in so I elected to simply check weather the attribute was a list. Maybe create a PR that adds
@after {
<AssertIsList("$r")>
}
to yours and then deletes ListLabelsOnRuleRefStartOfAlt.txt?
There was a problem hiding this comment.
Ah. I didn't see that one there. I believe I cut and paste something from code you or someone else was proposing earlier to make that one. Are you suggesting we drop ListLabelsOnRuleRefStartOfAlt.txt because it is redundant? I think it is testing the same thing so maybe we should. Well I guess my checks to type and yours doesn't check the output :)
Yes - I think that this code was with the original issue, so I just made the <AssertIsList()> work. Either way is fine by me. Just tell me if you wish to lose the comments generated by - or you guys can just edit them out of PR #3848 and merge it. The other code in that PR is required for the test to work and fix the bug that was causing it to fail.
|
I didn’t write this test. Not sure if there was a reason for it being the
way it was. Does the same thing I guess :)
…On Wed, Aug 31, 2022 at 21:09 Ivan Kochurkin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
runtime-testsuite/resources/org/antlr/v4/test/runtime/descriptors/ParserExec/ListLabelsOnRuleRefStartOfAlt.txt
<#3841 (comment)>:
> +grammar Test;
+
+expression
***@***.*** {
+<AssertIsList("$args")>
+}
+ : op=NOT args+=expression
+ | args+=expression (op=AND args+=expression)+
+ | args+=expression (op=OR args+=expression)+
+ | IDENTIFIER
+ ;
+
+AND : 'and' ;
+OR : 'or' ;
+NOT : 'not' ;
+IDENTIFIER : [a-zA-Z_][a-zA-Z0-9_]* ;
+WS : [ \t\r\n]+ -> skip ;
+
By the way, my sample from #3837
<#3837> looks more succinctly:
grammar Test;
r
: args+=r (op=PLUS args+=r)+
| IDENTIFIER
;PLUS : '+';IDENTIFIER : [a-zA-Z_][a-zA-Z0-9_]*;
—
Reply to this email directly, view it on GitHub
<#3841 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7TMDA5GURRVWH5EAO63LV35KRBANCNFSM57Z4KL3A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Supercedes #2022 Fixes #2016
@jimidle Go target fails this new test: