Is antlr4's golang visitor pattern in a usable state?

[ianamason]

I asked this question over on stack overflow, without much enlightenment. I was told to ask it
here.

Is antlr4's golang visitor pattern in a usable state?

I see no working examples, and I see a few pull requests that remain open. I would like to avoid going down the garden path.

#1807

#1843

There are also pull requests that make me think the golang target might be dead in the water.

#2152

The fact that the antlr4 documentation page for golang is over two years stale is also not
encouraging.

So all I am really asking is should I steer clear of golang, or is the documentation just somewhere that google doesn't see :-)

[nikunjy]

Looking to use antlr with golang. Can you address some of these @parrt ?

[parrt]

Sorry. I don't know Go.

[ianamason]

Might want to be honest then, and remove go from the supported language targets. At least warn the gullible user that they may be disappointed.

[parrt]

Are you claiming it doesn't support Go in any way?

[ianamason]

I am claiming that I could not get the --visitor pattern to work. Nor could I find any working examples
of other people getting it to work. What I did find were issues in this repository that seem to indicate that it never worked, and that people have tried and failed to fix it. I ended up having to fall back on the listener pattern for the task I had at hand, but given what I know now, would probably
have not chosen go for the project I am/was working on.

[parrt]

I see. ok, well, my apologies. There are lots of different language targets for this project and it's very hard to keep track of the current state of every piece of every API. It's also no longer my focus.

[ianamason]

Might be an idea then to keep the issue open, so that someone with time and inclination
may succeed where others have failed.

[ocurr]

@ianamason According to @pboyer 's comment on #1841 the Go visitor implementation is in a usable state; however, the user is currently required to put in more work then, for instance, the java implementation.

[ianamason]

@ocurr indeed I did see that issue thread, though may have incorrectly got the impression that the
matter was still unresolved. The certainly was no link to any working visitor example that I could see.
I may add a link to the issue in the original post so that others can benefit.

Is there a working example somewhere that people could follow?

[davesisson]

The trick is that you need to include all of the methods the visitor will use and not just the ones that you would be implementing. It's not incredibly convenient which is why there was a proposal at one point for redesigning the way visitors were defined.

…

On Mon, Apr 15, 2019 at 1:47 PM Ian A Mason ***@***.***> wrote: @ocurr <https://github.com/ocurr> indeed I did see that issue thread, though may have incorrectly got the impression that the matter was still unresolved. The certainly was no link to any working visitor example that I could see. I may add a link to the issue in the original post so that others can benefit. Is there a working example somewhere that people could follow? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2504 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALUC_ajCCWJK08wr_r120qKuseSMF0feks5vhOVEgaJpZM4bg9rx> .

[ianamason]

@davesisson, a simple working example would go a long way to "resolving" this issue. Having to flesh out all the methods still sounds a simpler task than trying to get the listener pattern to accomplish the same thing.

[monodop]

I would also like to see a working example from somewhere. I am getting hung up on some typing issues that prevent my project from even compiling, so I can't even get to the point where I can try your suggestion @davesisson

Disclaimer, I am very new to Golang so it could be something stupid that I am doing, but having a working, compiling starting point would really help me to get started.

[nikunjy]

Okay I wrote an implementation. I don't think it is the correct way of writing it but it is the best I could do with my limited understanding.

https://github.com/nikunjy/antlr-calc-go

I spent couple of hours trying to understand the antlr golang code base (which I think is not well written and can be improved by collaboration). If there are authors/reviewers who will review the code then I would be down to improve it but looking at the past threads it seems like golang requests have died with time.

@davesisson can you comment on this code base and let me know if this is the right way of doing the implementation. After getting frustrated I ended up writing a Visit(ParseTree) for my visitor which I think is the wrong way of writing, and I'd like you to comment on that if you have the time

func (c *CalculatorVisitorImpl) Visit(tree antlr.ParseTree) interface{} {
	switch val := tree.(type) {
	case *CalculateContext:
		return val.Accept(c)
	case *ToSetVarContext:
		return val.Accept(c)
	}
	return nil
}

