golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.25k stars 17.57k forks source link

cmd/compile/internal/types2: inconsistent source paths printed in error message #68292

Closed jfrech closed 1 month ago

jfrech commented 3 months ago

Go version

go version go1.22.3 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2953486244=/tmp/go-build -gno-record-gcc-switches'

What did you do?

$ cd /tmp
$ cat leaks.go
package main

func f[S any, T any](T) {}

func g() {
        f(0)
}

func main() {}
$ go run leaks.go
# command-line-arguments
./leaks.go:6:3: cannot infer S (/tmp/leaks.go:3:8)

What did you see happen?

In its error message, the compiler leaks the absolute path of "./leaks.go".

What did you expect to see?

I would have expected the error's source's source path to be presented relative to the current working directory (or relative to the current module's root). Such is the case on the playground: https://go.dev/play/p/890kN4mzITD

gabyhelp commented 3 months ago

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

gopherbot commented 1 month ago

Change https://go.dev/cl/603097 mentions this issue: cmd/compile/internal/types2: change inference error message

timothy-king commented 1 month ago

Thank you for the report.

What is happening is that types was formatting an error message using [link]:

            err.addf(posn, "cannot infer %s (%v)", obj.name, obj.pos)

cmd/go rewrites the output of gc in Shell.reportCmd. Excerpt from internal documentation:

// reportCmd formats the output as "# desc" followed by the given output. The // output is expected to contain references to 'dir', usually the source // directory for the package that has failed to build. reportCmd rewrites // mentions of dir with a relative path to dir when the relative path is // shorter. This is usually more pleasant.

When this happens is decided by replacePrefix. Docs:

// replacePrefix is like strings.ReplaceAll, but only replaces instances of old // that are preceded by ' ', '\t', or appear at the beginning of a line.

This manifests as rewriting the output /tmp/leaks.go:6:3: cannot infer S (/tmp/leaks.go:3:8) to ./leaks.go:6:3: cannot infer S (/tmp/leaks.go:3:8). The first instance begins a line and matches. The second is preceded by a ( so it does not match.

I think we should be consistent with the output here and have a relative path in both locations, e.g. ./leaks.go:6:3: cannot infer S (declared on ./leaks.go:3:8). This is being done for convenience, and we should try to get this consistent on a best effort basis. We can fix these as they are reported for now. I don't think there is a need for a more systematic review yet.

jfrech commented 1 month ago

@timothy-king I was wondering why play.go.dev didn't exhibit the same. Thanks for the writeup.

jfrech commented 1 month ago

Cf. #68737