Skip to content

Commit

Permalink
Revert "encoding/json: don't reuse slice elements when decoding"
Browse files Browse the repository at this point in the history
This reverts https://golang.org/cl/191783.

Reason for revert: Broke too many programs which depended on the previous
behavior, even when it was the opposite of what the documentation said.

We can attempt to fix the original issue again for 1.16, while keeping
those programs in mind.

Fixes #39427.

Change-Id: I7a7f24b2a594c597ef625aeff04fff29aaa88fc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/240657
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
  • Loading branch information
mvdan committed Jul 2, 2020
1 parent 188be6b commit 2b68dc3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 36 deletions.
40 changes: 18 additions & 22 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ func (d *decodeState) unmarshal(v interface{}) error {
d.scanWhile(scanSkipSpace)
// We decode rv not rv.Elem because the Unmarshaler interface
// test must be applied at the top level of the value.
if err := d.value(rv); err != nil {
err := d.value(rv)
if err != nil {
return d.addErrorContext(err)
}
return d.savedError
Expand Down Expand Up @@ -507,7 +508,6 @@ func (d *decodeState) array(v reflect.Value) error {
return nil
}
v = pv
initialSliceCap := 0

// Check type of target.
switch v.Kind() {
Expand All @@ -524,9 +524,8 @@ func (d *decodeState) array(v reflect.Value) error {
d.saveError(&UnmarshalTypeError{Value: "array", Type: v.Type(), Offset: int64(d.off)})
d.skip()
return nil
case reflect.Slice:
initialSliceCap = v.Cap()
case reflect.Array:
case reflect.Array, reflect.Slice:
break
}

i := 0
Expand All @@ -537,6 +536,7 @@ func (d *decodeState) array(v reflect.Value) error {
break
}

// Get element of array, growing if necessary.
if v.Kind() == reflect.Slice {
// Grow slice if necessary
if i >= v.Cap() {
Expand All @@ -552,22 +552,19 @@ func (d *decodeState) array(v reflect.Value) error {
v.SetLen(i + 1)
}
}
var into reflect.Value

if i < v.Len() {
into = v.Index(i)
if i < initialSliceCap {
// Reusing an element from the slice's original
// backing array; zero it before decoding.
into.Set(reflect.Zero(v.Type().Elem()))
// Decode into element.
if err := d.value(v.Index(i)); err != nil {
return err
}
} else {
// Ran out of fixed array: skip.
if err := d.value(reflect.Value{}); err != nil {
return err
}
}
i++
// Note that we decode the value even if we ran past the end of
// the fixed array. In that case, we decode into an empty value
// and do nothing with it.
if err := d.value(into); err != nil {
return err
}

// Next token must be , or ].
if d.opcode == scanSkipSpace {
Expand All @@ -583,17 +580,16 @@ func (d *decodeState) array(v reflect.Value) error {

if i < v.Len() {
if v.Kind() == reflect.Array {
// Zero the remaining elements.
zero := reflect.Zero(v.Type().Elem())
// Array. Zero the rest.
z := reflect.Zero(v.Type().Elem())
for ; i < v.Len(); i++ {
v.Index(i).Set(zero)
v.Index(i).Set(z)
}
} else {
v.SetLen(i)
}
}
if v.Kind() == reflect.Slice && v.IsNil() {
// Don't allow the resulting slice to be nil.
if i == 0 && v.Kind() == reflect.Slice {
v.Set(reflect.MakeSlice(v.Type(), 0, 0))
}
return nil
Expand Down
15 changes: 1 addition & 14 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2099,10 +2099,7 @@ func TestSkipArrayObjects(t *testing.T) {
// slices, and arrays.
// Issues 4900 and 8837, among others.
func TestPrefilled(t *testing.T) {
type T struct {
A, B int
}
// Values here change, cannot reuse the table across runs.
// Values here change, cannot reuse table across runs.
var prefillTests = []struct {
in string
ptr interface{}
Expand Down Expand Up @@ -2138,16 +2135,6 @@ func TestPrefilled(t *testing.T) {
ptr: &[...]int{1, 2},
out: &[...]int{3, 0},
},
{
in: `[{"A": 3}]`,
ptr: &[]T{{A: -1, B: -2}, {A: -3, B: -4}},
out: &[]T{{A: 3}},
},
{
in: `[{"A": 3}]`,
ptr: &[...]T{{A: -1, B: -2}, {A: -3, B: -4}},
out: &[...]T{{A: 3, B: -2}, {}},
},
}

for _, tt := range prefillTests {
Expand Down

0 comments on commit 2b68dc3

Please sign in to comment.