Skip to content
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

x/tools/gopls: "type checker error %q outside its Fset" bug reported by telemetry #66766

Open
adonovan opened this issue Apr 10, 2024 · 8 comments
Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Apr 10, 2024

#!stacks
"bug.Reportf" &&
  ("typeErrorsToDiagnostics.func1:+28" ||
  "typeErrorsToDiagnostics.func1:+34" ||
  "typeErrorsToDiagnostics.func1:+35" ||
  "typeErrorsToDiagnostics.func1:+37")

This stack _5BDSg was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics.func1:+28
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics:+119
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage:+127
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage:+70
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.15.0-pre.4 go1.21.5 darwin/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

Dups: T_TswA ypJFHg

Closely related to:

@adonovan adonovan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. gopls/telemetry-wins labels Apr 10, 2024
@gopherbot gopherbot added this to the Unreleased milestone Apr 10, 2024
@adonovan adonovan modified the milestones: Unreleased, gopls/v0.16.0 Apr 11, 2024
@adonovan
Copy link
Member Author

This stack T_TswA was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics.func1:+28
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics:+122
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage:+127
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage:+70
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.15.3 go1.22.1 windows/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

@adonovan
Copy link
Member Author

Found this in passing while looking at the debug server:

gopls/internal/cache/check.go:1890: internal error: type checker error "-: cannot use _ as value or type" outside its Fset

goroutine 73634 [running]:
runtime/debug.Stack()
	/Users/adonovan/w/goroot/src/runtime/debug/stack.go:26 +0x64
golang.org/x/tools/gopls/internal/util/bug.report({0x1400da1e5a0, 0x56})
	/Users/adonovan/w/xtools/gopls/internal/util/bug/bug.go:91 +0xd4
golang.org/x/tools/gopls/internal/util/bug.Reportf({0x1037db194?, 0x1401bf536d8?}, {0x1401bf53748?, 0x1d?, 0x103affe00?})
	/Users/adonovan/w/xtools/gopls/internal/util/bug/bug.go:54 +0x28
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics.func1({0x1401bf538b0, 0x1, 0x12b862290?})
	/Users/adonovan/w/xtools/gopls/internal/cache/check.go:1890 +0xaec
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics(0x0?, {0x14014711600?, 0x1400c27cf50?, 0x41?}, {0x10377161b?, 0x5?}, 0xa0?, 0x54?)
	/Users/adonovan/w/xtools/gopls/internal/cache/check.go:2004 +0xfc
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage(0x1400b568de0, {0x103c4e168, 0x14014229620}, 0x14013409c20)
	/Users/adonovan/w/xtools/gopls/internal/cache/check.go:1592 +0xae8
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage(0x1400b568de0, {0x103c4e168, 0x14014229620}, 0x0, {0x14000e72ba0, 0x56})
	/Users/adonovan/w/xtools/gopls/internal/cache/check.go:567 +0x534
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2()
	/Users/adonovan/w/xtools/gopls/internal/cache/check.go:418 +0x34
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/Users/adonovan/go/pkg/mod/golang.org/x/sync@v0.7.0/errgroup/errgroup.go:78 +0x58
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 71449
	/Users/adonovan/go/pkg/mod/golang.org/x/sync@v0.7.0/errgroup/errgroup.go:75 +0x98

Will try to repro.

@adonovan
Copy link
Member Author

No luck so far. The crash indicates a referring ast.Ident{Name: "_"} with a Pos that is invalid, either because it is 0 or out of the FileSet range. I don't see see how the parser can create one; its conceivable that fixAST could: for example, fixInitStmt tries to resolve if i := 0 at EOF by parsing the text i := 0 (a BadExpr) as a statement, but it does so in a throwaway FileSet, so the resulting positions are garbage. They are likely small values, which have a high chance of being valid positions in the first file of an ordinary FileSet, but the specific FileSet constructed by gopls' type checking is a synthetic one that cherry picks the files only of the current type-checking batch, so it may have a gap at the bottom.

@adonovan
Copy link
Member Author

This stack T_TswA was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics.func1:+28
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics:+122
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage:+127
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage:+70
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.15.3 go1.22.1 windows/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

@adonovan
Copy link
Member Author

This stack ypJFHg was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics.func1:+34
golang.org/x/tools/gopls/internal/cache.typeErrorsToDiagnostics:+153
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage:+128
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage:+70
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.16.0-pre.1 go1.22.0 linux/amd64 vscode (1)
golang.org/x/tools/gopls@v0.16.0-pre.2 go1.22.4 linux/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/621876 mentions this issue: gopls/internal/cache: refine bug reports

gopherbot pushed a commit to golang/tools that referenced this issue Oct 22, 2024
Refine a few bug reports related to incorrectly positioned type checker
errors, by differentiating the 'fixed file' case from the case with no
fixed files.

Since we haven't made progress on these bugs, we need more information
to guide our debugging.

If the bugs only occur in the presence of AST fixes, we can probably
downgrade them to not be a bug: if we're fixing the AST, we won't
display type checker errors anyway. Nevertheless, leave the bug reports
for now, so that we can collect data.

For golang/go#65960
For golang/go#66765
For golang/go#66766

Change-Id: I2060c897d249cdd5cc3e7bb183d3f563987bec57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/621876
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
@adonovan
Copy link
Member Author

This stack RQEPow was reported by telemetry:

golang.org/x/tools/gopls@v0.17.0-pre.2 go1.23.3 linux/amd64 vscode (1)

This stack rXJHRw was reported by telemetry:

golang.org/x/tools/gopls@v0.17.0-pre.1 go1.23.3 darwin/arm64 vscode (1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants