Skip to content

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

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

Closed
jfrech opened this issue Jul 3, 2024 · 5 comments
Closed
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jfrech
Copy link

jfrech commented Jul 3, 2024

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
Copy link

gabyhelp commented Jul 3, 2024

Related Issues

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

@cagedmantis cagedmantis changed the title cmd/go: absolute source path leaked during type inference cmd/compile/internal/types2: absolute source path leaked during type inference Jul 8, 2024
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 8, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 8, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Jul 8, 2024
@griesemer griesemer modified the milestones: Backlog, Go1.24 Jul 24, 2024
@timothy-king timothy-king changed the title cmd/compile/internal/types2: absolute source path leaked during type inference cmd/compile/internal/types2: inconsistent source paths printed in error message Aug 5, 2024
@gopherbot
Copy link
Contributor

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

@timothy-king
Copy link
Contributor

timothy-king commented Aug 5, 2024

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
Copy link
Author

jfrech commented Aug 6, 2024

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

@jfrech
Copy link
Author

jfrech commented Aug 6, 2024

Cf. #68737

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

6 participants