]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.regabi] cmd/compile: simplify ir.Find, replace ir.Inspect with ir.Visit
authorRuss Cox <rsc@golang.org>
Sat, 12 Dec 2020 23:50:21 +0000 (18:50 -0500)
committerRuss Cox <rsc@golang.org>
Thu, 17 Dec 2020 03:50:26 +0000 (03:50 +0000)
It seems clear after using these for a week that Find need not return
anything other than a bool saying whether the target was found.
The main reason for not using the boolean earlier was to avoid confusion
with Inspect: for Find, returning true means "it was found! stop walking"
while for Inspect, returning true means "keep walking the children".

But it turns out that none of the uses of Inspect need the boolean.
This makes sense because types can contain expressions, expressions
can contain statements (inside function literals), and so on, so there
are essentially no times when you can say based on the current AST node
that the children are irrelevant to a particular operation.

So this CL makes two changes:

1) Change Find to return a boolean and to take a callback function
returning a boolean. This simplifies all existing calls to Find.

2) Rename Inspect to Visit and change it to take a callback with no
result at all. This simplifies all existing calls to Inspect.

Removing the boolean result from Inspect's callback avoids having
two callbacks with contradictory boolean results in different APIs.
Renaming Inspect to Visit avoids confusion with ast.Inspect.

Passes buildall w/ toolstash -cmp.

Change-Id: I344ebb5e00b6842012be33e779db483c28e5f350
Reviewed-on: https://go-review.googlesource.com/c/go/+/277919
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/cmd/compile/internal/gc/alg.go
src/cmd/compile/internal/gc/const.go
src/cmd/compile/internal/gc/dcl.go
src/cmd/compile/internal/gc/escape.go
src/cmd/compile/internal/gc/initorder.go
src/cmd/compile/internal/gc/inl.go
src/cmd/compile/internal/gc/scc.go
src/cmd/compile/internal/gc/walk.go
src/cmd/compile/internal/ir/visit.go

