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

fmt: prints "<nil>" for %#v when type implements GoStringer #70305

Closed
RaduBerinde opened this issue Nov 12, 2024 · 8 comments
Closed

fmt: prints "<nil>" for %#v when type implements GoStringer #70305

RaduBerinde opened this issue Nov 12, 2024 · 8 comments
Labels
Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. Unfortunate
Milestone

Comments

@RaduBerinde
Copy link
Contributor

RaduBerinde commented Nov 12, 2024

Go version

1.22, 1.23, dev branch

Output of go env in your module/workspace:

N/A

What did you do?

https://go.dev/play/p/2eq4QlnXX9e

What did you see happen?

%#v outputs <nil> which is not correct Go syntax

What did you expect to see?

I expected to get (*time.Time)(nil) instead of <nil>.

It looks like this happens because we have a Time.GoString function with non-pointer receiver; I verified that using the A and B types.

One library that uses %#v and expects it to produce correct Go syntax is github.com/gogo/protobuf

@seankhliao seankhliao changed the title fmt: unexpected result with #v and pointer to type that implements GoString fmt: prints "<nil>" for %#v when type implements GoStringer Nov 12, 2024
@seankhliao
Copy link
Member

I think this is working as intended?

  • The fmt.GoStringer interface is called when the formatting verb is %#v
  • The method set for *time.Time includes that of time.Time
  • But it's a nil pointer dereference panic to dereference the pointer to call the GoString
  • fmt normalizes all nil pointer derefs to <nil>

It's wrong for the library to expect that %#v will not be overwritten by custom GoString implementations.

@RaduBerinde
Copy link
Contributor Author

%#v is documented as "a Go-syntax representation of the value". <nil> is not that. The caller isn't doing anything wrong by passing a nil value.

@RaduBerinde
Copy link
Contributor Author

Can't fmt use reflection to detect when GoString has a non-pointer receiver and the value is a nil pointer?

@seankhliao
Copy link
Member

That's just the general rule, more specifically, lower down:

Except when printed using the verbs %T and %p, special formatting considerations apply for operands that implement certain interfaces. In order of application:
...
3. If the %v verb is used with the # flag (%#v) and the operand implements the GoStringer interface, that will be invoked.

and

Format errors
...
If the panic is caused by a nil receiver to an Error or String method, however, the output is the undecorated string, "".

So <nil> is consistent with the rest of fmt.

@RaduBerinde
Copy link
Contributor Author

It is debatable if the nil receiver wording applies here since the method has a non-pointer receiver. It is the fmt code that chooses to dereferenciate the pointer, it is not the function panicing.

In any case, wording aside, why do you think this outcome is preferable? IMO the fact that by adding a correct GoString method you can break existing usages of nil seems very problematic.

@ianlancetaylor
Copy link
Member

It's not clear that we can change this behavior at this point. It's been this way for many releases. Changing it now seems likely to do more harm than good.

@RaduBerinde
Copy link
Contributor Author

I see. Could we at least call this out specifically in the documentation? Maybe: "If the panic is caused by a nil receiver to an Error, String, or GoString method (including the case where the method is defined on a non-pointer receiver), however, the output is the undecorated string"

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/627495 mentions this issue: fmt: document nil receiver handling for GoStringer

@cherrymui cherrymui added Documentation Issues describing a change to documentation. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Nov 15, 2024
@cherrymui cherrymui added this to the Unplanned milestone Nov 15, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Nov 16, 2024
@dmitshur dmitshur modified the milestones: Unplanned, Go1.24 Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. Unfortunate
Projects
None yet
Development

No branches or pull requests

6 participants