Marshal/Unmarshal functions are asymmetrical

[sbuss]

While doing more investigation and revising my tests for #19486 I discovered that the roundtrip through Marshal/Unmarshal returns different data structures.

Note that #15716, while similar, only tests roundtrips for marshaling with the zero Time and doesn't spot this mismatch.

I'm not sure if this is actually a bug or intended behavior, but it's surprising enough to report.

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

failing: 0e3355903d2ebcf5ee9e76096f51ac9a116a9dbb and later (this switched to using a monotonic clock)
passing: 8179b9b462eb2946de8488a26dca91a89b3d22e6 and before

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/usr/local/google/home/sbuss/workspace"
GORACE=""
GOROOT="/usr/lib/google-golang"
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build320942242=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

$ git rev-parse HEAD
0e3355903d2ebcf5ee9e76096f51ac9a116a9dbb
$ git diff
diff --git a/src/time/time_test.go b/src/time/time_test.go
index 2922560f09..d617c8765c 100644
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -1254,3 +1254,33 @@ func TestZeroMonthString(t *testing.T) {
                t.Errorf("zero month = %q; want %q", got, want)
        }
 }
+
+func TestMarshalJSON(t *testing.T) {
+       t0 := Now()
+       enc, err := t0.MarshalJSON()
+       if err != nil {
+               t.Fatal(err)
+       }
+       t1 := Time{}
+       if err := t1.UnmarshalJSON(enc); err != nil {
+               t.Fatal(err)
+       }
+       if t1 != t0 {
+               t.Errorf("t0=%#v\nt1=%#v\nwant identical structures", t0, t1)
+       }
+}
+
+func TestMarshalBinary(t *testing.T) {
+       t0 := Now()
+       enc, err := t0.MarshalBinary()
+       if err != nil {
+               t.Fatal(err)
+       }
+       t1 := Time{}
+       if err := t1.UnmarshalBinary(enc); err != nil {
+               t.Fatal(err)
+       }
+       if t1 != t0 {
+               t.Errorf("t0=%#v\nt1=%#v\nwant identical structures", t0, t1)
+       }
+}

What did you expect to see?

I expected the structures to be identical.

What did you see instead?

At 0e33559 I get this failure:

--- FAIL: TestMarshalJSON (0.00s)
        time_test.go:1341: t0=time.Time{wall:0xbe28e6ec1a9a17af, ext:2623648639, loc:(*time.Location)(0x70e5a0)}
                t1=time.Time{wall:0x1a9a17af, ext:63624780592, loc:(*time.Location)(0x70e5a0)}
                want identical structures
--- FAIL: TestMarshalBinary (0.00s)
        time_test.go:1356: t0=time.Time{wall:0xbe28e6ec1a9c07f0, ext:2623775682, loc:(*time.Location)(0x70e5a0)}
                t1=time.Time{wall:0x1a9c07f0, ext:63624780592, loc:(*time.Location)(0x70e5a0)}
                want identical structures
FAIL
FAIL    time    2.672s

[sbuss]

Actually, I think this is intended behavior, as pointed out in the docs for package time:

// Because the monotonic clock reading has no meaning outside
// the current process, the serialized forms generated by t.GobEncode,
// t.MarshalBinary, t.MarshalJSON, and t.MarshalText omit the monotonic
// clock reading, and t.Format provides no format for it. Similarly, the
// constructors time.Date, time.Parse, time.ParseInLocation, and time.Unix,
// as well as the unmarshalers t.GobDecode, t.UnmarshalBinary.
// t.UnmarshalJSON, and t.UnmarshalText always create times with
// no monotonic clock reading.
//
// Note that the Go == operator includes the monotonic clock reading in
// its comparison. If time values returned from time.Now and time values
// constructed by other means (for example, by time.Parse or time.Unix)
// are meant to compare equal when used as map keys, the times returned
// by time.Now must have the monotonic clock reading stripped, by setting
// t = t.AddDate(0, 0, 0). In general, prefer t.Equal(u) to t == u, since
// t.Equal uses the most accurate comparison available and correctly
// handles the case when only one of its arguments has a monotonic clock
// reading.

Closing this