I am having to write the Visit with this type switch statement, i would also have to probably write VisitChildren.

[nikunjy]

okay I think i am getting the hang of it (?)
Wrote a generic rules engine too if anyone is looking for that kind of thing
https://github.com/nikunjy/rules

[nikunjy]

Any comments here @davesisson ?

[apzimmer]

@nikunjy what do you think is wrong with your example? @parrt Huge fan of Antlr4, and agreed it is worth placing a warning here until the visitor pattern is known to work in Go.

[cvgw]

I've been able to get the visitor working but the default code generated with the -vistor option for golang seems like it could be improved.

The most glaring issue (imo) is the composition of the Base{xxxxx}Visitor type. The way the composition and the methods currently are it looks like you could do some inheritance/method overriding (similar to how listener works), but it does not work the way I think most users would reasonably expect.

From my trials the most effective solution was to copy every method from the Base{xxxxx}Visitor generated by antlr and define each method on a new visitor type.

The main issue seems to be with VisitChildren which is defined on the absolute base in the antlr package. I think the idea was that users should be able to override the VisitChildren in their unique implementation of visitor, but I don't believe golang composition works that way. Perhaps the implementation was copied from Java without enough refactoring for golang?

I apologize that I have not included a suggested fix, but I have yet to come up with one.

Example:

type FooVisitor struct {
  BaseFooGrammarVisitor
}

func (v *FooVisitor) VisitChildren(ctx antlr.RuleNode) interface{} {
// do some stuff
}

func (v *FooVisitor) VisitSomeRule(ctx SomeRuleContext) interface{} {
  return v.VisitChildren(ctx)
}

v := &FooVisitor{}

start.Accept(v)

// call *FooVisitor.VisitSomeRule() => *FooVisitor.VisitChildren() => *BaseFooGrammarVisitor.VisitRuleNotOverriden() =>  *BaseParseTreeVisitor.VisitChildren
// *BasePareTreeVisitor.VisitChildren always returns nil

Here is a small go playground demo which might to help highlight my thinking https://play.golang.com/p/oVqsS6eAZPf
Apologies if I have made any omissions or mistakes.

[DavidGamba]

This is my current implementation of the calculator from chapter 4 using the visitor pattern.
https://github.com/DavidGamba/go-antlr-calc
Hopefully someone finds it useful. Couldn't get the underline error to work.
As mentioned before, had to implement every method.

[mttbx]

I really need a elegent antlr visitor for go. Is anybody fix it? Can you send a PR?

[mcolburn]

I also would like to see an official example on how to use the visitor for golang antlr.

[mitar]

There are few PRs which are improving state here:

• [Go] Add Super property for Base*Visitor struct #3087
• Implement runtime/Go Visit methods for the BaseParseTreeVisitor. #3086

[prnvbn]

I have personally used #3086 to write a compiler for a language. Before this the Visitor pattern was non-exist. Any other suggested features for this PR are also welcome.

[parrt]

Hi Guys, sounds like the Go target needs some love and previous guy can't work on it anymore. The tests seem to fail even. Shall we organize a posse to solve this among the folks here? I don't know Go.

[Joakker]

I'd be willing to help. Personally I opened #3066 to improve the documentation, and have created a parser implementation here in go, so I'd consider myself moderately knowledgeable in the subject

[prnvbn]

I would also like to help! I used Go and ANTLR to write a compiler for a language(WACC) and also opened up #3086. Would definitely want to see better Go support in ANTLR.

[parrt]

okay, so it sounds like we have @Joakker and @prnvbn as a Go team!

Could we start by figuring out what's up with the feeling test suite for Go? Then we can add the cool new things you got. I will start a new Go issue

[Joakker]

Now that the matter of the test suite is settled, I believe the next course of action would be to clean up the go runtime and divide it into smaller subpackages because the codebase is pretty cluttered and difficult to navigate as is.

This, of course, would break the tests again, but I plan on addressing the matter through many smaller PRs so things will break small.

[parrt]

