gopls/internal/golang: remove a use of panic for flow control
FindDeclAndField in hover.go used panic() and recover() for flow control.
This CL replaces that with tests for early successful completion. The new
code is somewhat fiddlier.
When running all the gopls tests, the old code returned 32,339 non-nil values.
The new code returns exactly the same results in all these cases.
The new code is generally faster than the old code. As they use wall-clock
times, the timings are somewhat noisy, but the median and 99th percentiles
were 1520ns, 12070ns for the new code, and 7870ns, 26500ns for the old.
For 270 of the 32,339 calls, the old code was faster.
The stack is preallocated to capacity 20. The 99th percentile of stack
size is 80, which is only 2 reallocations. The large stacks appear to
happend looking in the go source tree while processing deep completions
(This change was written by pjw in https://go.dev/cl/445337 but it
went stale.)
Change-Id: If23c756d0d671b70ad6286d5e0487c78ed3eb277
Reviewed-on: https://go-review.googlesource.com/c/tools/+/563935
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go
index ef48601..7e613f7 100644
--- a/gopls/internal/golang/hover.go
+++ b/gopls/internal/golang/hover.go
@@ -1096,22 +1096,16 @@
// TODO(rfindley): this function has tricky semantics, and may be worth unit
// testing and/or refactoring.
func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spec, field *ast.Field) {
- // panic(found{}) breaks off the traversal and
- // causes the function to return normally.
- type found struct{}
- defer func() {
- switch x := recover().(type) {
- case nil:
- case found:
- default:
- panic(x)
- }
- }()
+ found := false
// Visit the files in search of the node at pos.
stack := make([]ast.Node, 0, 20)
+
// Allocate the closure once, outside the loop.
f := func(n ast.Node) bool {
+ if found {
+ return false
+ }
if n != nil {
stack = append(stack, n) // push
} else {
@@ -1144,7 +1138,8 @@
if id.Pos() == pos {
field = n
findEnclosingDeclAndSpec()
- panic(found{})
+ found = true
+ return false
}
}
@@ -1153,7 +1148,8 @@
if n.Pos() == pos {
field = n
findEnclosingDeclAndSpec()
- panic(found{})
+ found = true
+ return false
}
// Also check "X" in "...X". This makes it easy to format variadic
@@ -1164,13 +1160,15 @@
if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil && ell.Elt.Pos() == pos {
field = n
findEnclosingDeclAndSpec()
- panic(found{})
+ found = true
+ return false
}
case *ast.FuncDecl:
if n.Name.Pos() == pos {
decl = n
- panic(found{})
+ found = true
+ return false
}
case *ast.GenDecl:
@@ -1180,14 +1178,16 @@
if s.Name.Pos() == pos {
decl = n
spec = s
- panic(found{})
+ found = true
+ return false
}
case *ast.ValueSpec:
for _, id := range s.Names {
if id.Pos() == pos {
decl = n
spec = s
- panic(found{})
+ found = true
+ return false
}
}
}
@@ -1197,6 +1197,9 @@
}
for _, file := range files {
ast.Inspect(file, f)
+ if found {
+ return decl, spec, field
+ }
}
return nil, nil, nil
