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

Visitor Pattern in Golang #4395

Closed
mkohlhaas opened this issue Aug 26, 2023 · 7 comments
Closed

Visitor Pattern in Golang #4395

mkohlhaas opened this issue Aug 26, 2023 · 7 comments

Comments

@mkohlhaas
Copy link

mkohlhaas commented Aug 26, 2023

No description provided.

@kaby76
Copy link
Contributor

kaby76 commented Aug 27, 2023

Try implementing a dispatch routine. E.g.,

func (v *Visitor) Visit(tree antlr.ParseTree) any {
    switch val := tree.(type) {
    case *parser.ProgContext:
        return v.VisitProg(val)
    case *parser.DeclContext:
        return v.VisitDecl(val)
    case *parser.ExprContext:
        return v.VisitExpr(val)
    }
    return 0
}

@kaby76
Copy link
Contributor

kaby76 commented Aug 27, 2023

There is already a Visit method in the base class which I use and should be used.

Of course. The Antlr Visitor code doesn't work. Likely for Listeners as well.

#2504
#3265
#3812
#1843

But, you asked: "Anyone [have] any idea how the visitor pattern should work in Go?". I am just saying, that if you are trying to get something to work now, then you might want to give that a try.

@kaby76
Copy link
Contributor

kaby76 commented Aug 28, 2023

I think an end-to-end example of a Visitor is in order. The doc only shows a Listener. Both are needed.

I could not get Antlr Visitors to work unless I do the following:

  • Define Visitor as you describe:
    type Visitor struct {
        parser.BaseExprVisitor
    }
    
  • Define Visit() as you describe:
    func (v *Visitor) Visit(tree antlr.ParseTree) any {
        return tree.Accept(v)
    }
    
  • Define each and every Visit...() function:
    func (v *Visitor) VisitProg(ctx *parser.ProgContext) any {
        fmt.Println("VisitProg")
        return v.VisitChildren(ctx)
    }
    
    func (v *Visitor) VisitDecl(ctx *parser.DeclContext) any {
        fmt.Println("VisitDecl")
        return v.VisitChildren(ctx)
    }
    
    func (v *Visitor) VisitExpr(ctx *parser.ExprContext) any {
        fmt.Println("VisitExpr")
        return v.VisitChildren(ctx)
    }
    
    In other words, the generated Visit...() functions do not work:
    func (v *BaseExprVisitor) VisitProg(ctx *ProgContext) interface{} {
        return v.VisitChildren(ctx)
    }
    
    func (v *BaseExprVisitor) VisitDecl(ctx *DeclContext) interface{} {
        return v.VisitChildren(ctx)
    }
    
    func (v *BaseExprVisitor) VisitExpr(ctx *ExprContext) interface{} {
        return v.VisitChildren(ctx)
    }
    
  • Define VisitChildren(), because the default implementation is "return nil".
    func (v *Visitor) VisitChildren(tree antlr.RuleNode) any {
        n := tree.GetChildCount()
        for i := 0; i < n; i++ {
            c := tree.GetChild(i)
            val := c.(antlr.ParseTree)
            _ = v.Visit(val)
        }
        return 0
    }
    
    Note, this implementation does not "sum" the results of visiting the children, nor does it "short-circuit" the tree walking.

Also, in the example in the doc for the listener, the struct type declares the base as pointer.

*parser.BaseJSONListener
If I try to do the same for a Visitor, the tree walker crashes because the VisitErrorNode() and VisitTerminal() funcs also need to be defined:

func (v *Visitor) VisitErrorNode(_ antlr.ErrorNode) interface{}   { return nil }
func (v *Visitor) VisitTerminal(_ antlr.TerminalNode) interface{} { return nil }

VisitProg() must still be defined.

@jimidle
Copy link
Collaborator

jimidle commented Aug 28, 2023 via email

@kaby76
Copy link
Contributor

kaby76 commented Aug 28, 2023

The code attached implements a Visitor for the Java and Go targets. I have one for CSharp, but it behaves like the Java target, so I haven't included it. The applications are generated for Windows Msys2.

In this example, I define all Visit...() methods/func except ProgVisit().

For Java, the tree walker visits the entire tree, which you can see by the output for DeclVisit() and ExprVisit().

For Go, the visitor does not appear to visit the Expr and Decl nodes because "func ProgVisit()" is commented out. If "func ProgVisit()" is uncommented, I get the output from the ExprVisit() and DeclVisit() funcs. I haven't determined whether it calls the Antlr tool generated func's, but it certainly doesn't call the ones that I define.