Gang, it looks like we have multiple related Go visitor PRs... once the world starts up again next week might be good to take a look at things; adding @jcking here as well

[Sauci]

Hello all, any news on this topic?

[elexx]

Hello. Is there any news on this issue?

[parrt]

Hi. Go target is currently being worked on but I'm unclear how long it will be before we have an update. The main developer is busy at work. I still haven't learned enough Go to make progress myself.

[perezd]

Am I correct to understand that -visitor is now implemented as expected?
https://github.com/antlr/antlr4/releases/tag/4.11.0

[parrt]

Heh @jimidle can you remind me where we are on Go visitors?

[perezd]

FWIW, when I run with -visitor, I see files! That's gotta be a good sign right? :)

[parrt]

a great sign and @jimidle has done a HUGE overhaul of target.

[jimidle]

The visitor and listener all seem to work. I am using a listener but not a visitor.

Create your own struct with the generate base struct embedded in it, then implement the funcs you want to override on your created struct. If you do not override a func in the base, then the base will be called and it will do nothing. If you have a func in your local struct, then it will be called with that receiver. I have not looked in to the visitor too much, but the listener works as advertised. I have no reason to believe that the visitor does not work, but I will do a little research on it and obviously fix anything that needs fixing.

[perezd]

@jimidle So I tried what you suggested but it seems to be misbehaving, here's a repo w/ a devcontainer all set up w/ deps and what not to repro it: https://github.com/perezd/visitor-example

Am I just doing this wrong somehow? "HERE" never gets printed...I'm trying the standard JSON grammar.

[ghost]

Hi @perezd,

I Had a similar experience.

I noticed some nice comments in "tree.go" on line 72 to 91

they may be related to the problem, am not sure though.

// TODO
//func (this ParseTreeVisitor) Visit(ctx) {
//	if (Utils.isArray(ctx)) {
//		self := this
//		return ctx.map(function(child) { return VisitAtom(self, child)})
//	} else {
//		return VisitAtom(this, ctx)
//	}
//}
//
//func VisitAtom(Visitor, ctx) {
//	if (ctx.parser == nil) { //is terminal
//		return
//	}
//
//	name := ctx.parser.ruleNames[ctx.ruleIndex]
//	funcName := "Visit" + Utils.titleCase(name)
//
//	return Visitor[funcName](ctx)
//}

using the suggestion at #1841 (comment)

I got Visit and VisitChildren - when inheriting from antlr.ParseTreeVisitor - to say Hello,

The rule-based functions seems to be overridden when inheriting from Base....ParserVisitor , they seems to be on mute for now

type ...ParserVisitorImpl struct {
	antlr.ParseTreeVisitor
	//ParseTreeVisitor *Base...ParserVisitor
}

getting to understand all the involved antlr code to get something to work again may be quite an occupation

no quick win possible using the visitor pattern in go I suppose

[ghost]

I recommend to use a tree walker instead.

An example that can be transposed to go is available at https://tomassetti.me/antlr-mega-tutorial/#chapter38

[ghost]

I got something to work using the "visitor" pattern.

I share this to @perezd, @parrt, @pboyer and @jimidle, @danaugrs with enthusiasm

based on https://www.antlr.org/download/antlr-4.11.1-complete.jar

gaining inspiration from

• https://stackoverflow.com/questions/65038949/how-to-write-a-antlr4-visitor
• Go Target - Visit implementation - two other minor changes runtimeimport tokenstream #1841 (comment)

antlr4 -Dlanguage=Go -visitor -no-listener -package parser -o ./ parser/Blah.g4

type BlahVisitorImpl struct {
	BaseBlahVisitor
}

implement Visit and VisitChildren similar to stated in #1841 (comment)

override all visit methods towards and including all levels (deep) in the grammer you want to visit, based on the generated BaseBlahVisitor, style. copy pased from BaseBlahVisitor and change to BlahVisitorImpl basically and do your other stuff where needed

func (v *BlahVisitorImpl) VisitVisitBlahLevel1(ctx *Snowflake_fileContext) interface{} {
	fmt.Println(fmt.Sprintf("DEBUG VisitBlahLevel1"))
	return v.VisitChildren(ctx)
}

