This change makes the fast path for ASCII characters inlineable in
DecodeRune and DecodeRuneInString and removes most instances of manual
inlining at call sites.
Here are some benchmark results (no change to allocations):
goos: darwin
goarch: amd64
pkg: unicode/utf8
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
│ old │ new │
│ sec/op │ sec/op vs base │
DecodeASCIIRune-8 2.4545n ± 2% 0.6253n ± 2% -74.52% (p=0.000 n=20)
DecodeJapaneseRune-8 3.988n ± 1% 4.023n ± 1% +0.86% (p=0.050 n=20)
DecodeASCIIRuneInString-8 2.4675n ± 1% 0.6264n ± 2% -74.61% (p=0.000 n=20)
DecodeJapaneseRuneInString-8 3.992n ± 1% 4.001n ± 1% ~ (p=0.625 n=20)
geomean 3.134n 1.585n -49.43%
Note: when #61502 gets resolved, DecodeRune and DecodeRuneInString should
be reverted to their idiomatic implementations.
Fixes#31666
Updates #48195
Change-Id: I4be25c4f52417dc28b3a7bd72f1b04018470f39d
GitHub-Last-Rev: 2e352a0045
GitHub-Pull-Request: golang/go#75181
Reviewed-on: https://go-review.googlesource.com/c/go/+/699675
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Add a section to the package doc which details the security
considerations of using encoding/json, in particular with respect to
parser misalignment issues.
Additionally, clarify previously ambiguous statement in the Unmarshal
doc about how case is used when matching keys in objects, and add a note
about how duplicate keys are handled.
Fixes#14750
Change-Id: I66f9b845efd98c86a684d7333b3aa8a456564922
Reviewed-on: https://go-review.googlesource.com/c/go/+/684315
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
This imports the proposed new v2 JSON API implemented in
github.com/go-json-experiment/json as of commit
d3c622f1b874954c355e60c8e6b6baa5f60d2fed.
When GOEXPERIMENT=jsonv2 is set, the encoding/json/v2 and
encoding/jsontext packages are visible, the encoding/json
package is implemented in terms of encoding/json/v2, and
the encoding/json package include various additional APIs.
(See #71497 for details.)
When GOEXPERIMENT=jsonv2 is not set, the new API is not
present and the encoding/json package is unchanged.
The experimental API is not bound by the Go compatibility
promise and is expected to evolve as updates are made to
the json/v2 proposal.
The contents of encoding/json/internal/jsontest/testdata
are compressed with zstd v1.5.7 with the -19 option.
Fixes#71845
For #71497
Change-Id: Ib8c94e5f0586b6aaa22833190b41cf6ef59f4f01
Reviewed-on: https://go-review.googlesource.com/c/go/+/665796
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
It is not true that Unmarshal always treats a JSON null
as being equivalent to a no-op.
For bools, ints, uints, floats, strings, arrays, and structs,
it treats a JSON null as a no-op. However, for []byte, slice,
map, pointer, or interface, it zeros the underlying value.
Remove this suggestion as the actual behavior is inconsistent.
Note that the proposed behavior in v2 Unmarshal is to consistently
zero out the underlying value.
Change-Id: I02cef0bf7919f25cfd0aceb04486d37498761181
Reviewed-on: https://go-review.googlesource.com/c/go/+/638416
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
This change applies a fix for a reflect.Value incorrect comparison
using "==" or reflect.DeepEqual.
This change is a precursor to the change that'll bring in the
static analyzer "reflectvaluecompare", by ensuring that all tests
pass beforehand.
Updates #43993
Change-Id: I6c47eb0a1de6353ac7495cb8cb49b318b7ebba56
Reviewed-on: https://go-review.googlesource.com/c/go/+/626116
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Including embedded struct inforamtion in error message.
Fixes#68941
Change-Id: I6a6f7d506104839a9a7cf1a2c3003272f5534a79
GitHub-Last-Rev: 717f680aca
GitHub-Pull-Request: golang/go#68966
Reviewed-on: https://go-review.googlesource.com/c/go/+/606956
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
When people return UnmarshalTypeError in UnmarshalJSON, we should append error's Field to FieldStack.
Fixes#68750
Change-Id: I0a5a9b259a1b569de1bebc815ec936c913e10469
GitHub-Last-Rev: 18796addc3
GitHub-Pull-Request: golang/go#68870
Reviewed-on: https://go-review.googlesource.com/c/go/+/605455
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For #49884
Change-Id: I1623201c47c820a152773d2f43d0072a1466d3bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/588118
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
While here, also fix doc link for encoding.TextMarshaler.
Fixes#67495
Change-Id: Ia2a674c5c35b5a849ce8f5eef3d34d165b3195b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/593335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
When storing literal to JSON number v, if s is valid number, the slicebytetostring operation will be performed twice. In fact, the operation is unavoidable on any code path, so just perform it at the very beginning.
This is not a big optimization, but better than nothing:
$ ../bin/go test ./encoding/json/ -bench UnmarshalNumber -run NOTEST -benchtime 10000000x -count 16 > old.txt
$ ../bin/go test ./encoding/json/ -bench UnmarshalNumber -run NOTEST -benchtime 10000000x -count 16 > new.txt
$ benchstat old.txt new.txt
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
UnmarshalNumber-8 234.5n ± 3% 228.2n ± 4% -2.67% (p=0.033 n=16)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
UnmarshalNumber-8 168.0 ± 0% 168.0 ± 0% ~ (p=1.000 n=16) ¹
¹ all samples are equal
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
UnmarshalNumber-8 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=16) ¹
¹ all samples are equal
Change-Id: I1dfdb1ed0883e385f753b2046b7f047c792aa4e3
GitHub-Last-Rev: d236dd7265
GitHub-Pull-Request: golang/go#61242
Reviewed-on: https://go-review.googlesource.com/c/go/+/508556
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: qiulaidongfeng <2645477756@qq.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
CL 567296 added {OverflowComplex, OverflowFloat, OverflowInt, OverflowUint}
to reflect.Type, this CL uses these methods to simplify code.
For #60427
Change-Id: I229aef9e4095a2f025afd782081f6c9e6d7710f3
GitHub-Last-Rev: c824e5a1b5
GitHub-Pull-Request: golang/go#66000
Reviewed-on: https://go-review.googlesource.com/c/go/+/567775
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Across all encoding packages, linkify declarations if possible.
In some cases, we convert a code block into a bulleted list,
which then further allows for more linkification.
Change-Id: I68fedf362615b34228bab5d4859b7d87d831c570
Reviewed-on: https://go-review.googlesource.com/c/go/+/524977
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: qiulaidongfeng <2645477756@qq.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Over the past few months as I read the standard library's documentation
I kept finding spots where godoc links would have helped me.
I kept adding to a stash of changes to fix them up bit by bit.
The stash has grown big enough by now, and we're nearing a release,
so I think it's time to merge to avoid git conflicts or bit rot.
Note that a few sentences are slightly reworded,
since "implements the Fooer interface" can just be "implements [Fooer]"
now that the link provides all the context needed to the user.
Change-Id: I01c31d3d3ff066d06aeb44f545f8dd0fb9a8d998
Reviewed-on: https://go-review.googlesource.com/c/go/+/508395
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For #60088
Change-Id: I2e471c76de62944b14472966b63f5778124b9b8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/514655
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
benchmark old ns/op new ns/op delta
BenchmarkUnmarshalMap-10 218 172 -21.28%
benchmark old allocs new allocs delta
BenchmarkUnmarshalMap-10 15 12 -20.00%
benchmark old bytes new bytes delta
BenchmarkUnmarshalMap-10 328 256 -21.95%
Change-Id: Ie20ab62731c752eb0040c6d1591fedd7d12b1e0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/514100
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
The folded name logic (despite all attempts to optimize it)
was fundamentally an O(n) operation where every field in a struct
needed to be linearly scanned in order to find a match.
This made unmashaling of unknown fields always O(n).
Instead of optimizing the comparison for each field,
make it such that we can look up a name in O(1).
We accomplish this by maintaining a map keyed by pre-folded names,
which we can pre-calculate when processing the struct type.
Using a stack-allocated buffer, we can fold the input name and
look up its presence in the map.
Also, instead of mapping from names to indexes,
map directly to a pointer to the field information.
The memory cost of this is the same and avoids an extra slice index.
The new logic is both simpler and faster.
Performance:
name old time/op new time/op delta
CodeDecoder 2.47ms ± 4% 2.42ms ± 2% -1.83% (p=0.022 n=10+9)
UnicodeDecoder 259ns ± 2% 248ns ± 1% -4.32% (p=0.000 n=10+10)
DecoderStream 150ns ± 1% 149ns ± 1% ~ (p=0.516 n=10+10)
CodeUnmarshal 3.13ms ± 2% 3.09ms ± 2% -1.37% (p=0.022 n=10+9)
CodeUnmarshalReuse 2.50ms ± 1% 2.45ms ± 1% -1.96% (p=0.001 n=8+9)
UnmarshalString 67.1ns ± 5% 64.5ns ± 5% -3.90% (p=0.005 n=10+10)
UnmarshalFloat64 60.1ns ± 4% 58.4ns ± 2% -2.89% (p=0.002 n=10+8)
UnmarshalInt64 51.0ns ± 4% 49.2ns ± 1% -3.53% (p=0.001 n=10+8)
Issue10335 80.7ns ± 2% 79.2ns ± 1% -1.82% (p=0.016 n=10+8)
Issue34127 28.6ns ± 3% 28.8ns ± 3% ~ (p=0.388 n=9+10)
Unmapped 177ns ± 2% 177ns ± 2% ~ (p=0.956 n=10+10)
Change-Id: I478b2b958f5a63a69c9a991a39cd5ffb43244a2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/471196
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
v.SetZero() is faster than v.Set(reflect.Zero(v.Type()))
and was recently added in Go 1.20.
Benchmark numbers are largely unchanged since this mainly
affects the unmarshaling of large numbers of JSON nulls,
which our benchmarks do not heavily exercise.
Change-Id: I464f60f63c9027e63a99fd5da92e7ab782018329
Reviewed-on: https://go-review.googlesource.com/c/go/+/471195
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The Grow method is generally a more efficient way to grow a slice.
The older approach of using reflect.MakeSlice has to
waste effort zeroing the elements overwritten by the older slice
and has to allocate the slice header on the heap.
Performance:
name old time/op new time/op delta
CodeDecoder 2.41ms ± 2% 2.42ms ± 2% ~
CodeUnmarshal 3.12ms ± 3% 3.13ms ± 3% ~
CodeUnmarshalReuse 2.49ms ± 3% 2.52ms ± 3% ~
name old alloc/op new alloc/op delta
CodeDecoder 2.00MB ± 1% 1.99MB ± 1% ~
CodeUnmarshal 3.05MB ± 0% 2.92MB ± 0% -4.23%
CodeUnmarshalReuse 1.68MB ± 0% 1.68MB ± 0% -0.32%
name old allocs/op new allocs/op delta
CodeDecoder 77.1k ± 0% 77.0k ± 0% -0.09%
CodeUnmarshal 92.7k ± 0% 91.3k ± 0% -1.47%
CodeUnmarshalReuse 77.1k ± 0% 77.0k ± 0% -0.07%
The Code benchmarks (which are the only ones that uses slices)
are largely unaffected. There is a slight reduction in allocations.
A histogram of slice lengths from the Code testdata is as follows:
≤1: 392
≤2: 256
≤4: 252
≤8: 152
≤16: 126
≤32: 78
≤64: 62
≤128: 46
≤256: 18
≤512: 10
≤1024: 8
A bulk majority of slice lengths are 8 elements or under.
Use of reflect.Value.Grow performs better for larger slices since
it can avoid the zeroing of memory and has a faster growth rate.
However, Grow grows starting from 1 element,
with a 2x growth rate until some threshold (currently 512),
Starting from 1 ensures better utilization of the heap,
but at the cost of more frequent regrowth early on.
In comparison, the previous logic always started
with a minimum of 4 elements, which leads to a wasted capacity
of 75% for the highly frequent case of a single element slice.
The older code always had a growth rate of 1.5x,
and so wastes less memory for number of elements below 512.
All in all, there are too many factors that hurt or help performance.
Rergardless, the simplicity of favoring reflect.Value.Grow
over manually managing growth rates is a welcome simplification.
Change-Id: I62868a7f112ece3c2da3b4f6bdf74d397110243c
Reviewed-on: https://go-review.googlesource.com/c/go/+/471175
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Change-Id: I71c9d9ef9d21a7ae9466d8c7b283fdfbba01f5a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/390734
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
go/doc in all its forms applies this replacement when rendering
the comments. We are considering formatting doc comments,
including doing this replacement as part of the formatting.
Apply it to our source files ahead of time.
For #51082.
Change-Id: Ifcc1f5861abb57c5d14e7d8c2102dfb31b7a3a19
Reviewed-on: https://go-review.googlesource.com/c/go/+/384262
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A future change to gofmt will rewrite
// Doc comment.
//
func f()
to
// Doc comment.
func f()
Apply that change preemptively to all doc comments.
For #51082.
Change-Id: I4023e16cfb0729b64a8590f071cd92f17343081d
Reviewed-on: https://go-review.googlesource.com/c/go/+/384259
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes#49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The decodeState type is a large part of the allocated space during Unmarshal.
The errorContext field is infrequently used, and only on error.
Extract it into a pointer and allocate it separate when necessary.
name old time/op new time/op delta
UnmarshalString-8 115ns ± 5% 114ns ± 3% ~ (p=0.170 n=15+15)
UnmarshalFloat64-8 113ns ± 2% 106ns ± 1% -6.42% (p=0.000 n=15+14)
UnmarshalInt64-8 93.3ns ± 1% 86.9ns ± 4% -6.89% (p=0.000 n=14+15)
name old alloc/op new alloc/op delta
UnmarshalString-8 192B ± 0% 160B ± 0% -16.67% (p=0.000 n=15+15)
UnmarshalFloat64-8 180B ± 0% 148B ± 0% -17.78% (p=0.000 n=15+15)
UnmarshalInt64-8 176B ± 0% 144B ± 0% -18.18% (p=0.000 n=15+15)
name old allocs/op new allocs/op delta
UnmarshalString-8 2.00 ± 0% 2.00 ± 0% ~ (all equal)
UnmarshalFloat64-8 2.00 ± 0% 2.00 ± 0% ~ (all equal)
UnmarshalInt64-8 1.00 ± 0% 1.00 ± 0% ~ (all equal)
Change-Id: I53f3f468e6c65f77a12e5138a2626455b197012d
Reviewed-on: https://go-review.googlesource.com/c/go/+/271338
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This reverts CL 254537.
Reason for revert: Reason for revert: The recommended way to check for a type of error is errors.As. API changes should also start with a proposal.
Change-Id: I07c37428575e99c80b17525833a61831d10963bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/254857
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Allows users to check:
errors.Is(err, &UnmarshalTypeError{})
errors.Is(err, &UnmarshalFieldError{})
errors.Is(err, &InvalidUnmarshalError{})
errors.Is(err, &UnsupportedValueError{})
errors.Is(err, &MarshalerError{})
which is the recommended way of checking for kinds of errors.
SyntaxError.Is was implemented in CL 253037.
As and Unwrap relevant methods will be added in future CLs.
Change-Id: I1f8a503b8fdc0f3afdfe9669a91f3af8d960e028
GitHub-Last-Rev: 930cda5384
GitHub-Pull-Request: golang/go#41360
Reviewed-on: https://go-review.googlesource.com/c/go/+/254537
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
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>
This reverts golang.org/cl/190659 and golang.org/cl/226218, minus the
regression tests in the latter.
The original work happened in golang.org/cl/151157, which was reverted
in golang.org/cl/190909 due to a crash found by fuzzing.
We tried a second time in golang.org/cl/190659, which shipped with Go
1.14. A bug was found, where strings would be mangled in certain edge
cases. The fix for that was golang.org/cl/226218, which was backported
into Go 1.14.4.
Unfortunately, a second regression was just reported in #39555, which is
a similar case of strings getting mangled when decoding under certain
conditions. It would be possible to come up with another small patch to
fix that edge case, but instead, let's just revert the entire
optimization, as it has proved to do more harm than good. Moreover, it's
hard to argue or prove that there will be no more such regressions.
However, all the work wasn't for nothing. First, we learned that the way
the decoder unquotes tokenized strings isn't simple; initially, we had
wrongly assumed that each string was unquoted exactly once and in order.
Second, we have gained a number of regression tests which will be useful
to prevent the same mistakes in the future, including the test cases we
add in this CL.
Fixes#39555.
Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/237838
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This reverts golang.org/cl/179337.
Reason for revert: broke a few too many reasonably valid Go programs.
The previous behavior was perhaps less consistent, but the docs were
never very clear about when the decoder merges with existing values,
versus replacing existing values altogether.
Fixes#39149.
Change-Id: I1c1d857709b8398969fe421aa962f6b62f91763a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234559
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
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>
The added comment contains some context. The original optimization
assumed that each call to unquoteBytes (or unquote) followed its
corresponding call to rescanLiteral. Otherwise, unquoting a literal
might use d.safeUnquote from another re-scanned literal.
Unfortunately, this assumption is wrong. When decoding {"foo": "bar"}
into a map[T]string where T implements TextUnmarshaler, the sequence of
calls would be as follows:
1) rescanLiteral "foo"
2) unquoteBytes "foo"
3) rescanLiteral "bar"
4) unquoteBytes "foo" (for UnmarshalText)
5) unquoteBytes "bar"
Note that the call to UnmarshalText happens in literalStore, which
repeats the work to unquote the input string literal. But, since that
happens after we've re-scanned "bar", we're using the wrong safeUnquote
field value.
In the added test case, the second string had a non-zero number of safe
bytes, and the first string had none since it was all non-ASCII. Thus,
"safely" unquoting a number of the first string's bytes could cut a rune
in half, and thus mangle the runes.
A rather simple fix, without a full revert, is to only allow one use of
safeUnquote per call to unquoteBytes. Each call to rescanLiteral when
we have a string is soon followed by a call to unquoteBytes, so it's no
longer possible for us to use the wrong index.
Also add a test case from #38126, which is the same underlying bug, but
affecting the ",string" option.
Before the fix, the test would fail, just like in the original two issues:
--- FAIL: TestUnmarshalRescanLiteralMangledUnquote (0.00s)
decode_test.go:2443: Key "开源" does not exist in map: map[开���:12345开源]
decode_test.go:2458: Unmarshal unexpected error: json: invalid use of ,string struct tag, trying to unmarshal "\"aaa\tbbb\"" into string
Fixes#38105.
For #38126.
Change-Id: I761e54924e9a971a4f9eaa70bbf72014bb1476e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/226218
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
The previous behavior directly contradicted the docs that have been in
place for years:
To unmarshal a JSON array into a slice, Unmarshal resets the
slice length to zero and then appends each element to the slice.
We could use reflect.New to create a new element and reflect.Append to
then append it to the destination slice, but benchmarks have shown that
reflect.Append is very slow compared to the code that manually grows a
slice in this file.
Instead, if we're decoding into an element that came from the original
backing array, zero it before decoding into it. We're going to be using
the CodeDecoder benchmark, as it has a slice of struct pointers that's
decoded very often.
Note that we still reuse existing values from arrays being decoded into,
as the documentation agrees with the existing implementation in that
case:
To unmarshal a JSON array into a Go array, Unmarshal decodes
JSON array elements into corresponding Go array elements.
The numbers with the benchmark as-is might seem catastrophic, but that's
only because the benchmark is decoding into the same variable over and
over again. Since the old decoder was happy to reuse slice elements, it
would save a lot of allocations by not having to zero and re-allocate
said elements:
name old time/op new time/op delta
CodeDecoder-8 10.4ms ± 1% 10.9ms ± 1% +4.41% (p=0.000 n=10+10)
name old speed new speed delta
CodeDecoder-8 186MB/s ± 1% 178MB/s ± 1% -4.23% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.19MB ± 0% 3.59MB ± 0% +64.09% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.8k ± 0% 92.7k ± 0% +20.71% (p=0.000 n=10+10)
We can prove this by moving 'var r codeResponse' into the loop, so that
the benchmark no longer reuses the destination pointer. And sure enough,
we no longer see the slow-down caused by the extra allocations:
name old time/op new time/op delta
CodeDecoder-8 10.9ms ± 0% 10.9ms ± 1% -0.37% (p=0.043 n=10+10)
name old speed new speed delta
CodeDecoder-8 177MB/s ± 0% 178MB/s ± 1% +0.37% (p=0.041 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 3.59MB ± 0% 3.59MB ± 0% ~ (p=0.780 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 92.7k ± 0% 92.7k ± 0% ~ (all equal)
I believe that it's useful to leave the benchmarks as they are now,
because the decoder does reuse memory in some cases. For example,
existing map elements are reused. However, subtle changes like this one
need to be benchmarked carefully.
Finally, add a couple of tests involving both a slice and an array of
structs.
Fixes#21092.
Change-Id: I8b1194f25e723a31abd146fbfe9428ac10c1389d
Reviewed-on: https://go-review.googlesource.com/c/go/+/191783
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a re-submission of CL 151157, since it was reverted in CL 190909
due to an introduced crash found by a fuzzer. The revert CL included
regression tests, while this CL includes a fixed version of the original
change.
In particular, what we forgot in the original optimization was that we
still need the length and trailing quote checks at the beginning of
unquoteBytes. Without those, we could end up in a crash later on.
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove the check that s[0]=='"', since all call
sites already meet that condition.
name old time/op new time/op delta
CodeDecoder-8 10.6ms ± 2% 10.5ms ± 1% -1.01% (p=0.004 n=20+10)
name old speed new speed delta
CodeDecoder-8 183MB/s ± 2% 185MB/s ± 1% +1.02% (p=0.003 n=20+10)
Updates #28923.
Change-Id: I8c6b13302bcd86a364bc998d72451332c0809cde
Reviewed-on: https://go-review.googlesource.com/c/go/+/190659
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
When unmarshaling to a map, the map's key type must either be a string,
an integer, or implement encoding.TextUnmarshaler. But for a user
defined type, reflect.Kind will not distinguish between the static type
and the underlying type. In:
var x MyString = "x"
t := reflect.TypeOf(x)
println(t.Kind() == reflect.String)
the Kind of x is still reflect.String, even though the static type of x
is MyString.
Moreover, checking for the map's key type is a string occurs first, so
even if the map key type MyString implements encoding.TextUnmarshaler,
it will be ignored.
To fix the bug, check for encoding.TextUnmarshaler first.
Fixes#34437
Change-Id: I780e0b084575e1dddfbb433fe03857adf71d05fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/200237
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unmarshaling a string into a json.Number should first check that the string is a valid Number.
If not, we should fail without decoding it.
Fixes#14702
Change-Id: I286178e93df74ad63c0a852c3f3489577072cf47
GitHub-Last-Rev: fe69bb68ee
GitHub-Pull-Request: golang/go#34272
Reviewed-on: https://go-review.googlesource.com/c/go/+/195045
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The indirect method checked the type of the child when indirecting a
pointer. If the current value is a pointer and we are decoding null, we
can skip this entirely and return early, avoiding the whole descent.
Fixes#31776
Change-Id: Ib8b2a2357572c41f56fceac59b5a858980f3f65e
Reviewed-on: https://go-review.googlesource.com/c/go/+/174699
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This is a documentation-only change
Fixes#33298
Change-Id: I816058a872b57dc868dff11887214d9de92d9342
Reviewed-on: https://go-review.googlesource.com/c/go/+/188821
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The decoder called this function to check numbers being decoded into a
json.Number. However, these can't be quoted as strings, so the tokenizer
has already verified they are valid JSON numbers.
Verified this by adding a test with such an input. As expected, it
produces a syntax error, not the fmt.Errorf - that line could never
execute.
Since the only remaining non-test caller of isvalidnumber is in
encode.go, move the function there.
This change should slightly reduce the amount of work when decoding into
json.Number, though that isn't very common nor part of any current
benchmarks.
Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/184817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts CL 151157.
CL 151157 introduced a crash when decoding into ",string" fields. It
came with a moderate speedup, so at this stage of the release cycle
let's just revert it, and reapply it in Go 1.14 with the fix in CL 190659.
Also applied the test cases from CL 190659.
Updates #33728
Change-Id: Ie46e2bc15224b251888580daf6b79d5865f3878e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190909
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
indirect walks down v until it gets to a non-pointer. But it does not
handle the case when v is a pointer to itself, like in:
var v interface{}
v = &v
Unmarshal(b, v)
So just stop immediately if we see v is a pointer to itself.
Fixes#31740
Change-Id: Ie396264119e24d70284cd9bf76dcb2050babb069
Reviewed-on: https://go-review.googlesource.com/c/go/+/174337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the common case, structs have a handful of fields and most inputs
match struct field names exactly.
The previous code would do a linear search over the fields, stopping at
the first exact match, and otherwise using the first case insensitive
match.
This is unfortunate, because it means that for the common case, we'd do
a linear search with bytes.Equal. Even for structs with only two or
three fields, that is pretty wasteful.
Worse even, up until the exact match was found via the linear search,
all previous fields would run their equalFold functions, which aren't
cheap even in the simple case.
Instead, cache a map along with the field list that indexes the fields
by their name. This way, a case sensitive field search doesn't involve a
linear search, nor does it involve any equalFold func calls.
This patch should also slightly speed up cases where there's a case
insensitive match but not a case sensitive one, as then we'd avoid
calling bytes.Equal on all the fields. Though that's not a common case,
and there are no benchmarks for it.
name old time/op new time/op delta
CodeDecoder-8 11.0ms ± 0% 10.6ms ± 1% -4.42% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 176MB/s ± 0% 184MB/s ± 1% +4.62% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.28MB ± 0% 2.28MB ± 0% ~ (p=0.725 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.9k ± 0% 76.9k ± 0% ~ (all equal)
Updates #28923.
Change-Id: I9929c1f06c76505e5b96914199315dbdaae5dc76
Reviewed-on: https://go-review.googlesource.com/c/go/+/172918
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove unnecessary checks like len(s)<2 and
s[0]=='"'. Add a comment explaining why.
name old time/op new time/op delta
CodeDecoder-8 11.2ms ± 0% 11.1ms ± 1% -1.63% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 173MB/s ± 0% 175MB/s ± 1% +1.66% (p=0.000 n=9+10)
Updates #28923.
Change-Id: I2436a3a7f8148a2f7a6a4cdbd7dec6b32ef5e20c
Reviewed-on: https://go-review.googlesource.com/c/go/+/151157
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Decoder.Decode and Unmarshal actually scan the input bytes twice - the
first time to check for syntax errors and the length of the value, and
the second to perform the decoding.
It's in the second scan that we actually tokenize the bytes. Since
syntax errors aren't a possibility, we can take shortcuts.
In particular, literals such as quoted strings are very common in JSON,
so we can avoid a lot of work by special casing them.
name old time/op new time/op delta
CodeDecoder-8 10.3ms ± 1% 9.1ms ± 0% -11.89% (p=0.002 n=6+6)
UnicodeDecoder-8 342ns ± 0% 283ns ± 0% -17.25% (p=0.000 n=6+5)
DecoderStream-8 239ns ± 0% 230ns ± 0% -3.90% (p=0.000 n=6+5)
CodeUnmarshal-8 11.0ms ± 0% 9.8ms ± 0% -11.45% (p=0.002 n=6+6)
CodeUnmarshalReuse-8 10.3ms ± 0% 9.0ms ± 0% -12.72% (p=0.004 n=5+6)
UnmarshalString-8 104ns ± 0% 92ns ± 0% -11.35% (p=0.002 n=6+6)
UnmarshalFloat64-8 93.2ns ± 0% 87.6ns ± 0% -6.01% (p=0.010 n=6+4)
UnmarshalInt64-8 74.5ns ± 0% 71.5ns ± 0% -3.91% (p=0.000 n=5+6)
name old speed new speed delta
CodeDecoder-8 189MB/s ± 1% 214MB/s ± 0% +13.50% (p=0.002 n=6+6)
UnicodeDecoder-8 40.9MB/s ± 0% 49.5MB/s ± 0% +20.96% (p=0.002 n=6+6)
CodeUnmarshal-8 176MB/s ± 0% 199MB/s ± 0% +12.93% (p=0.002 n=6+6)
Updates #28923.
Change-Id: I7a5e2aef51bd4ddf2004aad24210f6f50e01eaeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/151042
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In golang.org/cl/145218, a feature was added where the JSON decoder
would keep track of the entire path to a field when reporting an
UnmarshalTypeError.
However, we all failed to check if this affected the benchmarks - myself
included, as a reviewer. Below are the numbers comparing the CL's parent
with itself, once it was merged:
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 28.2ms ± 2% +119.33% (p=0.002 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 69MB/s ± 3% -54.40% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 109.39MB ± 0% +3891.83% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 168.5k ± 0% +117.30% (p=0.004 n=6+5)
The reason why the decoder got twice as slow is because it now allocated
~40x as many objects, which puts a lot of pressure on the garbage
collector.
The reason is that the CL concatenated strings every time a nested field
was decoded. In other words, practically every field generated garbage
when decoded. This is hugely wasteful, especially considering that the
vast majority of JSON decoding inputs won't return UnmarshalTypeError.
Instead, use a stack of fields, and make sure to always use the same
backing array, to ensure we only need to grow the slice to the maximum
depth once.
The original CL also introduced a bug. The field stack string wasn't
reset to its original state when reaching "d.opcode == scanEndObject",
so the last field in a decoded struct could leak. For example, an added
test decodes a list of structs, and encoding/json before this CL would
fail:
got: cannot unmarshal string into Go struct field T.Ts.Y.Y.Y of type int
want: cannot unmarshal string into Go struct field T.Ts.Y of type int
To fix that, simply reset the stack after decoding every field, even if
it's the last.
Below is the original performance versus this CL. There's a tiny
performance hit, probably due to the append for every decoded field, but
at least we're back to the usual ~150MB/s.
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 13.0ms ± 1% +1.25% (p=0.009 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 149MB/s ± 1% -1.24% (p=0.009 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 2.74MB ± 0% +0.00% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 77.5k ± 0% +0.00% (p=0.002 n=6+6)
Finally, make all of these benchmarks report allocs by default. The
decoder ones are pretty sensitive to generated garbage, so ReportAllocs
would have made the performance regression more obvious.
Change-Id: I67b50f86b2e72f55539429450c67bfb1a9464b67
Reviewed-on: https://go-review.googlesource.com/c/go/+/167978
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When parsing nested object, UnmarshalTypeError does not contain actual
path to nested field in original JSON.
This commit change Field to contain the full path to that field. One
can get the Field name by stripping all the leading path elements.
Fixes#22369
Change-Id: I6969cc08abe8387a351e3fb2944adfaa0dccad2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/145218
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Calling .Interface on a struct field's reflect.Value isn't always safe.
For example, if that field is an unexported anonymous struct.
We only descended into this branch if the struct type had any methods,
so this bug had gone unnoticed for a few release cycles.
Add the check, and add a simple test case.
Fixes#28145.
Change-Id: I02f7e0ab9a4a0c18a5e2164211922fe9c3d30f64
Reviewed-on: https://go-review.googlesource.com/c/141537
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
