Skip to content

Fix 2016 round 2 #3841

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Aug 29, 2022
Merged

Fix 2016 round 2 #3841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
23ae7df
add test for #2016 and fix java.
parrt Aug 27, 2022
50d0edc
ensure all targets have the appropriate argument list for the templat…
parrt Aug 27, 2022
ca0540b
Fix other targets
parrt Aug 27, 2022
ddbe172
fix format
parrt Aug 28, 2022
ff345a6
add check that $args is a list
parrt Aug 28, 2022
891b0f4
change made by @lingyv-li to fix bug in DART exposed by this test.
parrt Aug 28, 2022
2bcfe15
fix AssertIsList in multiple targets. Go doesn't pass test and has no…
parrt Aug 28, 2022
6a3b44b
C++ gets an exception with this test so I'm turning it off. See https…
parrt Aug 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...resources/org/antlr/v4/test/runtime/descriptors/ParserExec/ListLabelForClosureContext.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,5 @@ expression
[input]
a

[skip]
Go
34 changes: 34 additions & 0 deletions ...ources/org/antlr/v4/test/runtime/descriptors/ParserExec/ListLabelsOnRuleRefStartOfAlt.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
[notes]
Checks that this compiles; see https://github.com/antlr/antlr4/issues/2016

[type]
Parser

[grammar]
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 ;

Comment on lines +8 to +25
Copy link
Member

Choose a reason for hiding this comment

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

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_]*;

Copy link
Member Author

Choose a reason for hiding this comment

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

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 :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's the same, but in more short form.

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

[start]
expression

[input]
a and b

[skip]
Go
Cpp
2 changes: 1 addition & 1 deletion runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/CSharp.test.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AppendStr(a,b) ::= <%<Append(a,b)>%>

Concat(a,b) ::= "<a><b>"

AssertIsList(v) ::= "System.Collections.IList __ttt__ = <v>;" // just use static type system
AssertIsList(v) ::= "System.Collections.IList __ttt__ = (System.Collections.IList)<v>;" // just use static type system

AssignLocal(s,v) ::= "<s> = <v>;"

Expand Down
2 changes: 1 addition & 1 deletion runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/Go.test.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AppendStr(a,b) ::= "<a> + <b>"

Concat(a,b) ::= "<a><b>"

AssertIsList(v) ::= ""
AssertIsList(v) ::= "TODO!!"

AssignLocal(s, v) ::= "<s> = <v>;"

Expand Down
2 changes: 1 addition & 1 deletion runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/JavaScript.test.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AppendStr(a,b) ::= <%<Append(a,b)>%>

Concat(a,b) ::= "<a><b>"

AssertIsList(v) ::= <<if ( !(v instanceof Array) ) {throw "value is not an array";}>>
AssertIsList(v) ::= <<if ( !(<v> instanceof Array) ) {throw "value is not an array";}>>

AssignLocal(s,v) ::= "<s> = <v>;"

Expand Down
2 changes: 1 addition & 1 deletion runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/Python2.test.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AppendStr(a,b) ::= "<a> + <b>"

Concat(a,b) ::= "<a><b>"

AssertIsList(v) ::= "assert isinstance(v, (list, tuple))"
AssertIsList(v) ::= "assert isinstance(<v>, (list, tuple))"

AssignLocal(s,v) ::= "<s> = <v>"

Expand Down
2 changes: 1 addition & 1 deletion runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/Python3.test.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AppendStr(a,b) ::= "<a> + <b>"

Concat(a,b) ::= "<a><b>"

AssertIsList(v) ::= "assert isinstance(v, (list, tuple))"
AssertIsList(v) ::= "assert isinstance(<v>, (list, tuple))"

AssignLocal(s,v) ::= "<s> = <v>"

