Open nobelsmith opened 3 months ago
CC @adonovan @griesemer
A couple things going on here: (1) we should repair the AST in gopls if it records any positions outside of the file, but also (2) I'm not sure go/types should panic here. Simply using the default version seems acceptable.
Alan points out that this may be related to golang/go#66683.
@nobelsmith can you share anything about the structure of the code you were editing when this occurred?
This error I was doing work on a fiber api, but it was the first time I was trying live server within vs code with another dev. Running live server is the only thing that was out of the ordinary for me otherwise just go installed through brew and working in vscode. @findleyr
There are two problems here:
For example:
if cond
at EOF (sans braces) yields a synthetic BlockStmt with Pos=EOF End=EOF+1; all enclosing nodes have an End of at least EOF+1.
3 package p; func _() { if cond { if cond { if cond
yields three BlockStmts all at EOF.package p; func _() { if
at EOF yields an ExprStmt of a BadExpr, both with Pos=EOF End=EOF, and zero width.package p; var
yields a fake Ident _
whose End is greater than the End of the parent File (EOF).I was hoping we can repair the tree to achieve these invariants: (a) each node's (pos,end) range is a (non-strict subrange) of its parent range, or of (0,size) if the parent is File. (b) all token.Pos fields are within their enclosing Node's (pos,end) range (with an exception for FuncType.Func).
But I'm not so sure it's feasible. A consequence of (a) is that all computed End positions must be more careful not to report the suffix of fake tokens. For example, given if cond
, struct
or var
at EOF, the parse tree reflects these fake tokens: if cond{}
, struct{}
and var _
. In the first two cases, both fake brace tokens are placed at (EOF, EOF), so Pos is strictly out of range; in the third case the blank Ident has range (EOF, EOF+1), so its end is out of range. (It's tempting to change the fake BlockStmt to allow its Lbrace token to be optional, just like Rparen, but then then whole node would have no position at all, so that's a nonstarter.) So I don't see us getting away from the need for exactly one byte beyond EOF (as handled by logic in the safetoken package).
To answer @hyangah questions from the duplicate ticket I opened.
Interesting. @knisbet Is the crash log from the host?
Yes, I was hosting the remote session in vscode insiders and this is the crash/stack that automatically opened on my machine.
Is it possible to test with open source code and post logs?
I'll try it out next week and see. We saw the crash a couple of times in about 2 hours so we'll have to see how lucky we are.
Realistically, any fix for this will involve changes to go/ast, go/parser, and go/types, which are in the pre-go1.23 freeze right now. Pushing this off to gopls 0.17, assuming the tree opens by then.
gopls version: v0.15.2/go1.22.1 gopls flags: update flags: proxy extension version: 0.41.2 environment: Visual Studio Code darwin initialization error: undefined issue timestamp: Fri, 05 Apr 2024 20:02:00 GMT restart history: Thu, 04 Apr 2024 19:08:50 GMT: activation (enabled: true) Fri, 05 Apr 2024 17:57:26 GMT: manual (enabled: true)
ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.
Describe what you observed.
gopls stats -anon
{ "DirStats": { "Files": 805, "TestdataFiles": 0, "GoFiles": 41, "ModFiles": 1, "Dirs": 322 }, "GOARCH": "arm64", "GOOS": "darwin", "GOPACKAGESDRIVER": "", "GOPLSCACHE": "", "GoVersion": "go1.22.1", "GoplsVersion": "v0.15.2", "InitialWorkspaceLoadDuration": "708.152041ms", "MemStats": { "HeapAlloc": 33689896, "HeapInUse": 51576832, "TotalAlloc": 551701312 }, "WorkspaceStats": { "Files": { "Total": 1850, "Largest": 935931, "Errs": 0 }, "Views": [ { "GoCommandVersion": "go1.22.1", "AllPackages": { "Packages": 307, "LargestPackage": 155, "CompiledGoFiles": 1846, "Modules": 42 }, "WorkspacePackages": { "Packages": 16, "LargestPackage": 12, "CompiledGoFiles": 41, "Modules": 1 }, "Diagnostics": 1 } ] } }