index 8550edb9e0fb56e74b26280451c23049d05cc832..3938dce46cebcfadeadbfad4fc934a6d4832b18f 100644 (file)
@@ -783,13 +783,11 @@ func geneq(t *types.Type) *obj.LSym {
 }
 
 func hasCall(fn *ir.Func) bool {
-       found := ir.Find(fn, func(n ir.Node) interface{} {
-               if op := n.Op(); op == ir.OCALL || op == ir.OCALLFUNC {
-                       return n
-               }
-               return nil
+       return ir.Find(fn, func(n ir.Node) bool {
+               // TODO(rsc): No methods?
+               op := n.Op()
+               return op == ir.OCALL || op == ir.OCALLFUNC
        })
-       return found != nil
 }
 
 // eqfield returns the node
index 677ed17dd9ee6fadb2e77c9c83d37461c128ac46..1ef199c793457acb96da6a7c578c6e4566314f61 100644 (file)
@@ -781,7 +781,7 @@ func isGoConst(n ir.Node) bool {
 
 // hasCallOrChan reports whether n contains any calls or channel operations.
 func hasCallOrChan(n ir.Node) bool {
-       found := ir.Find(n, func(n ir.Node) interface{} {
+       return ir.Find(n, func(n ir.Node) bool {
                switch n.Op() {
                case ir.OAPPEND,
                        ir.OCALL,
@@ -803,11 +803,10 @@ func hasCallOrChan(n ir.Node) bool {
                        ir.OREAL,
                        ir.ORECOVER,
                        ir.ORECV:
-                       return n
+                       return true
                }
-               return nil
+               return false
        })
-       return found != nil
 }
 
 // A constSet represents a set of Go constant expressions.
index 89873e2facc473ac18536e9e3a33b5118129324a..ad2dc99f89082d99171ec449e05a6e595c8cef84 100644 (file)
@@ -855,22 +855,22 @@ func newNowritebarrierrecChecker() *nowritebarrierrecChecker {
                        continue
                }
                c.curfn = n.(*ir.Func)
-               ir.Inspect(n, c.findExtraCalls)
+               ir.Visit(n, c.findExtraCalls)
        }
        c.curfn = nil
        return c
 }
 
-func (c *nowritebarrierrecChecker) findExtraCalls(n ir.Node) bool {
+func (c *nowritebarrierrecChecker) findExtraCalls(n ir.Node) {
        if n.Op() != ir.OCALLFUNC {
-               return true
+               return
        }
        fn := n.Left()
        if fn == nil || fn.Op() != ir.ONAME || fn.Class() != ir.PFUNC || fn.Name().Defn == nil {
-               return true
+               return
        }
        if !isRuntimePkg(fn.Sym().Pkg) || fn.Sym().Name != "systemstack" {
-               return true
+               return
        }
 
        var callee *ir.Func
@@ -887,7 +887,6 @@ func (c *nowritebarrierrecChecker) findExtraCalls(n ir.Node) bool {
                base.Fatalf("expected ODCLFUNC node, got %+v", callee)
        }
        c.extraCalls[c.curfn] = append(c.extraCalls[c.curfn], nowritebarrierrecCall{callee, n.Pos()})
-       return true
 }
 
 // recordCall records a call from ODCLFUNC node "from", to function
index f317e9999cc536e89fc87c7a28e3f0d6170e1733..5fce1184481a1e08e924dfe79ada549baf298c0a 100644 (file)
@@ -225,7 +225,7 @@ func (e *Escape) walkFunc(fn *ir.Func) {
        fn.SetEsc(EscFuncStarted)
 
        // Identify labels that mark the head of an unstructured loop.
-       ir.InspectList(fn.Body(), func(n ir.Node) bool {
+       ir.Visit(fn, func(n ir.Node) {
                switch n.Op() {
                case ir.OLABEL:
                        if e.labels == nil {
@@ -240,8 +240,6 @@ func (e *Escape) walkFunc(fn *ir.Func) {
                                e.labels[n.Sym()] = looping
                        }
                }
-
-               return true
        })
 
        e.curfn = fn
index d39e8189d7a495de5fafdecb9620444dfb513c04..7870e00221aac9b666800679be682ed801a391ac 100644 (file)
@@ -268,18 +268,25 @@ func collectDeps(n ir.Node, transitive bool) ir.NameSet {
 type initDeps struct {
        transitive bool
        seen       ir.NameSet
+       cvisit     func(ir.Node)
 }
 
-func (d *initDeps) inspect(n ir.Node)      { ir.Inspect(n, d.visit) }
-func (d *initDeps) inspectList(l ir.Nodes) { ir.InspectList(l, d.visit) }
+func (d *initDeps) cachedVisit() func(ir.Node) {
+       if d.cvisit == nil {
+               d.cvisit = d.visit // cache closure
+       }
+       return d.cvisit
+}
+
+func (d *initDeps) inspect(n ir.Node)      { ir.Visit(n, d.cachedVisit()) }
+func (d *initDeps) inspectList(l ir.Nodes) { ir.VisitList(l, d.cachedVisit()) }
 
 // visit calls foundDep on any package-level functions or variables
 // referenced by n, if any.
-func (d *initDeps) visit(n ir.Node) bool {
+func (d *initDeps) visit(n ir.Node) {
        switch n.Op() {
        case ir.OMETHEXPR:
                d.foundDep(methodExprName(n))
-               return false
 
        case ir.ONAME:
                n := n.(*ir.Name)
@@ -294,8 +301,6 @@ func (d *initDeps) visit(n ir.Node) bool {
        case ir.ODOTMETH, ir.OCALLPART:
                d.foundDep(methodExprName(n))
        }
-
-       return true
 }
 
 // foundDep records that we've found a dependency on n by adding it to
index 04256d5aeb62fa7fe582866723ccaaa606e60dee..9342046dcce7bf584194200d34658525e801b351 100644 (file)
@@ -255,7 +255,7 @@ func inlFlood(n *ir.Name) {
        // Recursively identify all referenced functions for
        // reexport. We want to include even non-called functions,
        // because after inlining they might be callable.
-       ir.InspectList(ir.AsNodes(fn.Inl.Body), func(n ir.Node) bool {
+       ir.VisitList(ir.AsNodes(fn.Inl.Body), func(n ir.Node) {
                switch n.Op() {
                case ir.OMETHEXPR, ir.ODOTMETH:
                        inlFlood(methodExprName(n))
@@ -282,7 +282,6 @@ func inlFlood(n *ir.Name) {
                        //     inlFlood(n.Func.Closure.Func.Nname)
                        base.Fatalf("unexpected closure in inlinable function")
                }
-               return true
        })
 }
 
@@ -458,14 +457,10 @@ func (v *hairyVisitor) doNode(n ir.Node) error {
 
 func isBigFunc(fn *ir.Func) bool {
        budget := inlineBigFunctionNodes
-       over := ir.Find(fn, func(n ir.Node) interface{} {
+       return ir.Find(fn, func(n ir.Node) bool {
                budget--
-               if budget <= 0 {
-                       return n
-               }
-               return nil
+               return budget <= 0
        })
-       return over != nil
 }
 
 // Inlcalls/nodelist/node walks fn's statements and expressions and substitutes any
@@ -707,8 +702,6 @@ FindRHS:
        return rhs
 }
 
-var errFound = errors.New("found")
-
 // reassigned takes an ONAME node, walks the function in which it is defined, and returns a boolean
 // indicating whether the name has any assignments other than its declaration.
 // The second return value is the first such assignment encountered in the walk, if any. It is mostly
@@ -723,22 +716,21 @@ func reassigned(name *ir.Name) bool {
        if name.Curfn == nil {
                return true
        }
-       a := ir.Find(name.Curfn, func(n ir.Node) interface{} {
+       return ir.Find(name.Curfn, func(n ir.Node) bool {
                switch n.Op() {
                case ir.OAS:
                        if n.Left() == name && n != name.Defn {
-                               return n
+                               return true
                        }
                case ir.OAS2, ir.OAS2FUNC, ir.OAS2MAPR, ir.OAS2DOTTYPE:
                        for _, p := range n.List().Slice() {
                                if p == name && n != name.Defn {
-                                       return n
+                                       return true
                                }
                        }
                }
-               return nil
+               return false
        })
-       return a != nil
 }
 
 func inlParam(t *types.Field, as ir.Node, inlvars map[*ir.Name]ir.Node) ir.Node {
@@ -916,11 +908,10 @@ func mkinlcall(n ir.Node, fn *ir.Func, maxCost int32, inlMap map[*ir.Func]bool,
        }
 
        nreturns := 0
-       ir.InspectList(ir.AsNodes(fn.Inl.Body), func(n ir.Node) bool {
+       ir.VisitList(ir.AsNodes(fn.Inl.Body), func(n ir.Node) {
                if n != nil && n.Op() == ir.ORETURN {
                        nreturns++
                }
-               return true
        })
 
        // We can delay declaring+initializing result parameters if:
@@ -1287,11 +1278,10 @@ func pruneUnusedAutos(ll []*ir.Name, vis *hairyVisitor) []*ir.Name {
 // concrete-type method calls where applicable.
 func devirtualize(fn *ir.Func) {
        Curfn = fn
-       ir.InspectList(fn.Body(), func(n ir.Node) bool {
+       ir.VisitList(fn.Body(), func(n ir.Node) {
                if n.Op() == ir.OCALLINTER {
                        devirtualizeCall(n)
                }
-               return true
        })
 }
 
index 063aaa09bd2bdf2acd126d726bd4fd8dc7ff2cfb..fa7af1274b264705c539a1179ee1d4611c3d334e 100644 (file)
@@ -75,7 +75,7 @@ func (v *bottomUpVisitor) visit(n *ir.Func) uint32 {
        min := v.visitgen
        v.stack = append(v.stack, n)
 
-       ir.InspectList(n.Body(), func(n ir.Node) bool {
+       ir.Visit(n, func(n ir.Node) {
                switch n.Op() {
                case ir.ONAME:
                        if n.Class() == ir.PFUNC {
@@ -111,7 +111,6 @@ func (v *bottomUpVisitor) visit(n *ir.Func) uint32 {
                                min = m
                        }
                }
-               return true
        })
 
        if (min == id || min == id+1) && !n.IsHiddenClosure() {
index ad5103f8514c17cb0d2857884d665b18b96ed42c..041eb900c803ac7418510929864fe6f7332de1c3 100644 (file)
@@ -3764,11 +3764,11 @@ func usefield(n ir.Node) {
 
 // hasSideEffects reports whether n contains any operations that could have observable side effects.
 func hasSideEffects(n ir.Node) bool {
-       found := ir.Find(n, func(n ir.Node) interface{} {
+       return ir.Find(n, func(n ir.Node) bool {
                switch n.Op() {
                // Assume side effects unless we know otherwise.
                default:
-                       return n
+                       return true
 
                // No side effects here (arguments are checked separately).
                case ir.ONAME,
@@ -3824,29 +3824,28 @@ func hasSideEffects(n ir.Node) bool {
                        ir.OREAL,
                        ir.OIMAG,
                        ir.OCOMPLEX:
-                       return nil
+                       return false
 
                // Only possible side effect is division by zero.
                case ir.ODIV, ir.OMOD:
                        if n.Right().Op() != ir.OLITERAL || constant.Sign(n.Right().Val()) == 0 {
-                               return n
+                               return true
                        }
 
                // Only possible side effect is panic on invalid size,
                // but many makechan and makemap use size zero, which is definitely OK.
                case ir.OMAKECHAN, ir.OMAKEMAP:
                        if !ir.IsConst(n.Left(), constant.Int) || constant.Sign(n.Left().Val()) != 0 {
-                               return n
+                               return true
                        }
 
                // Only possible side effect is panic on invalid size.
                // TODO(rsc): Merge with previous case (probably breaks toolstash -cmp).
                case ir.OMAKESLICE, ir.OMAKESLICECOPY:
-                       return n
+                       return true
                }
-               return nil
+               return false
        })
-       return found != nil
 }
 
 // Rewrite
index 4f3575614df64987573f2fd73772a48eb99d2b97..bc2b8083ba29865f4ffec69171844e8911589d8d 100644 (file)
@@ -57,46 +57,40 @@ import (
 //     }
 //     do(root)
 //
-// The Inspect function illustrates a further simplification of the pattern,
-// only considering processing before visiting children, and letting
-// that processing decide whether children are visited at all:
+// The Visit function illustrates a further simplification of the pattern,
+// only processing before visiting children and never stopping:
 //
-//     func Inspect(n ir.Node, inspect func(ir.Node) bool) {
+//     func Visit(n ir.Node, visit func(ir.Node)) {
 //             var do func(ir.Node) error
 //             do = func(x ir.Node) error {
-//                     if inspect(x) {
-//                             ir.DoChildren(x, do)
-//                     }
-//                     return nil
+//                     visit(x)
+//                     return ir.DoChildren(x, do)
 //             }
 //             if n != nil {
-//                     do(n)
+//                     visit(n)
 //             }
 //     }
 //
 // The Find function illustrates a different simplification of the pattern,
 // visiting each node and then its children, recursively, until finding
-// a node x such that find(x) returns a non-nil result,
-// at which point the entire traversal stops:
+// a node x for which find(x) returns true, at which point the entire
+// traversal stops and returns true.
 //
-//     func Find(n ir.Node, find func(ir.Node) interface{}) interface{} {
+//     func Find(n ir.Node, find func(ir.Node)) bool {
 //             stop := errors.New("stop")
-//             var found interface{}
 //             var do func(ir.Node) error
 //             do = func(x ir.Node) error {
-//                     if v := find(x); v != nil {
-//                             found = v
+//                     if find(x) {
 //                             return stop
 //                     }
 //                     return ir.DoChildren(x, do)
 //             }
-//             do(n)
-//             return found
+//             return do(n) == stop
 //     }
 //
-// Inspect and Find are presented above as examples of how to use
+// Visit and Find are presented above as examples of how to use
 // DoChildren effectively, but of course, usage that fits within the
-// simplifications captured by Inspect or Find will be best served
+// simplifications captured by Visit or Find will be best served
 // by directly calling the ones provided by this package.
 func DoChildren(n Node, do func(Node) error) error {
        if n == nil {
@@ -122,71 +116,59 @@ func DoList(list Nodes, do func(Node) error) error {
        return nil
 }
 
-// Inspect visits each node x in the IR tree rooted at n
-// in a depth-first preorder traversal, calling inspect on each node visited.
-// If inspect(x) returns false, then Inspect skips over x's children.
-//
-// Note that the meaning of the boolean result in the callback function
-// passed to Inspect differs from that of Scan.
-// During Scan, if scan(x) returns false, then Scan stops the scan.
-// During Inspect, if inspect(x) returns false, then Inspect skips x's children
-// but continues with the remainder of the tree (x's siblings and so on).
-func Inspect(n Node, inspect func(Node) bool) {
+// Visit visits each non-nil node x in the IR tree rooted at n
+// in a depth-first preorder traversal, calling visit on each node visited.
+func Visit(n Node, visit func(Node)) {
        var do func(Node) error
        do = func(x Node) error {
-               if inspect(x) {
-                       DoChildren(x, do)
-               }
-               return nil
+               visit(x)
+               return DoChildren(x, do)
        }
        if n != nil {
                do(n)
        }
 }
 
-// InspectList calls Inspect(x, inspect) for each node x in the list.
-func InspectList(list Nodes, inspect func(Node) bool) {
+// VisitList calls Visit(x, visit) for each node x in the list.
+func VisitList(list Nodes, visit func(Node)) {
        for _, x := range list.Slice() {
-               Inspect(x, inspect)
+               Visit(x, visit)
        }
 }
 
 var stop = errors.New("stop")
 
 // Find looks for a non-nil node x in the IR tree rooted at n
-// for which find(x) returns a non-nil value.
+// for which find(x) returns true.
 // Find considers nodes in a depth-first, preorder traversal.
-// When Find finds a node x such that find(x) != nil,
-// Find ends the traversal and returns the value of find(x) immediately.
-// Otherwise Find returns nil.
-func Find(n Node, find func(Node) interface{}) interface{} {
+// When Find finds a node x such that find(x) is true,
+// Find ends the traversal and returns true immediately.
+// Otherwise Find returns false after completing the entire traversal.
+func Find(n Node, find func(Node) bool) bool {
        if n == nil {
-               return nil
+               return false
        }
-       var found interface{}
        var do func(Node) error
        do = func(x Node) error {
-               if v := find(x); v != nil {
-                       found = v
+               if find(x) {
                        return stop
                }
                return DoChildren(x, do)
        }
-       do(n)
-       return found
+       return do(n) == stop
 }
 
-// FindList calls Find(x, ok) for each node x in the list, in order.
-// If any call find(x) returns a non-nil result, FindList stops and
+// FindList calls Find(x, find) for each node x in the list, in order.
+// If any call Find(x, find) returns true, FindList stops and
 // returns that result, skipping the remainder of the list.
-// Otherwise FindList returns nil.
-func FindList(list Nodes, find func(Node) interface{}) interface{} {
+// Otherwise FindList returns false.
+func FindList(list Nodes, find func(Node) bool) bool {
        for _, x := range list.Slice() {
-               if v := Find(x, find); v != nil {
-                       return v
+               if Find(x, find) {
+                       return true
                }
        }
-       return nil
+       return false
 }
 
 // EditChildren edits the child nodes of n, replacing each child x with edit(x).