08/28-02:29:36 ~/issues/antlr-4395/x/Generated-Java
$ ls
build.sh  ErrorListener.java  ExprBaseVisitor.java  makefile                        perf.sh  Test.java  test1.txt
clean.sh  Expr.g4             ExprVisitor.java      MyDiagnosticErrorListener.java  run.sh   test.sh    Visitor.java
08/28-02:29:37 ~/issues/antlr-4395/x/Generated-Java
$ make
bash build.sh
08/28-02:29:45 ~/issues/antlr-4395/x/Generated-Java
$ bash run.sh test1.txt
visitDecl
visitDecl
visitExpr
visitExpr
visitExpr
visitExpr
visitExpr
visitExpr
visitDecl
Java 0 test1.txt success 0.011
Total Time: 0.115
08/28-02:30:16 ~/issues/antlr-4395/x/Generated-Java
$ cd ..
08/28-02:30:19 ~/issues/antlr-4395/x
$ cd Generated-Go/
08/28-02:30:23 ~/issues/antlr-4395/x/Generated-Go
$ ls
build.sh  clean.sh  go.mod  go.sum  makefile  parser/  perf.sh  run.sh  Test.go  test.sh  test1.txt
08/28-02:30:26 ~/issues/antlr-4395/x/Generated-Go
$ make
bash build.sh
08/28-02:30:32 ~/issues/antlr-4395/x/Generated-Go
$ bash run.sh test1.txt
Go 0 test1.txt success 0.000
Total Time: 0.001 s
08/28-02:30:40 ~/issues/antlr-4395/x/Generated-Go
$

Here are the two applications in .zip files.

Generated-Java.zip
Generated-Go.zip

Again, just looking through the Antlr Go runtime, there's no func that defines VisitChildren() that actually traverses all children of a tree node. The only VisitChildren() implementation is the stub implementation in tree.go.

08/28-02:54:29 /c/Users/Kenne/Documents/GitHub/antlr4-dev/runtime/Go/antlr/v4
$ grep VisitChildren *.go
parser_rule_context.go: return visitor.VisitChildren(prc)
tree.go:        VisitChildren(node RuleNode) interface{}
tree.go:func (v *BaseParseTreeVisitor) VisitChildren(_ RuleNode) interface{}     { return nil }
08/28-02:54:38 /c/Users/Kenne/Documents/GitHub/antlr4-dev/runtime/Go/antlr/v4
$ pushd
~/issues/antlr-4395/x/Generated-Go /c/Users/Kenne/Documents/GitHub/antlr4-dev/runtime/Go/antlr/v4
08/28-02:57:48 ~/issues/antlr-4395/x/Generated-Go
$ grep VisitChildren *.go
        return v.VisitChildren(ctx)
        return v.VisitChildren(ctx)
        return v.VisitChildren(ctx)
func (v *Visitor) VisitChildren(tree antlr.RuleNode) any {
08/28-02:57:59 ~/issues/antlr-4395/x/Generated-Go
$ grep VisitChildren parser/*.go
parser/expr_base_visitor.go:    return v.VisitChildren(ctx)
parser/expr_base_visitor.go:    return v.VisitChildren(ctx)
parser/expr_base_visitor.go:    return v.VisitChildren(ctx)
parser/expr_parser.go:          return t.VisitChildren(s)
parser/expr_parser.go:          return t.VisitChildren(s)
parser/expr_parser.go:          return t.VisitChildren(s)
08/28-02:58:15 ~/issues/antlr-4395/x/Generated-Go
$

Every other runtime has an implementation somewhere to loop through all children of a parse tree node. I just don't see how this could have ever worked for Go--unless one implements a VisitChildren(). as I have done

@kaby76
Copy link
Contributor

kaby76 commented Aug 28, 2023

Nice. But this is not the visitor pattern:

func (v *Visitor) Visit(tree antlr.ParseTree) any {
    return tree.Accept(v)
}

The method from the base class should be used.

That function is essentially the code that you gave above!

func (v *Visitor) Visit(tree antlr.ParseTree) any {
    return tree.accept(v)
}

But, besides, the implementation I made is equivalent to the "base class" implementation except for the function receiver.

func (v *BaseParseTreeVisitor) Visit(tree ParseTree) interface{} { return tree.Accept(v) }

Doing what you suggest--commenting out the Visit() implementation in Test.go that I made--does not work either.

It's moot anyway because the overarching fact remains: VisitChildren() is unimplemented, yet the Antlr tool generates code with VisitChildren() function calls. The Visitor code can only work with all the func's implemented as I mentioned above.

@kaby76
Copy link
Contributor

kaby76 commented Aug 28, 2023

It's moot anyway because the overarching fact remains: VisitChildren() is unimplemented, yet the Antlr tool generates
code with VisitChildren() function calls. The Visitor code can only work with all the func's implemented as
#4395 (comment).

It's not moot. It's a bug!

I really don't know what you are referring to. What is "it" in "It's a bug!"?? So as to be clear, I've checked in all the generated driver code here, https://github.com/kaby76/antlr-4395, so you can use Github Permalinks to address what "it" is. Thanks.

@mkohlhaas mkohlhaas reopened this Aug 29, 2023
@mkohlhaas mkohlhaas closed this as not planned Won't fix, can't repro, duplicate, stale Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants