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: fmt: append decimals to whole-number floats when using '%#v' #26363

Closed
dave opened this issue Jul 13, 2018 · 11 comments
Closed

proposal: fmt: append decimals to whole-number floats when using '%#v' #26363

dave opened this issue Jul 13, 2018 · 11 comments

Comments

@dave
Copy link
Contributor

@dave dave commented Jul 13, 2018

Please answer these questions before submitting your issue. Thanks!

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

1.10.3

Does this issue reproduce with the latest release?

Yes

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

darwin/amd64

What did you do?

fmt.Printf("%#v", float64(0.0))
Expected: 0.0
Actual: 0
https://play.golang.org/p/KkcUz1voJG4

As per the description in e97f407 I would expect to see the decimal point added for all floats. The %#v format string is intended to return valid Go syntax. In this case, the returned Go syntax represents an integer instead of a float.

It looks like the commit linked above forgot that the sharp flag is replaced with sharpV in the case of the v verb here...

There's more discussion of this here: dave/jennifer#39
Attn: @martisch

@martisch martisch self-assigned this Jul 13, 2018
@martisch martisch added the NeedsFix label Jul 13, 2018
@dave
Copy link
Contributor Author

@dave dave commented Jul 13, 2018

Hey @martisch I'd love to have a go at fixing this myself... Would that be ok with you?

@martisch martisch removed their assignment Jul 13, 2018
@martisch martisch removed the NeedsFix label Jul 13, 2018
@martisch
Copy link
Contributor

@martisch martisch commented Jul 13, 2018

Sure, happy if you send a fix with test case, if this is indeed agreed upon to need a fix (https://golang.org/doc/contribute.html).

Looking at it again It is not 100% clear to me if we should fix this and documentation breaking backwards compatibility or remove the mention of v in the code that handles # for floats.

Note that the behavior of printing 0 also seems to be in go1.4 and possibly earlier so not introduced by e97f407. #v is special and the documentation says "always print a decimal point for %e, %E, %f, %F, %g and %G;" so my commit message was inconsistent in that regard but from the code it should have applied to #v too.

Go language spec says "A floating-point literal is a decimal representation of a floating-point constant. It has an integer part, a decimal point, a fractional part, and an exponent part." (where some parts can be omitted) and fmt documentation says "%#v a Go-syntax representation of the value". Interestingly it only says of the value and not a literal of the type, so it would seem to me 0 is ok according to the documentation and does not force a 0.0 or 0. literal.

@robpike for further opinion if #v should always have a dot or leave current formatting to not break existing uses of #v (since at least go1.4).

I lean to keeping output as is and fix the code to not handle the unreachable case of verb v in conjunction with f.sharp.

@dave
Copy link
Contributor Author

@dave dave commented Jul 13, 2018

Yep I agree it could break backwards compatibility. It would definitely break the work-around I made for the bug here.

I'll leave you guys to make the call. Happy to fix if needed...

@martisch martisch added this to the Go1.12 milestone Jul 13, 2018
@robpike
Copy link
Contributor

@robpike robpike commented Jul 14, 2018

I think it's a bug and should be fixed.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 14, 2018

Change https://golang.org/cl/123956 mentions this issue: fmt: fix incorrect format of whole-number floats when using %#v

@dave
Copy link
Contributor Author

@dave dave commented Jul 14, 2018

So there's a couple of corner cases that I've made assumptions:

Should 1.0 print 1.0 or 1.?
Although 1. is more compact, I think 1.0 looks like more natural Go syntax...

Should 1e6 print 1e+06 or 1.0e+06?
Since 1e+06 (which is the current output) will already be interpreted as a float, I don't think we want to change it.

Here's a PR: #26383

@dave
Copy link
Contributor Author

@dave dave commented Aug 28, 2018

Hey is there anything I need to do to keep this from going stale?

dave added a commit to dave/go that referenced this issue Aug 28, 2018
This fixes the unwanted behaviour where printing a zero float with the #v
fmt verb outputs "0" - e.g. missing the trailing decimal. This means that
the output would be interpreted as an int rather than a float when parsed
as Go source. After this change the the output is "0.0".

Fixes golang#26363
dave added a commit to dave/go that referenced this issue Aug 28, 2018
This fixes the unwanted behaviour where printing a zero float with the #v
fmt verb outputs "0" - e.g. missing the trailing decimal. This means that
the output would be interpreted as an int rather than a float when parsed
as Go source. After this change the the output is "0.0".

Fixes golang#26363
dave added a commit to dave/go that referenced this issue Aug 28, 2018
This fixes the unwanted behaviour where printing a zero float with the
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes golang#26363
dave added a commit to dave/go that referenced this issue Aug 28, 2018
This fixes the unwanted behaviour where printing a zero float with the
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes golang#26363
dave added a commit to dave/go that referenced this issue Aug 28, 2018
This fixes the unwanted behaviour where printing a zero float with the
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes golang#26363
@bcmills
Copy link
Member

@bcmills bcmills commented Oct 10, 2018

As noted in #27634, a breaking change to fmt should go through the proposal process, bug or otherwise.

@bcmills bcmills reopened this Oct 10, 2018
@bcmills bcmills changed the title fmt: v verb with sharp modifier incorrectly formats whole-number floats proposal: fmt: v verb with sharp modifier incorrectly formats whole-number floats Oct 10, 2018
@gopherbot gopherbot removed the NeedsFix label Oct 10, 2018
@bcmills bcmills changed the title proposal: fmt: v verb with sharp modifier incorrectly formats whole-number floats proposal: fmt: append decimals to whole-number floats when using '%#v' Oct 10, 2018
@bcmills
Copy link
Member

@bcmills bcmills commented Oct 11, 2018

See also #27634 (comment) for an argument that the existing behavior was correct (but admittedly not the clearest possible choice).

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 16, 2018

Change https://golang.org/cl/142597 mentions this issue: Revert "fmt: fix incorrect format of whole-number floats when using %#v"

gopherbot pushed a commit that referenced this issue Oct 16, 2018
Numbers without decimals are valid Go representations of whole-number
floats. That is, "var x float64 = 5" is valid Go. Avoid breakage in
tests that expect a certain output from %#v by reverting to it.

To guarantee the right type is generated by a print use %T(%#v) instead.

Added a test to lock in this behavior.

This reverts commit 7c7cecc.

Fixes #27634
Updates #26363

Change-Id: I544c400a0903777dd216452a7e86dfe60b0b0283
Reviewed-on: https://go-review.googlesource.com/c/142597
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@rsc
Copy link
Contributor

@rsc rsc commented Oct 17, 2018

Because this broke things, we rolled it back. We're going to leave things as is. Declining the proposal.

@rsc rsc closed this Oct 17, 2018
@golang golang locked and limited conversation to collaborators Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.