Skip to content
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

cmd/compile: what exactly should -L flag do? #36988

Open
jkassis opened this issue Feb 2, 2020 · 8 comments
Open

cmd/compile: what exactly should -L flag do? #36988

jkassis opened this issue Feb 2, 2020 · 8 comments

Comments

@jkassis
Copy link

@jkassis jkassis commented Feb 2, 2020

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

[I] xxxxx@Jeremys-MBP ~/C/ledgie> go version
go version go1.13.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

macos 10.15.2

go env Output
[I] jkassis@Jeremys-MBP ~/C/ledgie> go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jkassis/Library/Caches/go-build"
GOENV="/Users/jkassis/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jkassis/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jkassis/Code/ledgie/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mm/7l9_l1sn39n2kc_jpb8t0tnh0000gn/T/go-build372119512=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

[I] xxxxxx@Jeremys-MBP ~/C/ledgie> go build -gcflags="-L"  dockie/main.go
# github.com/jkassis/xxxxx.net/dockie/lib
dockie/lib/schema.go:112:1: missing return at end of function
dockie/lib/service.go:419:35: serviceDoc.V.String undefined (type *ServiceDocV has no field or method String)

What did you expect to see?

full paths in the errors list

What did you see instead?

partial paths

@ianlancetaylor ianlancetaylor changed the title compiler -L switch does not work cmd/compile: compiler -L switch does not work Feb 3, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Feb 3, 2020
@jkassis
Copy link
Author

@jkassis jkassis commented Feb 4, 2020

thanks for looking at this... it's significant because IDEs like VSCode cannot click through to the filename listed in the terminal if it is not rooted in the workspace.

@jkassis
Copy link
Author

@jkassis jkassis commented Mar 13, 2020

should be an easy one i would think.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented May 25, 2020

Thank you for the report @jkassis and welcome to the Go project!
Sorry that we didn’t look at this for Go1.15, but I shall put it on the backlog and assign to myself for perhaps Go1.16.

@odeke-em odeke-em modified the milestones: Go1.15, Go1.16 May 25, 2020
@odeke-em odeke-em self-assigned this May 26, 2020
@aclements
Copy link
Member

@aclements aclements commented Dec 1, 2020

I tried this back to 1.10 and as far as I can tell, -L has never worked. This was introduced (in its current incarnation) in CL 77090 by @griesemer . @griesemer , can you comment?

@griesemer griesemer self-assigned this Dec 3, 2020
@griesemer
Copy link
Contributor

@griesemer griesemer commented Dec 3, 2020

@aclements Will investigate.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Dec 8, 2020

Note that CL 77090 explicitly states that -L prints absolute positions for positions following line directives. Given

package main

var _ = int

//line foo.go:42
var x = y

func /*line bar.go:7:1*/ init(int){}

(full path $HOME/tmp/main.go) I get the following outputs when invoking the compiler in a variety of combinations:

$ go tool compile main.go
main.go:3:9: type int is not an expression
foo.go:42: undefined: y
bar.go:7:2: func init must have no arguments and no return values

$ go tool compile -L main.go
main.go:3:9: type int is not an expression
foo.go:42[main.go:6:9]: undefined: y
bar.go:7:2[main.go:8:26]: func init must have no arguments and no return values

$ go build main.go
# command-line-arguments
./main.go:3:9: type int is not an expression
foo.go:42: undefined: y
bar.go:7:2: func init must have no arguments and no return values

$ go build -gcflags=-L main.go
# command-line-arguments
./main.go:3:9: type int is not an expression
foo.go:42[/Users/gri/tmp/main.go:6:9]: undefined: y
bar.go:7:2[/Users/gri/tmp/main.go:8:26]: func init must have no arguments and no return values

Thus, -L does work as defined, but perhaps not as expected.

It shouldn't be difficult to make -L work for all positions if that is what this issue is asking for.

@odeke-em odeke-em changed the title cmd/compile: compiler -L switch does not work cmd/compile: compiler -L switch does not work properly for all positions Dec 8, 2020
@odeke-em odeke-em modified the milestones: Go1.16, Go1.17 Dec 8, 2020
@griesemer griesemer changed the title cmd/compile: compiler -L switch does not work properly for all positions cmd/compile: what exactly should -L flag do? Dec 8, 2020
@griesemer
Copy link
Contributor

@griesemer griesemer commented Dec 8, 2020

Since -L "doesn't work" as expected, it is unlikely that there is widespread use, so we have a chance to correct the current behavior.

We have a variety of options for reporting error positions: file paths can be relative (to cwd) or absolute. And positions can be relative to the most recent //line directive (or start of file), or absolute (pointing to actual file containing the source that's being compiled).

We can abbreviate these 4 possible combinations with 4 letters: r and a stand for relative vs absolute positions using relative paths, and R and A stand for the respective positions r and a but using absolute paths. Note though that it's not clear what R means: The filename is simply taken from the //line directive, and making it "absolute" (say, if it doesn't start with a /) is not obviously correct. So we really only have 3 meaningful formats: r, a, and A.

Currently, without the -L flag, errors use the r format:

r: message

With the -L flag, errors use one if these formats:

r: message       // example: main.go:3:9: type int is not an expression
r[A]: message    // example: foo.go:42[/Users/gri/tmp/main.go:6:9]: undefined: y

where the r[A] version is only used if r != a, that is, if the position is after a //line directive.

There are many possible combinations (r, r[a], r[A], a, a[r], a[A], etc.), not all of which are meaningful.

I suspect (from the original comment in this issue) that the expectation for -L is that it simply switches from relative to absolute filenames. Furthermore, if we have a relative position, we probably should always provide the absolute position in []. Thus, the suggestion is to change the behavior as follows:

  1. If relative and absolute position are the same (r == a), i.e., if the file position is not dependent on a //line directive, the format is:
a: message

if there's no -L flag; or

A: message

if -L is given.

  1. If relative and absolute positions are different, i.e., if the file position is dependent on a //line directive, the format is:
r[a]: message

if there's no -L flag; or

r[A]

if -L is given.

Comments?

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 24, 2021

This issue is currently labeled as early-in-cycle for Go 1.17.
That time is now, so a friendly reminder to look at it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants