Skip to content

Commit

Permalink
internal/lsp: fix crash completing recursive pointer types
Browse files Browse the repository at this point in the history
We were recursing infinitely evaluating objects of recursive pointer
types such as "type foo *foo". Now we track named pointer types we
have already seen to avoid trying to dereference such objects forever.
I lazily initialized the "seen" map to avoid the allocation in the
normal case when you aren't dealing with named pointer types.

Fixes golang/go#37104.

Change-Id: I5f294cfc5a641e7b5fd24e1d9dc55520726ea560
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218579
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
  • Loading branch information
muirdm authored and stamblerre committed Feb 7, 2020
1 parent 6dcdf1d commit 61798d6
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 8 deletions.
24 changes: 19 additions & 5 deletions internal/lsp/source/completion.go
Expand Up @@ -324,7 +324,7 @@ func (c *completer) found(cand candidate) {
return
}

if c.matchingCandidate(&cand) {
if c.matchingCandidate(&cand, nil) {
cand.score *= highScore
} else if isTypeName(obj) {
// If obj is a *types.TypeName that didn't otherwise match, check
Expand Down Expand Up @@ -1737,8 +1737,9 @@ func (c *completer) fakeObj(T types.Type) *types.Var {
}

// matchingCandidate reports whether a candidate matches our type
// inferences.
func (c *completer) matchingCandidate(cand *candidate) bool {
// inferences. seen is used to detect recursive types in certain cases
// and should be set to nil when calling matchingCandidate.
func (c *completer) matchingCandidate(cand *candidate, seen map[types.Type]struct{}) bool {
if isTypeName(cand.obj) {
return c.matchingTypeName(cand)
} else if c.wantTypeName() {
Expand Down Expand Up @@ -1788,15 +1789,28 @@ func (c *completer) matchingCandidate(cand *candidate) bool {

// Check if dereferencing cand would match our type inference.
if ptr, ok := cand.obj.Type().Underlying().(*types.Pointer); ok {
if c.matchingCandidate(&candidate{obj: c.fakeObj(ptr.Elem())}) {
// Notice if we have already encountered this pointer type before.
_, saw := seen[cand.obj.Type()]

if _, named := cand.obj.Type().(*types.Named); named {
// Lazily allocate "seen" since it isn't used normally.
if seen == nil {
seen = make(map[types.Type]struct{})
}

// Track named pointer types we have seen to detect cycles.
seen[cand.obj.Type()] = struct{}{}
}

if !saw && c.matchingCandidate(&candidate{obj: c.fakeObj(ptr.Elem())}, seen) {
// Mark the candidate so we know to prepend "*" when formatting.
cand.dereference = true
return true
}
}

// Check if cand is addressable and a pointer to cand matches our type inference.
if cand.addressable && c.matchingCandidate(&candidate{obj: c.fakeObj(types.NewPointer(candType))}) {
if cand.addressable && c.matchingCandidate(&candidate{obj: c.fakeObj(types.NewPointer(candType))}, seen) {
// Mark the candidate so we know to prepend "&" when formatting.
cand.takeAddress = true
return true
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/source/completion_literal.go
Expand Up @@ -75,7 +75,7 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) {
cand.addressable = true
}

if !c.matchingCandidate(&cand) {
if !c.matchingCandidate(&cand, nil) {
return
}

Expand Down
11 changes: 11 additions & 0 deletions internal/lsp/testdata/lsp/primarymod/address/address.go
Expand Up @@ -36,6 +36,17 @@ func _() {
var _ int = _ //@rank("_ //", addrCPtr, addrA),snippet("_ //", addrCPtr, "*c", "*c")

wantsVariadic() //@rank(")", addrCPtr, addrA),snippet(")", addrCPtr, "*c", "*c")

type namedPtr *int
var np namedPtr
*np //@item(addrNamedPtr, "*np", "namedPtr", "var")
var _ int = _ //@rank("_ //", addrNamedPtr, addrA)

// don't get tripped up by recursive pointer type
type dontMessUp *dontMessUp
var dmu *dontMessUp //@item(addrDMU, "dmu", "*dontMessUp", "var")

var _ int = dmu //@complete(" //", addrDMU)
}

func (f foo) ptr() *foo { return &f }
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/testdata/lsp/summary.txt.golden
@@ -1,10 +1,10 @@
-- summary --
CompletionsCount = 226
CompletionsCount = 227
CompletionSnippetCount = 66
UnimportedCompletionsCount = 9
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
RankedCompletionsCount = 83
RankedCompletionsCount = 84
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 38
FoldingRangesCount = 2
Expand Down

0 comments on commit 61798d6

Please sign in to comment.