Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ MAKEFLAGS = -s
export GOBIN=$(PWD)/bin
export GO111MODULE=on
export GODEBUG=tls13=0
export REWRITER=go/vt/sqlparser/rewriter.go

# Disabled parallel processing of target prerequisites to avoid that integration tests are racing each other (e.g. for ports) and may fail.
# Since we are not using this Makefile for compilation, limiting parallelism will not increase build time.
Expand Down Expand Up @@ -87,6 +88,11 @@ install: build
parser:
make -C go/vt/sqlparser

visitor:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

shouldn't we be calling this whenever there is a change to sql.y (and sql.go)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not really. only when there is a change to ast.go

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

will people need to know to call this manually? is there a test that will catch it if someone forgets to do so after changing ast.go?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added pre-commit hook

go build -o visitorgen go/visitorgen/main/main.go
./visitorgen -input=go/vt/sqlparser/ast.go -output=$(REWRITER)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's better to just go run go/visitorgen/main/main.go -input go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go. Then you don't have to worry about cleanup of the binary. I just tried, and it works as intended. For some reason, I had to remove the = after input.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I can't get this to run on my Mac. All I get is:

> go run -o visitorgen go/visitorgen/main/main.go -input go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go
flag provided but not defined: -o
usage: go run [build flags] [-exec xprog] package [arguments...]
Run 'go help run' for details.
 > go run -o visitorgen go/visitorgen/main/main.go -input=go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go
flag provided but not defined: -o
usage: go run [build flags] [-exec xprog] package [arguments...]
Run 'go help run' for details.
> go run -o visitorgen go/visitorgen/main/main.go -input=go/vt/sqlparser/ast.go -output go/vt/sqlparser/rewriter.go
flag provided but not defined: -o
usage: go run [build flags] [-exec xprog] package [arguments...]
Run 'go help run' for details.
> go run -o visitorgen go/visitorgen/main/main.go -input go/vt/sqlparser/ast.go -output go/vt/sqlparser/rewriter.go
flag provided but not defined: -o
usage: go run [build flags] [-exec xprog] package [arguments...]
Run 'go help run' for details.

Copy link
Copy Markdown
Collaborator

@dweitzman dweitzman Jan 21, 2020

Choose a reason for hiding this comment

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

I agree with Sugu that go run would be much nicer here. In the error output you saw, it looks like you had go run -o vistorgen and an error about the -o. If you get rid of those arguments it seems like go run should work

rm ./visitorgen
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How do you feel about having the Makefile invoke go generate and the //go:generate annotation could have the go run command?

Being able to use pure go build tools seems nice. Anyone who wants to use make to invoke go would have the option, although I don't think it would do much for them.


# To pass extra flags, run test.go manually.
# For example: go run test.go -docker=false -- --extra-flag
# For more info see: go run test.go -help
Expand All @@ -100,9 +106,10 @@ clean:
go clean -i ./go/...
rm -rf third_party/acolyte
rm -rf go/vt/.proto.tmp
rm -rf ./visitorgen

# Remove everything including stuff pulled down by bootstrap.sh
cleanall:
cleanall: clean
# directories created by bootstrap.sh
# - exclude vtdataroot and vthook as they may have data we want
rm -rf bin dist lib pkg
Expand Down
130 changes: 130 additions & 0 deletions go/visitorgen/ast_walker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What do you think about putting this package at go/ast/visitorgen so it's more clear that it's related to ast?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Could do. I was thinking I wanted to use visitorgen for our Primitive plans as well. We don't rewrite them today, but we do visit them using the Find method, and I foresee rewriting of plans in our future as well.

Never mind - it will be easy to move it somewhere else when we do decide to start using it for more than the ast. For now, I'll move it into the sqlparser package

Copyright 2019 The Vitess Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package visitorgen

import (
"go/ast"
"reflect"
)

var _ ast.Visitor = (*walker)(nil)

type walker struct {
result SourceFile
}

// Walk walks the given AST and translates it to the simplified AST used by the next steps
func Walk(node ast.Node) *SourceFile {
var w walker
ast.Walk(&w, node)
return &w.result
}

// Visit implements the ast.Visitor interface
func (w *walker) Visit(node ast.Node) ast.Visitor {
switch n := node.(type) {
case *ast.TypeSpec:
switch t2 := n.Type.(type) {
case *ast.InterfaceType:
w.append(&InterfaceDeclaration{
name: n.Name.Name,
block: "",
})
case *ast.StructType:
var fields []*Field
for _, f := range t2.Fields.List {
for _, name := range f.Names {
fields = append(fields, &Field{
name: name.Name,
typ: sastType(f.Type),
})
}

}
w.append(&StructDeclaration{
name: n.Name.Name,
fields: fields,
})
case *ast.ArrayType:
w.append(&TypeAlias{
name: n.Name.Name,
typ: &Array{inner: sastType(t2.Elt)},
})
case *ast.Ident:
w.append(&TypeAlias{
name: n.Name.Name,
typ: &TypeString{t2.Name},
})

default:
panic(reflect.TypeOf(t2))
}
case *ast.FuncDecl:
if len(n.Recv.List) > 1 || len(n.Recv.List[0].Names) > 1 {
panic("don't know what to do!")
}
var f *Field
if len(n.Recv.List) == 1 {
r := n.Recv.List[0]
t := sastType(r.Type)
if len(r.Names) > 1 {
panic("don't know what to do!")
}
if len(r.Names) == 1 {
f = &Field{
name: r.Names[0].Name,
typ: t,
}
} else {
f = &Field{
name: "",
typ: t,
}
}
}

w.append(&FuncDeclaration{
receiver: f,
name: n.Name.Name,
block: "",
arguments: nil,
})
}

return w
}

func (w *walker) append(line Sast) {
w.result.lines = append(w.result.lines, line)
}

func sastType(e ast.Expr) Type {
switch n := e.(type) {
case *ast.StarExpr:
return &Ref{sastType(n.X)}
case *ast.Ident:
return &TypeString{n.Name}
case *ast.ArrayType:
return &Array{inner: sastType(n.Elt)}
case *ast.InterfaceType:
return &TypeString{"interface{}"}
case *ast.StructType:
return &TypeString{"struct{}"}
}

panic(reflect.TypeOf(e))
}
Loading