-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
proposal: x/tools/go/ast/inspector: add Cursor, to enable partial and multi-level traversals #70859
Comments
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
Change https://go.dev/cl/636156 mentions this issue: |
Change https://go.dev/cl/636656 mentions this issue: |
Summarizing discussion from the CL: we're a little concerned about
We are going to use the |
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]>
Background: The inspector package enables efficient repeated traversal over the list of ast.Files that make up a package. However, unlike ast.Inspect, it provides no way to inspect just some subtree, which is a common and recurring need. In particular, it is common to need a two-level traversal, in which one first inspects to find each node of type A, then conditionally visits some subtrees of it looking for nodes of type B.
This need could be addressed if the inspector could iterate over a sequence of abstract cursor positions from which the actual ast.Node could be queried. A cursor can then be used as the basis for a new traversal of the subtree rooted at that node, with a different type filter.
As a bonus, the WithStack operation would not be needed because the stack can be derived relatively efficiently from the cursor position by walking back from the current point, skipping from pop to push nodes.
Unfortunately the inspector as defined is stateless, so we can't simply add a
Current() Cursor
method to it, nor sneak a Cursor among the arguments to the existing callback functions. Cursor would need all the same visitation methods as Inspector, and Inspector's methods would conceptually delegate to the "root" Cursor. However, we can do better than simply duplicating the old API.Proposal: We propose to add the Cursor type and related methods:
Example usage:
See https://go.dev/cl/636656 for the implementation.
@findleyr @timothy-king @griesemer
The text was updated successfully, but these errors were encountered: