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

proposal: runtime: print columns (not just lines) in panic messages #65060

Open
komuw opened this issue Jan 11, 2024 · 4 comments
Open

proposal: runtime: print columns (not just lines) in panic messages #65060

komuw opened this issue Jan 11, 2024 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal
Milestone

Comments

@komuw
Copy link
Contributor

komuw commented Jan 11, 2024

Proposal Details

background:
Whenever you try to dereference a nil pointer in Go, you get a helpful panic message that includes the line number that the attempted dereference occurred.
Sometimes however the expression where the dereference occurs has multiple access points and it can sometimes be hard to know which of them is the nil pointer.
If you have an expression like;

_ = person.product

and that leads to a panic, it is reasonable to assume that product is the nil pointer.
But in an expression like;

_ = person.product.country.location.Name

each one of person, product, country or location could have been the nil pointer.
It would be helpful if in such circumstances the panic message included the column/offset number as well so that you can know which field(access point) is nil.

Here is a small program that illustrates the issue; https://go.dev/play/p/2dhrd3_SUCs .

I've wanted something like this for a number of years and always felt like maybe it is too hard to implement. I've already encountered the need for it twice already this year; so I thought it wouldn't hurt to ask.

proposal:
Instead of reporting panic error messages like this:

panic: runtime error: invalid memory address or nil pointer dereference
main.main()
	/tmp/main.go:22 +0x1a

We could report them like this:

panic: runtime error: invalid memory address or nil pointer dereference
main.main()
	/tmp/main.go:22:18 +0x1a

Notice the 18. ie, the message would contain filename:line:column. For the definition of column(or byte offset), see; 2a5cf48

This proposal is similar in spirit to #30116 and #10324 which were proposed in the past and implemented. Although I'm not sure how feasible to implement this one would be.

@komuw komuw added the Proposal label Jan 11, 2024
@gopherbot gopherbot added this to the Proposal milestone Jan 11, 2024
@Jorropo
Copy link
Member

Jorropo commented Jan 11, 2024

I guess the cons are that binary size might increase since we couldn't merge lines as much.
I don't know if the .debugloc format even support columns.

Side effects pros, debuggers could step into each evaluation of an expression if we encoded instructions based on columns too.

@bcmills bcmills added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 11, 2024
@randall77
Copy link
Contributor

The compiler has this data (added as part of #10324), so it could in principle encode it into the pcln tables in the binary.
I don't think we have a good feel for how expensive this would be. My feeling is it might be pretty expensive, but there's no real way to know without a prototype.

@cherrymui
Copy link
Member

Somewhat related: We may want to add column number metadata to the binary so we can get more accurate profiles, for PGO. This issue and that share the same (or similar) binary size cost.

@AlexanderYastrebov
Copy link
Contributor

FWIW java added null receiver name in the error message several years ago https://openjdk.org/jeps/358

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. Proposal
Projects
Status: No status
Status: Incoming
Development

No branches or pull requests

7 participants