Expand Down
5 changes: 3 additions & 2 deletions runtime-testsuite/test/org/antlr/v4/test/runtime/Processor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ public class Processor {
public final Map<String, String> environmentVariables;
public final boolean throwOnNonZeroErrorCode;

public static ProcessorResult run(String[] arguments, String workingDirectory, Map<String, String> environmentVariables
) throws InterruptedException, IOException {
public static ProcessorResult run(String[] arguments, String workingDirectory, Map<String, String> environmentVariables)
throws InterruptedException, IOException
{
return new Processor(arguments, workingDirectory, environmentVariables, true).start();
}

Expand Down
19 changes: 19 additions & 0 deletions runtime-testsuite/test/org/antlr/v4/test/runtime/cpp/CppRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,25 @@
import static org.antlr.v4.test.runtime.RuntimeTestUtils.getOS;
import static org.antlr.v4.test.runtime.RuntimeTestUtils.isWindows;

/**
* For my own information on I'm recording what I needed to do to get a unit test to compile and run in C++ on the Mac.
* I got a segmentation violation and couldn't figure out how to get information about it, so I turned on debugging
* and then figured out lldb enough to create this issue: https://github.com/antlr/antlr4/issues/3845 on a bug.
*
* cd ~/antlr/code/antlr4/runtime/Cpp
* cmake . -D CMAKE_OSX_ARCHITECTURES="arm64; x86_64" -DCMAKE_BUILD_TYPE=Debug
* make -j 8
*
* In test dir with generated test code:
*
* clang++ -g -std=c++17 -I /Users/parrt/antlr/code/antlr4/runtime/Cpp/runtime/src -L. -lantlr4-runtime *.cpp
* ./a.out input
*
* $ lldb ./a.out input
* (lldb) run
* ... crash ...
* (lldb) thread backtrace
*/
public class CppRunner extends RuntimeRunner {
@Override
public String getLanguage() {
Expand Down
2 changes: 1 addition & 1 deletion runtime/Dart/lib/src/prediction_context.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ abstract class PredictionContext {
return a_;
}

mergedParents = List.generate(k, (n) => mergedParents[n]!);
mergedParents = List.generate(k, (n) => mergedParents[n]);
mergedReturnStates = List.generate(k, (n) => mergedReturnStates[n]);
}

Expand Down
10 changes: 8 additions & 2 deletions tool/resources/org/antlr/v4/tool/templates/codegen/Cpp/Cpp.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1104,9 +1104,15 @@ recRuleAltPredicate(ruleName,opPrec) ::= "precpred(_ctx, <opPrec>)"
recRuleSetReturnAction(src,name) ::= "recRuleSetReturnAction(src,name) $<name>=$<src>.<name>;"
recRuleSetStopToken() ::= "_ctx->stop = _input->LT(-1);"

recRuleAltStartAction(ruleName, ctxName, label) ::= <<
recRuleAltStartAction(ruleName, ctxName, label, isListLabel) ::= <<
_localctx = _tracker.createInstance\<<ctxName>Context>(parentContext, parentState);
<if (label)>_localctx-><label> = previousContext;<endif>
<if(label)>
<if(isListLabel)>
_localctx-><label>.push_back(previousContext);
<else>
_localctx-><label> = previousContext;
<endif>
<endif>
pushNewRecursionContext(_localctx, startState, Rule<ruleName; format = "cap">);
>>

Expand Down
1 change: 0 additions & 1 deletion tool/resources/org/antlr/v4/tool/templates/codegen/Java/Java.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,6 @@ _localctx.<label>.add(_prevctx);
_localctx.<label> = _prevctx;
<endif>
<endif>
<if(label)>_localctx.<label> = _prevctx;<endif>
pushNewRecursionContext(_localctx, _startState, RULE_<ruleName>);
>>

Expand Down
10 changes: 8 additions & 2 deletions tool/resources/org/antlr/v4/tool/templates/codegen/JavaScript/JavaScript.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,15 @@ recRuleAltPredicate(ruleName,opPrec) ::= "this.precpred(this._ctx, <opPrec>)"
recRuleSetReturnAction(src,name) ::= "$<name>=$<src>.<name>"
recRuleSetStopToken() ::= "this._ctx.stop = this._input.LT(-1);"

recRuleAltStartAction(ruleName, ctxName, label) ::= <<
recRuleAltStartAction(ruleName, ctxName, label, isListLabel) ::= <<
localctx = new <ctxName>Context(this, _parentctx, _parentState);
<if(label)>localctx.<label> = _prevctx;<endif>
<if(label)>
<if(isListLabel)>
localctx.<label>.push(_prevctx);
<else>
localctx.<label> = _prevctx;
<endif>
<endif>
this.pushNewRecursionContext(localctx, _startState, <parser.name>.RULE_<ruleName>);
>>

Expand Down
10 changes: 8 additions & 2 deletions tool/resources/org/antlr/v4/tool/templates/codegen/Python2/Python2.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,15 @@ recRuleAltPredicate(ruleName,opPrec) ::= "self.precpred(self._ctx, <opPrec>)"
recRuleSetReturnAction(src,name) ::= "$<name>=$<src>.<name>"
recRuleSetStopToken() ::= "self._ctx.stop = self._input.LT(-1)"

recRuleAltStartAction(ruleName, ctxName, label) ::= <<
recRuleAltStartAction(ruleName, ctxName, label, isListLabel) ::= <<
localctx = <parser.name>.<ctxName>Context(self, _parentctx, _parentState)
<if(label)>localctx.<label> = _prevctx<endif>
<if(label)>
<if(isListLabel)>
localctx.<label>.append(_prevctx)
<else>
localctx.<label> = _prevctx
<endif>
<endif>
self.pushNewRecursionContext(localctx, _startState, self.RULE_<ruleName>)
>>

Expand Down
10 changes: 8 additions & 2 deletions tool/resources/org/antlr/v4/tool/templates/codegen/Python3/Python3.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,9 +721,15 @@ recRuleAltPredicate(ruleName,opPrec) ::= "self.precpred(self._ctx, <opPrec>)"
recRuleSetReturnAction(src,name) ::= "$<name>=$<src>.<name>"
recRuleSetStopToken() ::= "self._ctx.stop = self._input.LT(-1)"

recRuleAltStartAction(ruleName, ctxName, label) ::= <<
recRuleAltStartAction(ruleName, ctxName, label, isListLabel) ::= <<
localctx = <parser.name>.<ctxName>Context(self, _parentctx, _parentState)
<if(label)>localctx.<label> = _prevctx<endif>
<if(label)>
<if(isListLabel)>
localctx.<label>.append(_prevctx)
<else>
localctx.<label> = _prevctx
<endif>
<endif>
self.pushNewRecursionContext(localctx, _startState, self.RULE_<ruleName>)
>>

Expand Down
1 change: 0 additions & 1 deletion tool/resources/org/antlr/v4/tool/templates/codegen/Swift/Swift.stg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,6 @@ _localctx.<label>.append(_prevctx)
_localctx.<label> = _prevctx
<endif>
<endif>
<if(label)>_localctx.<label> = _prevctx;<endif>
try pushNewRecursionContext(_localctx, _startState, <parser.name>.RULE_<ruleName>)
>>

Expand Down