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

fix: #2016 - Generate correct iGo code for lists in a grammar, such a… #3834

Merged
merged 1 commit into from Aug 23, 2022
Merged

fix: #2016 - Generate correct iGo code for lists in a grammar, such a… #3834

merged 1 commit into from Aug 23, 2022

Conversation

jimidle
Copy link
Collaborator

@jimidle jimidle commented Aug 23, 2022

fix: #2016 Generate the correct Go code for label+=arg+ in a grammar

Signed-off-by: Jim.Idle jimi@gatherstars.com

…s `label+=arg+`

Signed-off-by: Jim.Idle <jimi@gatherstars.com>
@parrt parrt added this to the 4.10.2 milestone Aug 23, 2022
@parrt parrt merged commit a9b301b into antlr:dev Aug 23, 2022
@jimidle jimidle deleted the feature/2016 branch August 23, 2022 06:15
@KvanTTT
Copy link
Member

KvanTTT commented Aug 23, 2022

A test is missing.

@jimidle
Copy link
Collaborator Author

jimidle commented Aug 23, 2022 via email

@KvanTTT
Copy link
Member

KvanTTT commented Aug 23, 2022

We use descriptor files for runtime tests, see antlr-project-testing.md. You can skip other runtimes using [skip] flag but in general it's not recommended.

I added ListLabelInsideClosure.txt, but it fails on the execution step, see https://github.com/antlr/antlr4/runs/7982817143?check_suite_focus=true#step:23:1987

Test: ListLabelInsideClosure; State: Execute; java.lang.InterruptedException: 
panic: interface conversion: antlr.PredictionContext is *antlr.EmptyPredictionContext, not *antlr.ArrayPredictionContext [recovered]
	panic: interface conversion: antlr.PredictionContext is *antlr.EmptyPredictionContext, not *antlr.ArrayPredictionContext

goroutine 1 [running]:
test/parser.(*TestParser).r.func2()
	/tmp/GoRunner-ForkJoinPool-1-worker-1-1661288428057/parser/test_parser.go:280 +0xfe
panic({0x4e0900, 0xc00008d200})
	/opt/hostedtoolcache/go/1.19.0/x64/src/runtime/panic.go:884 +0x212
github.com/antlr/antlr4/runtime/Go/antlr.merge({0x52f9f0?, 0xc0000a0940}, {0x52faa0?, 0xc000098038}, 0x0, 0x52fe68?)
	/home/runner/work/antlr4/antlr4/runtime/Go/antlr/prediction_context.go:407 +0x351
github.com/antlr/antlr4/runtime/Go/antlr.(*BaseATNConfigSet).Add(0xc00009e680, {0x52fe68?, 0xc0000ca230}, 0x0?)
	/home/runner/work/antlr4/antlr4/runtime/Go/antlr/atn_config_set.go:140 +0x21f
github.com/antlr/antlr4/runtime/Go/antlr.(*ParserATNSimulator).closureWork(0xc00009a1e0, {0x52fe68, 0xc0000ca230}, {0x530f38, 0xc00009e680}, 0xc0000c4b78?, 0x0, 0xbc?, 0xffffffffffffffff, 0x0)
...
	/home/runner/work/antlr4/antlr4/runtime/Go/antlr/parser_atn_simulator.go:145 +0xbef
test/parser.(*TestParser).r(0xc000098168, 0xc00009e180?)
	/tmp/GoRunner-ForkJoinPool-1-worker-1-1661288428057/parser/test_parser.go:296 +0x2dd
test/parser.(*TestParser).R(...)
	/tmp/GoRunner-ForkJoinPool-1-worker-1-1661288428057/parser/test_parser.go:254
main.main()
	/tmp/GoRunner-ForkJoinPool-1-worker-1-1661288428057/Test.go:37 +0x15c
exit status 2
Test directory: /tmp/GoRunner-ForkJoinPool-1-worker-1-1661288428057

In fact, the bug is not fixed.

@chenquan
Copy link
Contributor

I found this problem in github.com/antlr/antlr4/runtime/Go/antlr v1.4.10. grammar:https://github.com/antlr/grammars-v4/tree/master/sql/mysql/Positive-Technologies
image
out:

panic: interface conversion: antlr.PredictionContext is *antlr.EmptyPredictionContext, not *antlr.ArrayPredictionContext [recovered]
	panic: interface conversion: antlr.PredictionContext is *antlr.EmptyPredictionContext, not *antlr.ArrayPredictionContext [recovered]
	panic: interface conversion: antlr.PredictionContext is *antlr.EmptyPredictionContext, not *antlr.ArrayPredictionContext

@jimidle
Copy link
Collaborator Author

jimidle commented Oct 16, 2022 via email

@parrt parrt mentioned this pull request Oct 16, 2022
@jimidle
Copy link
Collaborator Author

jimidle commented Mar 15, 2023

@parrt @KvanTTT - I cannot reproduce this with the latest runtime code. I have some recollection of fixing this as part of some other code fix, but I cannot be sure. Code should refer to the PredictionContext interface rather than a specific instance.

But I believe that I fixed this as a byproduct of changing the code that callsmergeArrays, many months ago. It was due to the fact that the "inheritance" was being implemented in PredictionContext was FUBAR and now it is not.

I think that we can close this. I have verified that it all works at both codegen and runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants