x/tools/gopls/internal/lsp/source: different hover for same type #47098

katsusan · 2021-07-08T19:21:53Z

What version of Go are you using (`go version`)?

➜  ~ go version
go version go1.16.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
➜  ~ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.5"
GCCGO="/usr/local/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3210144513=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Version of gopls:

➜  ~ gopls version
golang.org/x/tools/gopls v0.7.0
    golang.org/x/tools/gopls@v0.7.0 h1:JQBHW81Gsyim6iDjUwGoPeSpXrSqwen3isPJLfDfaYU=

Editor: VSCode
Reproducing steps:

Open /usr/local/go/src/runtime/panic.go in VSCode.
Hover for _panic in following code.

go/src/runtime/panic.go

Line 871 in 7677616

func reflectcallSave(p *_panic, fn, arg unsafe.Pointer, argsize uint32) {
Hover for _panic in another line.

go/src/runtime/panic.go

Line 916 in 7677616

var p _panic

What did you expect to see?

step 3 give the same output as step 2.

What did you see instead?

step 2 gives:

type _panic struct {
    argp      unsafe.Pointer // pointer to arguments of deferred call run during panic; cannot move - known to liblink
    arg       interface{}    // argument to panic
    link      *_panic        // link to earlier panic
    pc        uintptr        // where to return to in runtime if this panic is bypassed
    sp        unsafe.Pointer // where to return to in runtime if this panic is bypassed
    recovered bool           // whether this panic is over
    aborted   bool           // the panic was aborted
    goexit    bool
}
A _panic holds information about an active panic.

A _panic value must only ever live on the stack.

The argp and link fields are stack pointers, but don't need special handling during stack growth: because they are pointer-typed and _panic values only live on the stack, regular stack pointer adjustment takes care of them.

step 3 gives:

type _panic struct{argp unsafe.Pointer; arg interface{}; link *_panic; pc uintptr; sp unsafe.Pointer; recovered bool; aborted bool; goexit bool}

Appendix

Here is the relevant rpc trace log.

// step 2

[Trace - 02:51:14.391 AM] Sending request 'textDocument/hover - (27)'.
Params: {"textDocument":{"uri":"file:///usr/local/go/src/runtime/panic.go"},"position":{"line":870,"character":27}}

[Trace - 02:51:14.399 AM] Received response 'textDocument/hover - (27)' in 7ms.
Result: {"contents":{"kind":"markdown","value":"```go\ntype _panic struct {\n\targp      unsafe.Pointer // pointer to arguments of deferred call run during panic; cannot move - known to liblink\n\targ       interface{}    // argument to panic\n\tlink      *_panic        // link to earlier panic\n\tpc        uintptr        // where to return to in runtime if this panic is bypassed\n\tsp        unsafe.Pointer // where to return to in runtime if this panic is bypassed\n\trecovered bool           // whether this panic is over\n\taborted   bool           // the panic was aborted\n\tgoexit    bool\n}\n```\n\nA \\_panic holds information about an active panic\\.\n\nA \\_panic value must only ever live on the stack\\.\n\nThe argp and link fields are stack pointers, but don\\'t need special\nhandling during stack growth\\: because they are pointer\\-typed and\n\\_panic values only live on the stack, regular stack pointer\nadjustment takes care of them\\.\n"},"range":{"start":{"line":870,"character":24},"end":{"line":870,"character":30}}}

// step 3

[Trace - 02:52:02.504 AM] Sending request 'textDocument/hover - (29)'.
Params: {"textDocument":{"uri":"file:///usr/local/go/src/runtime/panic.go"},"position":{"line":915,"character":10}}


[Trace - 02:52:02.511 AM] Received response 'textDocument/hover - (29)' in 7ms.
Result: {"contents":{"kind":"markdown","value":"```go\ntype _panic struct{argp unsafe.Pointer; arg interface{}; link *_panic; pc uintptr; sp unsafe.Pointer; recovered bool; aborted bool; goexit bool}\n```"},"range":{"start":{"line":915,"character":7},"end":{"line":915,"character":13}}}

I tried to dig into the difference and found function HoverIdentifier returned different HoverInformation.
But the source hasn't been changed for a long time. Is it feature or I need some extra options for gopls?

https://github.com/golang/tools/blob/55cd4804dfa0984a27cd61589b7f56f937e05545/internal/lsp/source/hover.go#L66-L71

func Hover(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) (*protocol.Hover, error) {
	ident, err := Identifier(ctx, snapshot, fh, position)Rohan Challa, 1 year ago: • internal/lsp: support textDocument/hover for …
	if err != nil {
		return nil, nil
	}
	h, err := HoverIdentifier(ctx, ident)

// step 2

>>> p *h
$1 = {
  Signature = "type _panic struct {\n\targp      unsafe.Pointer // pointer to arguments of deferred call run during panic; cannot move - known to liblink\n\targ       interface{}    // argument to panic\n\tlink      *_pan"...,
  SingleLine = "type _panic struct{argp unsafe.Pointer; arg interface{}; link *_panic; pc uintptr; sp unsafe.Pointer; recovered bool; aborted bool; goexit bool}",
  Synopsis = "A _panic holds information about an active panic.",
  FullDocumentation = "A _panic holds information about an active panic.\n\nA _panic value must only ever live on the stack.\n\nThe argp and link fields are stack pointers, but don't need special\nhandling during stack growth: b"...,
  LinkPath = "",
  LinkAnchor = "",
  importPath = "",
  symbolName = "",
  source = {
    _type = 0xd6cc40,
    data = 0xc011e614a0
  },
  comment = 0xc00534ed20,
  typeName = "_panic",
  isTypeAlias = false
}

// step3

>>> p *h
$2 = {
  Signature = "type _panic struct{argp unsafe.Pointer; arg interface{}; link *_panic; pc uintptr; sp unsafe.Pointer; recovered bool; aborted bool; goexit bool}",
  SingleLine = "type _panic struct{argp unsafe.Pointer; arg interface{}; link *_panic; pc uintptr; sp unsafe.Pointer; recovered bool; aborted bool; goexit bool}",
  Synopsis = "",
  FullDocumentation = "",
  LinkPath = "",
  LinkAnchor = "",
  importPath = "",
  symbolName = "",
  source = {
    _type = 0xe23c00,
    data = 0xc0034e2f50
  },
  comment = 0x0,
  typeName = "",
  isTypeAlias = false
}

The text was updated successfully, but these errors were encountered:

findleyr · 2021-07-09T16:35:35Z

Thank you for the repro.

I believe this is the same as #46158, but I am investigating.

stamblerre · 2021-09-10T17:50:45Z

Duplicate of #46158

katsusan · 2021-09-21T18:47:34Z

Hi @findleyr, thank you for the fix, it seems there is still an edge case not covered.

Another reproduce:

version:

D:\Projects\leetcode>gopls version
golang.org/x/tools/gopls v0.7.2
    golang.org/x/tools/gopls@v0.7.2 h1:kRKKdvA8GOzra8rhSFDClOR7hV/x8v0J0Vm4C/gWq8s=

D:\Projects\leetcode>go version
go version go1.17 windows/amd64

steps:

open $GOROOT/src/runtime/proc.go.

hover for the global variable debug in following code.

go/src/runtime/proc.go

Lines 199 to 202 in 6097ebe

    
           if debug.inittrace != 0 { 
        
           	inittrace.id = getg().goid 
        
           	inittrace.active = true 
        
           }

hover result:

findleyr · 2021-09-21T22:08:39Z

Thanks very much for the repro.

findleyr · 2022-02-11T01:39:04Z

Just looked into this. The debug struct is a real edge case: it is a var with a large struct literal type. For vars we use types.ObjectString to format their signature, which usually looks reasonable but in this case produces a very large type expression. It's hard to imagine doing something else: if we naively used the declaration syntax we'd have to handle all sorts of edge cases, like x := 2 // no type or x := []byte{/* lots of data */}, etc.

So I think the original issue is fixed, and we are unlikely to spend engineering time addressing this particular edge case. Closing so that this issue doesn't stagnate.

katsusan · 2022-02-11T09:19:32Z

Thank you Robert for the investigation, the edge case doesn't miss type information and gopls needs improvement in other more important aspects, so it doesn't deserve if needs too much time.

gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 8, 2021

gopherbot added this to the Unreleased milestone Jul 8, 2021

findleyr added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 9, 2021

findleyr modified the milestones: Unreleased, gopls/v0.7.1 Jul 9, 2021

findleyr self-assigned this Jul 9, 2021

stamblerre modified the milestones: gopls/v0.7.1, gopls/v0.7.2 Jul 26, 2021

stamblerre modified the milestones: gopls/v0.7.2, gopls/on-deck Sep 9, 2021

stamblerre marked this as a duplicate of #46158 Sep 10, 2021

stamblerre closed this as completed Sep 10, 2021

stamblerre modified the milestones: gopls/on-deck, gopls/v0.7.3 Sep 13, 2021

stamblerre reopened this Sep 21, 2021

stamblerre modified the milestones: gopls/v0.7.3, gopls/on-deck Oct 19, 2021

findleyr closed this as completed Feb 11, 2022

rsc unassigned findleyr Jun 23, 2022

findleyr closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2022

golang locked and limited conversation to collaborators Jul 1, 2023

gopherbot added the FrozenDueToAge label Jul 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/gopls/internal/lsp/source: different hover for same type #47098

x/tools/gopls/internal/lsp/source: different hover for same type #47098

katsusan commented Jul 8, 2021

findleyr commented Jul 9, 2021

stamblerre commented Sep 10, 2021

katsusan commented Sep 21, 2021 •

edited

findleyr commented Sep 21, 2021

findleyr commented Feb 11, 2022

katsusan commented Feb 11, 2022

x/tools/gopls/internal/lsp/source: different hover for same type #47098

x/tools/gopls/internal/lsp/source: different hover for same type #47098

Comments

katsusan commented Jul 8, 2021

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

Appendix

findleyr commented Jul 9, 2021

stamblerre commented Sep 10, 2021

katsusan commented Sep 21, 2021 • edited

findleyr commented Sep 21, 2021

findleyr commented Feb 11, 2022

katsusan commented Feb 11, 2022

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

katsusan commented Sep 21, 2021 •

edited