encoding/json: reuse values when decoding map elements

When we decode into a struct, each input key-value may be decoded into
one of the struct's fields. Particularly, existing data isn't dropped,
so that some sub-fields can be decoded into without zeroing all other
data.

However, decoding into a map behaved in the opposite way. Whenever a
key-value was decoded, it completely replaced the previous map element.
If the map contained any non-zero data in that key, it's dropped.

Instead, try to reuse the existing element value if possible. If the map
element type is a pointer, and the value is non-nil, we can decode
directly into it. If it's not a pointer, make a copy and decode into
that copy, as map element values aren't addressable.

This means we have to parse and convert the map element key before the
value, to be able to obtain the existing element value. This is fine,
though. Moreover, reporting errors on the key before the value follows
the input order more closely.

Finally, add a test to explore the four combinations, involving pointer
and non-pointer, and non-zero and zero values. A table-driven test
wasn't used, as each case required different checks, such as checking
that the non-nil pointer case doesn't end up with a different pointer.

Fixes #31924.

Change-Id: I5ca40c9963a98aaf92f26f0b35843c021028dfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/179337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/encoding/json/decode_test.go

@@ -2569,3 +2569,57 @@ func TestUnmarshalMaxDepth(t *testing.T) {
 		}
 	}
 }
+
+func TestUnmarshalMapPointerElem(t *testing.T) {
+	type S struct{ Unchanged, Changed int }
+	input := []byte(`{"S":{"Changed":5}}`)
+	want := S{1, 5}
+
+	// First, a map with struct pointer elements. The key-value pair exists,
+	// so reuse the existing value.
+	s := &S{1, 2}
+	ptrMap := map[string]*S{"S": s}
+	if err := Unmarshal(input, &ptrMap); err != nil {
+		t.Fatal(err)
+	}
+	if s != ptrMap["S"] {
+		t.Fatal("struct pointer element in map was completely replaced")
+	}
+	if got := *s; got != want {
+		t.Fatalf("want %#v, got %#v", want, got)
+	}
+
+	// Second, a map with struct elements. The key-value pair exists, but
+	// the value isn't addresable, so make a copy and use that.
+	s = &S{1, 2}
+	strMap := map[string]S{"S": *s}
+	if err := Unmarshal(input, &strMap); err != nil {
+		t.Fatal(err)
+	}
+	if *s == strMap["S"] {
+		t.Fatal("struct element in map wasn't copied")
+	}
+	if got := strMap["S"]; got != want {
+		t.Fatalf("want %#v, got %#v", want, got)
+	}
+
+	// Finally, check the cases where the key-value pair exists, but the
+	// value is zero.
+	want = S{0, 5}
+
+	ptrMap = map[string]*S{"S": nil}
+	if err := Unmarshal(input, &ptrMap); err != nil {
+		t.Fatal(err)
+	}
+	if got := *ptrMap["S"]; got != want {
+		t.Fatalf("want %#v, got %#v", want, got)
+	}
+
+	strMap = map[string]S{"S": {}}
+	if err := Unmarshal(input, &strMap); err != nil {
+		t.Fatal(err)
+	}
+	if got := strMap["S"]; got != want {
+		t.Fatalf("want %#v, got %#v", want, got)
+	}
+}