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: inconsistent output when building with gcflag -json, "inlineCall" missing #40585

Closed
leitzler opened this issue Aug 5, 2020 · 3 comments

Comments

@leitzler
Copy link
Contributor

@leitzler leitzler commented Aug 5, 2020

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

$ go version
go version go1.14.6 linux/amd64
$ go1.15rc1 version
go version go1.15rc1 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

What did you do?

$ cd $(mktemp -d)
$ txtar -x <<EOF
-- go.mod --
module foo

go 1.14
-- main.go --
package main

func main() {
        fn()
}

func fn() {}
EOF

$ go1.15rc1 build -gcflags="-m -m"
# foo
./main.go:7:6: can inline fn with cost 0 as: func() {  }
./main.go:3:6: can inline main with cost 2 as: func() { fn() }
./main.go:4:11: inlining call to fn func() {  }

$ go1.15rc1 build -gcflags=-json=0,$(pwd)/1.15rc1 && cat 1.15rc1/main/main.json
{"version":0,"package":"main","goos":"linux","goarch":"amd64","gc_version":"go1.15rc1","file":"/tmp/tmp.N0iBtxTK0r/main.go"}
{"range":{"start":{"line":3,"character":6},"end":{"line":3,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 2"}
{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 0"}

What did you expect to see?

I expect to see the same amount of entries when running with -m -m and -json. In this example a json entry for ./main.go:4:11: inlining call to fn func() { }.

For reference, go1.14.6 includes all three entries:

$ go build -gcflags="-m -m"
# foo
./main.go:7:6: can inline fn as: func() {  }
./main.go:3:6: can inline main as: func() { fn() }
./main.go:4:11: inlining call to fn func() {  }

$ go build -gcflags=-json=0,$(pwd)/1.14.6 && cat 1.14.6/main/main.json
{"version":0,"package":"main","goos":"linux","goarch":"amd64","gc_version":"go1.14.6","file":"/tmp/tmp.N0iBtxTK0r/main.go"}
{"range":{"start":{"line":3,"character":6},"end":{"line":3,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 2"}
{"range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}},"severity":3,"code":"inlineCall","source":"go compiler","message":"main.fn"}
{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 0"}

What did you see instead?

main.go:4:11 entry missing.

@jayconrod jayconrod changed the title cmd/go: inconsistent output when building with gcflag -json, "inlineCall" missing cmd/compile: inconsistent output when building with gcflag -json, "inlineCall" missing Aug 5, 2020
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Aug 5, 2020

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Aug 5, 2020

Working as intended. https://go-review.googlesource.com/c/go/+/223298

After slogging through logs, and getting confused about what was good and what was bad, I decided that the information reported should be (where possible) consistently bad news -- thus, failure to inline is what is reported, instead of inlining.

If this is a problem, we need to coordinate with other users and figure out what people would actually like to see, etc. There's a plan for changing this gracefully, I didn't realize that anyone was using this until literally just last week, so I didn't bother.
But detailed reproduction of "-m -m" behavior in an IDE is not a goal; what I want to do is what's best for IDEs, and I decided that if the is-this-good-is-this-bad mental-gear-grinding was bad for me, the guy who wrote it (and who write several of the compiler diagnostics) then it was likely to be a bad experience for O(everyone).

@leitzler
Copy link
Contributor Author

@leitzler leitzler commented Aug 5, 2020

Thanks for the context, reporting bad news only sounds like a very good plan.

It isn't a problem for me, I just noticed that -m -m and -json didn't have the same number of entries any more in 1.15.
I'm adding the (experimental) gopls feature "Toggle gc annotation details" that uses -json to govim so anything that improves -json is good since that is what is shown to the user in vim.

Happy to close this one if -json isn't intended to have the same content as -m -m.

@leitzler leitzler closed this Aug 5, 2020
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
4 participants
You can’t perform that action at this time.