func (v *BlahVisitorImpl) VisitVisitBlahLevel2(ctx *Snowflake_fileContext) interface{} {
	fmt.Println(fmt.Sprintf("DEBUG VisitBlahLevel2"))
	return v.VisitChildren(ctx)
}

// ...

at each level, the return v.VisitChildren(ctx) call at the end seems to steer the "visit"

the main body would be something similar to

input := antlr.NewInputStream(strSqlText)
lexer := parser.NewBlahLexer(input)
stream := antlr.NewCommonTokenStream(lexer, antlr.TokenDefaultChannel)
myparser := parser.NewBlahParser(stream)
myparser.BuildParseTrees = true
myparser.AddErrorListener(antlr.NewDiagnosticErrorListener(true))
fileContext := myparser.Level1()
visitor := parser.BlahVisitorImpl{}
visitor.Visit(fileContext)

I admit that a working reference example - maintained per release - would be nice

[fikin]

i think we should have different visitor template generated by antlr for go.

currently generated class and its base are influenced way too literally by java. in go inheritance and polymorphism are not implemented by structure composition (as facilitated by generated visitor and corresponding antlr base class), but by code duplication and execution branching.
plus the other problems with current template, like use of interface{} which leads to extra type casting and error handling for the calling code, and lack of error in returned arguments makes this generated+base class code, imho completely useless.

for example, the most straightforward visitor to print entire tree are 2 general purpose functions and nothing more:

func printNodeVisitor(node antlr.Tree) (string,error) {
  switch n:= node.(type) {
    case antlr.ErrorNode:
      return "", fmt.Errorf(">>%s<<",n.(antlr.ParseTree).GetText())
    case antlr.TerminalNode:
      return n.(antlr.ParseTree).GetText(), nil
    default:
      return printNodeChildrenVisitor(n)
  }
}

func printNodeChildrenVisitor(node antlr.Tree) (string, error) {
  arr := []string{}
  for _, c := range node.GetChildren() {
    str, err := printNodeVisitor(c)
    if err != nil {
      return "", err
    }
    arr = append(arr, str)
  }
  return strings.Join(arr, ""), nil
}

of course, in practice one could use a bit more stricter form of switch construct:

  switch n:= node.(type) {
    case IXYZContext, IDEFContext: /* these are generated interfaces, for each parser rule */
      return printNodeChildrenVisitor(n) /* node specific logic */
    case antlr.ErrorNode:
      return "", fmt.Errorf(">>%s<<",n.(antlr.ParseTree).GetText())
    case antlr.TerminalNode:
      return n.(antlr.ParseTree).GetText(), nil
    default:
      return "", fmt.Errorf("unrecognized node type : %v", n)
  }

and use purpose designed typed arguments, suitable for the visiting task.

i'm of the opinion that for golang target, antlr should generate template functions with switch statements and ask the coder to customize them as per need.
current code is generally not beneficial for any use case and basically it is only complicating the end result.

[hugheaves]

Just an an "FYI" in case anyone else runs into this issue:

My current "workaround" for this is to execute a one-line patch on the generated visitor code during the build process. The patch makes the "VisitChildren" function a function pointer in the BaseVisitor struct, so that it can be overridden in my Visitor implementation.

Here's the patch:
https://github.com/hugheaves/scadformat/blob/main/internal/parser/openscad_base_visitor.go.patch

Here's where VisitChildren is overridden:
https://github.com/hugheaves/scadformat/blob/018681900884365676409ae2fddef814d76bf60e/internal/formatter/formattingvisitor.go#L50

It's obviously a bit of a hack, but it avoids having to implement every VisitXXX function in my visitor.

[codershangfeng]

@hugheaves Yep, your approach works well on my side, but I keep the *antlr.BaseParseTreeVisitor in the generated base visitor to avoid impl. it in the customized visitor 😄

[jimidle]

