x/tools/gopls: definition request fails on method with receiver type of form C.T (cgo)

ahamza360 commented 1 year ago

gopls version

Build info

golang.org/x/tools/gopls v0.11.0 golang.org/x/tools/gopls@v0.11.0 h1:/nvKHdTtePQmrv9XN3gIUN9MOdUrKzO/dcqgbG6x8EY= github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= golang.org/x/exp@v0.0.0-20221031165847-c99f073a8326 h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE= golang.org/x/exp/typeparams@v0.0.0-20221031165847-c99f073a8326 h1:fl8k2zg28yA23264d82M4dp+YlJ3ngDcpuB1bewkQi4= golang.org/x/mod@v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= golang.org/x/sync@v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sys@v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= golang.org/x/text@v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= golang.org/x/tools@v0.3.1-0.20221213193459-ca17b2c27ca8 h1:7/HkGkN/2ktghBCSRRgp31wAww4syfsW52tj7yirjWk= golang.org/x/vuln@v0.0.0-20221109205719-3af8368ee4fe h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww= honnef.co/go/tools@v0.3.3 h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA= mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM= mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc= go: go1.20.3

go env

GO111MODULE="auto" GOARCH="arm64" GOBIN="" GOCACHE="/Users/hamza/Library/Caches/go-build" GOENV="/Users/hamza/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="arm64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/hamza/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/hamza/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64" GOVCS="" GOVERSION="go1.20.3" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/hamza/workspace/Go/test2/go.mod" GOWORK="" CGO_CFLAGS="-O2 -g" CGO_CPPFLAGS="" CGO_CXXFLAGS="-O2 -g" CGO_FFLAGS="-O2 -g" CGO_LDFLAGS="-O2 -g" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/2q/n7_883yd56v1jnstwt643pw00000gn/T/go-build263698593=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have 2 similar versions of the same program. Both work as expected while running the code from terminal. However, version 2 gives the error in vscode even though it still compiles.

Version 1:

package main

/*
#include <stdlib.h>

typedef struct Person {
    char* name;
    int age;
} Person;
*/
import "C"
import (
    "fmt"
    "unsafe"
)

type Person C.Person

func (p Person) PrintName() {
    name := C.GoString(p.name)
    fmt.Println(name)
}

func main() {
    name := C.CString("John")
    defer C.free(unsafe.Pointer(name))

    age := C.int(30)

    cPerson := C.Person{
        name: name,
        age:  age,
    }

    person := Person(cPerson)
    person.PrintName()
}

Version 2:

package main

/*
#include <stdlib.h>

typedef struct Person {
    char* name;
    int age;
} Person;
*/
import "C"
import (
    "fmt"
    "unsafe"
)

func (p *C.Person) PrintName() {
    name := C.GoString(p.name)
    fmt.Println(name)
}

func main() {
    name := C.CString("John")
    defer C.free(unsafe.Pointer(name))

    age := C.int(30)

    cPerson := C.Person{
        name: name,
        age:  age,
    }

    person := C.Person(cPerson)
    person.PrintName()
}

What did you expect to see?

Version 2 of the code should have worked the same way as version 1 in VSCode

What did you see instead?

For version 2, at person.PrintName() it shows

person.PrintName undefined (type _Ctype_struct_Person has no field or method PrintName)compilerMissingFieldOrMethod

Moreover, when PrintName is clicked, it doesn't take to the function.

This is a simplified example of the issue that we are facing in our code base which is not very easy to refactor.

Editor and settings

VS Code Settings:

"go.lintTool": "golangci-lint",
  "go.lintFlags": ["--fast"],
  "[jsonc]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  },
  "go.toolsManagement.autoUpdate": true,
  "go.gopath": "",
  "go.lintOnSave": "file",
  "go.gocodeAutoBuild": true,
  "go.useLanguageServer": true,
  "go.buildOnSave": true,
  "[json]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  }

Logs

[Error - 02:20:39] Request textDocument/definition failed. Message: no object found for ident PrintName Code: 0

adonovan commented 1 year ago

Thanks for the very clear bug report. I confirm the error in program 2 (but not 1) using VSCode and gopls@v0.11.0 and gopls@master.

The fundamental problem is that it probably shouldn't be legal to declare a method whose receiver is *C.T since ostensibly that's a type from another package. The Go compiler allows it, because the current implementation of cgo does a source transformation step that replaces C.T with a generated type in the current package, something like CT_mumble. But really the tool should disallow it. By constrast, gopls is based on the go/types package, which doesn't do a source transformation and interprets the C.T reference directly. It therefore complains about the method declaration.

I argue that go/types is correct and the go command (cgo) has a bug, but that question hasn't been resolved yet. See https://github.com/golang/go/issues/57926

In any case, we have the unfortunate situation of code (like both of your example programs) existing in the wild that depends on the buggy behavior. It is probably not straightforward for us to change go/types to match the compiler behavior (nor desirable, if we all agree it's a bug).

If you have the power, I recommend that you delete any method declarations whose receiver type is of the form C.T.

adonovan commented 1 year ago

Now that https://github.com/golang/go/issues/57926 has been resolved by making the cgo tool reject attempts to declare methods on types of the form C.T, I'm going to close this. The error message could perhaps be improved, but I think this will become vanishingly rare in practice.

golang / go