Skip to content

Commit

Permalink
internal/astutil/cursor: Cursor API for inspector
Browse files Browse the repository at this point in the history
This CL defines the Cursor type, which represents a node in the
traversal done by go/ast/inspector (represented by the index
of its push event). From this, we can derive all the operators for
navigating the AST in the four "cardinal directions", similar
to the DOM of a web browser:

- Parent (the immediate parent)
- Stack (all ancestors)
- Children, FirstChild, and LastChild
- PrevSibling and NextSibling

All operations are O(1) except Parent, which is O(n) for a
node in a list of length n.

In addition, all the usual traversals over the inspector can
be offered as traversals within a subtree rooted at a given
node, allowing multi-level traversals with independent
control over type-based node filtering and independent control
over pruning descent into subtrees. We can also provide fast
means to obtain the cursor for a particular node or position.

- Cursor.Inspect(types []ast.Node, f func(c Cursor, push bool) (bool))
- Cursor.Preorder(types ...ast.Node) iter.Seq[Cursor]
- Cursor.FindNode(n ast.Node) (Cursor, bool)
- Cursor.FindPos(start, end token.Pos) (Cursor, bool)

All of these operations are simple to express using inspector's
existing threaded tree representation, and they make it both
simpler and more efficient to express the kinds of queries
that turn up in our refactoring efforts. For example:
"Find all function literals; then find all defer statements
within them; then go to the previous statement to see
if it is a call to Mutex.Lock" etc.

This CL also includes one token usage of Cursor from an
existing analyzer.

The CursorStack benchmark is significantly slower than the
existing WithStack operation, but I suspect the frequency of
calls to Cursor.Stack is actually much rarer in practice than
the benchmark would suggest, because one typically calls Stack
only after a highly selective filtering. When Stack is called
infrequently, the Cursor-based traversal is about 27% faster
while still providing the option to obtain the stack when
needed.

We will evaluate these operators in the coming weeks and
update proposal golang/go#70859 in light of our experience.
In the meantime, we must use linkname hackery to augment
the existing inspector for use in gopls.

Updates golang/go#70859

Change-Id: I1ec8c1a20dc07ad80dad8f0038c0cf3f8f791050
Reviewed-on: https://go-review.googlesource.com/c/tools/+/636656
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
  • Loading branch information
adonovan committed Dec 23, 2024
1 parent ebeac1b commit b75baa0
Show file tree
Hide file tree
Showing 6 changed files with 886 additions and 3 deletions.
4 changes: 4 additions & 0 deletions go/ast/inspector/inspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ package inspector

import (
"go/ast"
_ "unsafe"
)

// An Inspector provides methods for inspecting
Expand All @@ -44,6 +45,9 @@ type Inspector struct {
events []event
}

//go:linkname events
func events(in *Inspector) []event { return in.events }

// New returns an Inspector for the specified syntax trees.
func New(files []*ast.File) *Inspector {
return &Inspector{traverse(files)}
Expand Down
3 changes: 3 additions & 0 deletions go/ast/inspector/typeof.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ package inspector
import (
"go/ast"
"math"

_ "unsafe"
)

const (
Expand Down Expand Up @@ -215,6 +217,7 @@ func typeOf(n ast.Node) uint64 {
return 0
}

//go:linkname maskOf
func maskOf(nodes []ast.Node) uint64 {
if nodes == nil {
return math.MaxUint64 // match all node types
Expand Down
8 changes: 5 additions & 3 deletions gopls/internal/analysis/unusedparams/unusedparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/gopls/internal/util/moreslices"
"golang.org/x/tools/internal/analysisinternal"
"golang.org/x/tools/internal/astutil/cursor"
)

//go:embed doc.go
Expand Down Expand Up @@ -141,7 +142,7 @@ func run(pass *analysis.Pass) (any, error) {
(*ast.FuncDecl)(nil),
(*ast.FuncLit)(nil),
}
inspect.WithStack(filter, func(n ast.Node, push bool, stack []ast.Node) bool {
cursor.Root(inspect).Inspect(filter, func(c cursor.Cursor, push bool) bool {
// (We always return true so that we visit nested FuncLits.)

if !push {
Expand All @@ -153,7 +154,7 @@ func run(pass *analysis.Pass) (any, error) {
ftype *ast.FuncType
body *ast.BlockStmt
)
switch n := n.(type) {
switch n := c.Node().(type) {
case *ast.FuncDecl:
// We can't analyze non-Go functions.
if n.Body == nil {
Expand Down Expand Up @@ -182,7 +183,8 @@ func run(pass *analysis.Pass) (any, error) {
// Find the symbol for the variable (if any)
// to which the FuncLit is bound.
// (We don't bother to allow ParenExprs.)
switch parent := stack[len(stack)-2].(type) {
stack := c.Stack(nil)
switch parent := stack[len(stack)-2].Node().(type) {
case *ast.AssignStmt:
// f = func() {...}
// f := func() {...}
Expand Down
Loading

0 comments on commit b75baa0

Please sign in to comment.