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

Jump to bottom

cmd/compile: refactor generic visitor code #42981

Closed
mdempsky opened this issue Dec 3, 2020 · 5 comments
Closed

cmd/compile: refactor generic visitor code #42981

mdempsky opened this issue Dec 3, 2020 · 5 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.

Comments

@mdempsky
Copy link
Member

mdempsky commented Dec 3, 2020 •
edited

There's a bunch of implementations of ad hoc, generic IR tree walking code within the compiler frontend.

Examples:

go/src/cmd/compile/internal/gc/alg.go

Lines 789 to 814 in 351bc2f

if n.Left() != nil && hasCall(n.Left()) {
return true
}
if n.Right() != nil && hasCall(n.Right()) {
return true
}
for _, x := range n.Init().Slice() {
if hasCall(x) {
return true
}
}
for _, x := range n.Body().Slice() {
if hasCall(x) {
return true
}
}
for _, x := range n.List().Slice() {
if hasCall(x) {
return true
}
}
for _, x := range n.Rlist().Slice() {
if hasCall(x) {
return true
}
}

go/src/cmd/compile/internal/gc/const.go

Lines 810 to 818 in 351bc2f

if hascallchan(n.Left()) || hascallchan(n.Right()) {
return true
}
for _, n1 := range n.List().Slice() {
if hascallchan(n1) {
return true
}
}
for _, n2 := range n.Rlist().Slice() {

go/src/cmd/compile/internal/gc/inl.go

Lines 445 to 447 in 351bc2f

return v.visit(n.Left()) || v.visit(n.Right()) ||
v.visitList(n.List()) || v.visitList(n.Rlist()) ||
v.visitList(n.Init()) || v.visitList(n.Body())

go/src/cmd/compile/internal/gc/inl.go

Lines 490 to 503 in 351bc2f

cnt += countNodes(n.Left())
cnt += countNodes(n.Right())
for _, n1 := range n.Init().Slice() {
cnt += countNodes(n1)
}
for _, n1 := range n.Body().Slice() {
cnt += countNodes(n1)
}
for _, n1 := range n.List().Slice() {
cnt += countNodes(n1)
}
for _, n1 := range n.Rlist().Slice() {
cnt += countNodes(n1)
}

go/src/cmd/compile/internal/gc/inl.go

Lines 828 to 845 in 351bc2f

if a := v.visit(n.Left()); a != nil {
return a
}
if a := v.visit(n.Right()); a != nil {
return a
}
if a := v.visitList(n.List()); a != nil {
return a
}
if a := v.visitList(n.Rlist()); a != nil {
return a
}
if a := v.visitList(n.Init()); a != nil {
return a
}
if a := v.visitList(n.Body()); a != nil {
return a
}

go/src/cmd/compile/internal/gc/typecheck.go

Lines 3700 to 3705 in 351bc2f

markbreak(labels, n.Left(), implicit)
markbreak(labels, n.Right(), implicit)
markbreaklist(labels, n.Init(), implicit)
markbreaklist(labels, n.Body(), implicit)
markbreaklist(labels, n.List(), implicit)
markbreaklist(labels, n.Rlist(), implicit)

It should be possible to rewrite a lot of this code to use ir.Inspect or ir.InspectList instead. This will eventually be helpful for getting rid of the generic Left/Right/etc methods.

For the markbreak case (and maybe others), it might be useful to provide ir analogs to ast.Walk and ast.Visitor. (See also how ast.Inspect is implemented, as that API was inspirational for ir.Inspect.)

/cc @cuonglm
@mdempsky mdempsky added Suggested Issues that may be good for new contributors looking for work to do. help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Dec 3, 2020
@mdempsky
Copy link
Member Author

mdempsky commented Dec 3, 2020

/cc @rsc @zephyrtronium

@gopherbot
Copy link

Change https://golang.org/cl/275305 mentions this issue: [dev.regabi] cmd/compile: add RefersTo and DeclaredBy helpers

@rsc
Copy link
Contributor

rsc commented Dec 4, 2020 •
edited

I have been working on this the last couple days. Please don't start. CLs coming soon!

@rsc
Copy link
Contributor

rsc commented Dec 4, 2020

https://go-review.googlesource.com/c/go/+/275311 is the CL introducing the API. See the rest of the stack for uses.
Best to discuss on the CLs themselves - I get too much GitHub spam to keep up with the issue tracker.

@mdempsky
Copy link
Member Author

mdempsky commented Dec 4, 2020

Russ's CLs have landed. This is done now.

@mdempsky mdempsky closed this as completed Dec 4, 2020
@golang golang locked and limited conversation to collaborators Dec 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mdempsky @rsc @gopherbot