-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Description
Hey Antlr/Golang people,
(@cyberfox, @pboyer, @parrt, @willfaught, @sharwell)
I'm posting this to propose and discussion a breaking change to Listener of the Go target.
It seems opportune, based on @cyberfox's work I have a PR on a visitor implementation.
Based on this I would like to propose changes to the Listener interfaces, using interfaces instead of pointer to structures.
eg
func (s *MyListener) EnterStart(ctx parser.IStartContext) {}
instead of
func (s *MyListener) EnterStart(ctx *parser.StartContext) {}
As noted on a number of occasions, the Antlr Go code is not idiomatic Go.
The major manifestations of this is large verses small interfaces, and trying to fit Java/C# "OO" into Go. To explain the "OO" comment, Go isn't objected-oriented in the same sense as Java; no implementation inheritance, only interface sub-typing, interface sub-typing is 'structural' vs Java's 'nominal'. Using interfaces and specifically small interfaces are used instead of "OO".
A specific place the current Go runtime is a problem is in re-using Listeners and Visitors. Imagine you have two grammars (A and B) that are very similar. B is a copy of A with one added and one modified rule. Currently it is not possible to implement a BListener that delegates to AListener (without making new A-based structures for every call). Using interfaces the BListener just needs to do a type assertion (cast) in order to delegate to AListener methods.
One of the issues swapping to interface is that the current generated parser does not create interfaces for Alt contexts, only for rule contexts. Implementing Alt context interfaces required changes to StructDecl.java & ListenerFile.java.
The changes proposed are in
https://github.com/wxio/antlr4/tree/visit-runtimeimport-tokenstream
#1841
An example using these can be seen at
https://github.com/wxio/antlr4-go-examples/tree/interface-discussion/scratch
note: the base listener and base visitor are now empty, only contains an example implementations in comments.
Currently the A - B use-case from above only works for named Alt, but it can be made to work for un-alt-named rules.
Cheers
Gary