Go: fmt: 7c7cecc should be rolled back

[kortschak]

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

Any child of 7c7cecc.

Does this issue reproduce with the latest release?

No.

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

amd64/linux

What did you do?

Run tests for Gonum's mat package.

What did you expect to see?

Pass.

What did you see instead?

Failure because %#v now formats floats with a trailing ".0" if the value is integral.

For example here.

Test failure

=== RUN   TestFormat
--- FAIL: TestFormat (0.00s)
    format_test.go:146: unexpected format result test 0 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{0, 0, 0, 0, 0, 0, 0, 0, 0}, Stride:3}, capRows:3, capCols:3}
    format_test.go:146: unexpected format result test 1 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1, 1, 1, 1, 1, 1, 1, 1, 1}, Stride:3}, capRows:3, capCols:3}
    format_test.go:146: unexpected format result test 2 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1, 1, 1, 1, 1, 1, 1, 1, 1}, Stride:3}, capRows:3, capCols:3}
    format_test.go:146: unexpected format result test 3 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1, 0, 0, 0, 1, 0, 0, 0, 1}, Stride:3}, capRows:3, capCols:3}
    format_test.go:146: unexpected format result test 4 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{1.0, 2.0, 3.0, 4.0, 5.0, 6.0}, Stride:3}, capRows:2, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{1, 2, 3, 4, 5, 6}, Stride:3}, capRows:2, capCols:3}
    format_test.go:146: unexpected format result test 5 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:2, Data:[]float64{1.0, 2.0, 3.0, 4.0, 5.0, 6.0}, Stride:2}, capRows:3, capCols:2}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:2, Data:[]float64{1, 2, 3, 4, 5, 6}, Stride:2}, capRows:3, capCols:2}
    format_test.go:146: unexpected format result test 6 part 3:
        got:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{0.0, 1.0, 1.4142135623730951, 1.7320508075688772, 2.0, 2.23606797749979}, Stride:3}, capRows:2, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{0, 1, 1.4142135623730951, 1.7320508075688772, 2, 2.23606797749979}, Stride:3}, capRows:2, capCols:3}
    format_test.go:146: unexpected format result test 7 part 3:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:2, Data:[]float64{0.0, 1.0, 1.4142135623730951, 1.7320508075688772, 2.0, 2.23606797749979}, Stride:2}, capRows:3, capCols:2}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:2, Data:[]float64{0, 1, 1.4142135623730951, 1.7320508075688772, 2, 2.23606797749979}, Stride:2}, capRows:3, capCols:2}
    format_test.go:146: unexpected format result test 8 part 3:
        got:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{0.0, 1.0, 1.4142135623730951, 1.7320508075688772, 2.0, 2.23606797749979}, Stride:3}, capRows:2, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:2, Cols:3, Data:[]float64{0, 1, 1.4142135623730951, 1.7320508075688772, 2, 2.23606797749979}, Stride:3}, capRows:2, capCols:3}

Additional information

I believe this breaks the Go1 compatibility promise, and that it does not do so to fix a bug.

According to the fmt documentation, the %#v verb/flag specifies that "a Go-syntax representation of the value" being printed will be output. However, a string without a trailing ".0" is valid Go syntax for float values and so there was no bug.

It's true that it is not possible to infer the kind from the representation of the value when the suffix is omitted, but the fmt documentation does not say that this should possible. And already, already type information is lost (for example, even within the builtin types, signed integers are indistinguishable, unsigned integers are indistinguishable, and float types are indistinguishable).

[zegl]

The Go 1 compability does not guarantee that all programs will work the same way with all versions of Go 1, the compatibility promise gives Go the right to fix bugs. In the issue #26363 it was decided to fix this bug, even if it would change the behavior of existing programs.

[dsnet]

\cc @robpike for decision. I won't be surprised if we keep the new behavior.

[mvdan]

This was already discussed in #27598, and in the original issue where this particular behavior was changed.

In general, it's okay if tests that lock in internal behavior break between Go releases. For example, if you depend on exactly what os.File.Readdir does to handle very large directories - that logic has been rewritten twice in the past few releases, if I remember correctly.

You could say that %#v isn't internal behavior, but the new output is valid and fmt makes no promises on its exact output. Writing tests that precisely depend on its byte-by-byte output are bound to break from time to time.

[kortschak]

If it were a bug, I'd be very happy for the bug to be fixed, but this looks to me merely like a choice of aesthetic changing.

I'd argue strongly that %#v is not internal behaviour. I'd also argue that the go1compat does make claims that fmt will remain consistent unless a bug or security issue is found.

[robpike]

It was decided it was a bug, as the value "1" as a Go number is not automatically a floating-point constant and code could care. You can argue about the severity of the bug, and whether it deserves addressing, but it has been addressed, discussed, acted upon, and resolved. I believe the old behavior was incorrect and the new behavior is correct. Yes, it can break some programs. Almost _any_ change can. But this one affects very few. I would prefer we leave things as they are now. If we roll it back, we will have just one release with different behavior and that's even stranger.

There have been a number of relatively minor bug fixes like this made over time. They are considered carefully. I assure you that they are never an "aesthetic decision".

[kortschak]

If we roll it back, we will have just one release with different behavior and that's even stranger.

This change was not released.

I have found an elegant way to work around the change, but for the record, I think the decision here is wrong.

[bcmills]

0 is a valid float64 value. So is 1. Consider this program:
https://play.golang.org/p/GHO4-2G3lpv

And the analogous program using a non-default integer type, which also doesn't print any distinguishing type information:
https://play.golang.org/p/tnc2xGx252M

---

Even if the behavior is not what was originally intended, this change breaks existing tests (including a fair number within Google's internal codebase) for what seems like a marginal benefit.

I think we should roll this back. At the very least, a breaking change to fmt should go through the proposal process.

(CC @FiloSottile @andybons @rsc)

[rsc]

It's true that we do need to be able to make fixes, but this one doesn't seem important enough to force the issue. Rob is OK with rolling it back. Please do so.

[kortschak]

Interestingly, I ran into a bug in writing some tests just the other day that arose exactly because the old behaviour exists (assigning a code-generated literal 0 to an element of a map[string]interface{} where a float64 was expected). I am happy to recant the last sentence of my comment above.

[rsc]

If you really need to get the right type out of the print, you should be using %T(%#v) not just %#v.

[gopherbot]

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