I really need to crate a sample project. I have used both the visitor and the walker in anger. The thing to remember is that Go isn't a class based language. I am putting in some time on ANTLR shortly and will provide an example. I have tried to remove embeddings via pointers as that isn't the right way but it was initially implemented like that. I will try to provide an example.

…

On Mar 7 2024, at 2:31 am, Liu Shangfeng ***@***.***> wrote: @hugheaves (https://github.com/hugheaves) Yep, your approach works well on my side, but I keep the *antlr.BaseParseTreeVisitor in the generated base visitor to avoid impl. it in the customized visitor 😄 — Reply to this email directly, view it on GitHub (#2504 (comment)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAJ7TMAS6E3CPQBTI44KLSLYXAQUNAVCNFSM4G4D3LY2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJYGI4TEMZUHEZQ). You are receiving this because you were mentioned.

[hugheaves]

I really need to crate a sample project. I have used both the visitor and the walker in anger. The thing to remember is that Go isn't a class based language. I am putting in some time on ANTLR shortly and will provide an example. I have tried to remove embeddings via pointers as that isn't the right way but it was initially implemented like that. I will try to provide an example.

That would be helpful. Because Go is not a "class based language", the approach you describe above (#2504 (comment)) does not work.

[jimidle]

It does for me. But I think that the documentation (I have improved it but still needs more I think) is lacing in this regard. A working example is always going to be better than some code snippets etc. JIm

…

On Mar 7 2024, at 11:17 am, Hugh Eaves ***@***.***> wrote: > I really need to crate a sample project. I have used both the visitor and the walker in anger. The thing to remember is that Go isn't a class based language. I am putting in some time on ANTLR shortly and will provide an example. I have tried to remove embeddings via pointers as that isn't the right way but it was initially implemented like that. I will try to provide an example. > … (#) > On Mar 7 2024, at 2:31 am, Liu Shangfeng @.***> wrote: @hugheaves (https://github.com/hugheaves) (https://github.com/hugheaves) Yep, your approach works well on my side, but I keep the *antlr.BaseParseTreeVisitor in the generated base visitor to avoid impl. it in the customized visitor 😄 — Reply to this email directly, view it on GitHub (#2504 (comment) (#2504 (comment))), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAJ7TMAS6E3CPQBTI44KLSLYXAQUNAVCNFSM4G4D3LY2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJYGI4TEMZUHEZQ). You are receiving this because you were mentioned. > That would be helpful. Because Go is not a "class based language", the approach you describe above (#2504 (comment) (#2504 (comment))) does not work. — Reply to this email directly, view it on GitHub (#2504 (comment)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAJ7TMGVBW2UFPTQ6WCSXVLYXCOJDAVCNFSM4G4D3LY2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJYGQYDGOBUGU2Q). You are receiving this because you were mentioned.

[zbakkernomi]

Hello - any updates here? in go, embedded struct methods receive the embedded member, and with noop antlr.BaseParseTreeVisitor.VisitChildren base visitors are basically useless. yes, you can just implement the entire visitor interface, but when dealing with very large grammars this is untenable. Thanks

[lschmierer]

@fikin thank you for your input, your comment was very helpful!

I agree that working with the generated visitor seems not idiomatic and does not feel very natural. After fiddeling around with the generated visitor, I am now going with this pattern (as suggested by @fikin):

func evalExpression(target Collection, tree parser.IExpressionContext) (Collection, error) {
	switch t := tree.(type) {
	case *parser.InvocationExpressionContext:
		expr, err := evalExpression(target, t.Expression())
		if err != nil {
			return nil, err
		}
		return evalInvocation(expr, t.Invocation())
	case *parser.TermExpressionContext:
		return evalTerm(target, t.Term())	
	// all the other cases...	
	default:
		// as long as all cases are handled, this should never happen
		panic(fmt.Sprintf("unexpected expression %T", tree))
	}
}

Aside from feeling more concise and easier, it allows passing down parameter (vs. maintaining state in the visitor) and returning concrete values (vs. interface{}).
One drawback (among others that I currently dont see) is that you have to implement each visit manually, there is no default VisitChildren (which most of the confusion is about anyway).