Skip to content

Commit 945a754

Browse files
pjweinbgoadonovan
authored andcommitted
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 <[email protected]> Reviewed-by: Peter Weinberger <[email protected]>
1 parent e1a6261 commit 945a754

File tree

1 file changed

+20
-17
lines changed

1 file changed

+20
-17
lines changed

gopls/internal/golang/hover.go

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,22 +1096,16 @@ func formatDoc(h *hoverJSON, options *settings.Options) string {
10961096
// TODO(rfindley): this function has tricky semantics, and may be worth unit
10971097
// testing and/or refactoring.
10981098
func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spec, field *ast.Field) {
1099-
// panic(found{}) breaks off the traversal and
1100-
// causes the function to return normally.
1101-
type found struct{}
1102-
defer func() {
1103-
switch x := recover().(type) {
1104-
case nil:
1105-
case found:
1106-
default:
1107-
panic(x)
1108-
}
1109-
}()
1099+
found := false
11101100

11111101
// Visit the files in search of the node at pos.
11121102
stack := make([]ast.Node, 0, 20)
1103+
11131104
// Allocate the closure once, outside the loop.
11141105
f := func(n ast.Node) bool {
1106+
if found {
1107+
return false
1108+
}
11151109
if n != nil {
11161110
stack = append(stack, n) // push
11171111
} else {
@@ -1144,7 +1138,8 @@ func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spe
11441138
if id.Pos() == pos {
11451139
field = n
11461140
findEnclosingDeclAndSpec()
1147-
panic(found{})
1141+
found = true
1142+
return false
11481143
}
11491144
}
11501145

@@ -1153,7 +1148,8 @@ func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spe
11531148
if n.Pos() == pos {
11541149
field = n
11551150
findEnclosingDeclAndSpec()
1156-
panic(found{})
1151+
found = true
1152+
return false
11571153
}
11581154

11591155
// Also check "X" in "...X". This makes it easy to format variadic
@@ -1164,13 +1160,15 @@ func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spe
11641160
if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil && ell.Elt.Pos() == pos {
11651161
field = n
11661162
findEnclosingDeclAndSpec()
1167-
panic(found{})
1163+
found = true
1164+
return false
11681165
}
11691166

11701167
case *ast.FuncDecl:
11711168
if n.Name.Pos() == pos {
11721169
decl = n
1173-
panic(found{})
1170+
found = true
1171+
return false
11741172
}
11751173

11761174
case *ast.GenDecl:
@@ -1180,14 +1178,16 @@ func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spe
11801178
if s.Name.Pos() == pos {
11811179
decl = n
11821180
spec = s
1183-
panic(found{})
1181+
found = true
1182+
return false
11841183
}
11851184
case *ast.ValueSpec:
11861185
for _, id := range s.Names {
11871186
if id.Pos() == pos {
11881187
decl = n
11891188
spec = s
1190-
panic(found{})
1189+
found = true
1190+
return false
11911191
}
11921192
}
11931193
}
@@ -1197,6 +1197,9 @@ func findDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spe
11971197
}
11981198
for _, file := range files {
11991199
ast.Inspect(file, f)
1200+
if found {
1201+
return decl, spec, field
1202+
}
12001203
}
12011204

12021205
return nil, nil, nil

0 commit comments

Comments
 (0)