In order to generate more accurate or informative error messages from
the type checker, it can be helpful to interpret error messages in
context. This is currently achieved in a number of ways:
+ Return a boolean value, and then reverse-engineer the error at the
callsite (as in representable->representableConst).
+ Return a value causing the error (as in Checker.missingMethod), and
add the error at the callsite.
+ Pass a "reason" string pointer to capture the error (as in
Checker.assignableTo), and add the error at the callsite.
+ Pass a "context" string pointer, and use this when writing errors in
the delegated method.
In all cases, it is the responsibility of whatever code calls
Checker.error* to set the operand mode to invalid.
These methods are used as appropriate, depending on whether multiple
errors are generated, whether additional context is needed, and whether
the mere presence of an error needs to be interpreted at the callsite.
However, this practice has some downsides: the plurality of error
handling techniques can be a barrier to readability and composability.
In this CL, we introduce Yet Another Pattern, with the hope that it can
replace some or all of the existing techniques: factor out side-effect
free functions that evaluate a single error, and add helpers for
recording this error in the Checker.
As a proof of concept this is done for Checker.representable and
Checker.convertUntyped. If the general pattern does not seem appropriate
for replacing some or all of the error-handling techniques listed above,
we should revert to an established technique.
Some internal error APIs are refactored to operate on an error, rather
than a types.Error, with internal error metadata extracted using
errors.As. This seemed to have negligible impact on performance, but we
should be careful about actually wrapping errors: I expect that many
users will expect err to be a types.Error.
Change-Id: Ic5c6edcdc02768cd84e04638fad648934bcf3c17
Reviewed-on: https://go-review.googlesource.com/c/go/+/242082
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts CL 232799.
Reason for revert: net/http test is failing on all longtest builders.
Change-Id: I4694e34f35419bab2d0b45fa6d8c3ac2aa1f51a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/250597
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix a data race for clients that mutate requests after receiving a
response error which is caused by the writeLoop goroutine left
running, this can be seen on canceled requests.
Fixes#37669
Change-Id: I0e0e4fd63266326b32587d8596456760bf848b13
Reviewed-on: https://go-review.googlesource.com/c/go/+/232799
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Old behavior is still enabled because it doesn't hurt to leave
it in and existing users of this feature (there are dozens of
us!) will not be surprised. Adding this finer control allows
users to avoid writing ssa.html where they can't, shouldn't, or
just don't want to.
Example, both ways:
$ GOSSAFUNC="(*Reader).Reset" go test -c -o ./a compress/gzip
dumped SSA to bytes/ssa.html
dumped SSA to strings/ssa.html
dumped SSA to bufio/ssa.html
dumped SSA to compress/gzip/ssa.html
$ GOSSAFUNC="compress/gzip.(*Reader).Reset" go test -c -o ./a compress/gzip
dumped SSA to compress/gzip/ssa.html
Updates #40919.
Change-Id: I06b77c3c1d326372a32651570b5dd6e56dfb1d7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/250340
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
err != nil is already checked in the if condition one line above.
Change-Id: If36cdb41016f7be98a65be0a7211d85cd6017f87
Reviewed-on: https://go-review.googlesource.com/c/go/+/250477
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The StripPrefix wrapper strips a prefix string from the request's
URL.Path field, but doesn't touch the RawPath field. This leads to the
confusing situation when StripPrefix handles a request with URL.RawPath
populated (due to some escaped characters in the request path) and the
wrapped request's RawPath contains the prefix but Path does not.
This change modifies StripPrefix to strip the prefix from both Path and
RawPath. If there are escaped characters in the prefix part of the
request URL the stripped handler serves a 404 instead of invoking the
underlying handler with a mismatched Path/RawPath pair.
This is a backward incompatible change for a very small minority of
requests; I would be surprised if anyone is depending on this behavior,
but it is possible. If that's the case, we could make a more
conservative change where the RawPath is trimmed if possible, but when
the prefix contains escaped characters then we don't 404 but rather send
through the invalid Path/RawPath pair as before.
Fixes#24366
Change-Id: I7030b8c183a3dfce307bc0272bba9a18df4cfe08
Reviewed-on: https://go-review.googlesource.com/c/go/+/233637
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Previously, Readdirnames returned a *PathError on Windows and Plan 9,
but a *SyscallError on POSIX systems.
In contrast, similar methods (such as Stat) return a *PathError on all platforms.
Fixes#38923
Change-Id: I26395905b1e723933f07b792c7aeee7c335949cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/233917
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Right now we just prevent such types from being on the heap. This CL
makes it so they cannot appear on the stack either. The distinction
between heap and stack is pretty vague at the language level (e.g. it
is affected by -N), and we don't need the flexibility anyway.
Once go:notinheap types cannot be in either place, we don't need to
consider pointers to such types to be pointers, at least according to
the garbage collector and stack copying. (This is the big win of this
CL, in my opinion.)
The distinction between HasPointers and HasHeapPointer no longer
exists. There is only HasPointers.
This CL is cleanup before possible use of go:notinheap to fix#40954.
Update #13386
Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a
Reviewed-on: https://go-review.googlesource.com/c/go/+/249917
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
par.Work performs two different tasks: deduplicating work (a task
which overlaps with par.Cache), and executing limited active work in
parallel. It also requires the caller to re-invoke Do whenever the
workqueue transititions from empty to non-empty.
The new par.Queue only performs the second of those two tasks, and
presents a simpler API: it starts and stops its own goroutines as
needed (indicating its idle state via a channel), rather than
expecting the caller to drive the transitions explicitly.
For #36460
Change-Id: I5c38657dda63ab55718497467d05d41744ff59f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/247766
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Previously they were cached per mvsReqs instance. However, the
contents of the go.mod file of a given dependency version can only
vary if the 'replace' directives that apply to that version have
changed, and the only time we change 'replace' directives is in 'go
mod edit' (which does not care about the build list or MVS).
This not only simplifies the mvsReqs implementation, but also makes
more of the underlying logic independent of mvsReqs.
For #36460
Change-Id: Ieac20c2fcd56f64d847ac8a1b40f9361ece78663
Reviewed-on: https://go-review.googlesource.com/c/go/+/244774
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Previously, this cache was a member of the (ephemeral) modload.loader
struct. However, the Go language version for a given module version
does not vary based on the build list, the set of loaded packages, the
build tags in use, the meaning of the "all" pattern, or anything else
that can be configured for an instance of the package loader. The map
containing that information is therefore not appropriate as a field of
the (configurable, package-list-dependent) loader struct.
The Go language version mapping could, in theory, be read from the
go.mod file in the module cache (or replacement directory) every time
it is needed: this map is just a cache, and as such it belongs
alongside the other caches and indexes in the modload package, which
are currently found in modfile.go.
We may want to do the same sort of global caching for the mapping from
each module.Version to its list of direct requirements (which are
similarly idempotent), but for now that is left for a future change.
For #36460
For #36876
Change-Id: I90ac176ffea97f30c47d6540c3dfb874dc9cfa4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/244078
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Previously, we suppressed the module version annotation if the last
error in the stack was a *module.ModuleError, regardless of its path.
However, if the error is for a replacement module, that produces a
confusing error message: the error is attributed to the last module in
the error path, but actually originates in the replacement (which is
not otherwise indicated).
Now, we print both the original and the replacement modules when they
differ, which may add some unfortunate redundancy in the output but at
least doesn't drop the very relevant information about replacements.
Fixes#35039
Change-Id: I631a7398033602b1bd5656150a4fad4945a87ade
Reviewed-on: https://go-review.googlesource.com/c/go/+/247765
Reviewed-by: Jay Conrod <jayconrod@google.com>
Also factor out BuildListError to a separate file.
For #36460
Change-Id: Ibd1143893b09a2bbef659bea1e8c5dd35184a7ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/247764
Reviewed-by: Jay Conrod <jayconrod@google.com>
When we print the stack from a BuildListError, we print the main
module first and the error last. That was the opposite of the order in
which in was stored in memory, leading to (arguably) more complex code
and (definitely) my own inability to reason about the contents of the
slice.
For now, it's still more convenient to construct the stack reversed,
so we do that and then reverse it before packing it into the error.
For #36460
Change-Id: I6312fb67b2ad9bf9b64071fe829854833208bad7
Reviewed-on: https://go-review.googlesource.com/c/go/+/244759
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Previously, when we encountered an excluded version in any module's
requirements, we would resolve it to the next higher version.
Unfortunately, the meaning of “the next higher version” can change
over time.
Moreover, users who use 'exclude' directives normally either already
require some higher version (using the 'exclude' directive to prune
out invalid requirements from some intermediate version), or already
require some lower version (using the 'exclude' directive to prevent
'go get -u' from upgrading to a known-bad version). In both of these
cases, resolving an upgrade for the excluded version is needless work
even in the best case: it adds work for the 'go' command when there is
already a perfectly usable selected version of the module in the
requirement graph.
Instead, we now interpret the 'exclude' directive as dropping all
references to the excluded version.
This implements the approach described in
https://golang.org/issue/36465#issuecomment-572694990.
Fixes#36465
Updates #36460
Change-Id: Ibf0187daced417b4cc23b97125826778658e4b0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/244773
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This allows semver-based comparisons of the version without additional allocations.
Also comment on the reason for the loops that iterate over modFile instead.
(I was reading the vendor code in order to add the lazy-loading version check,
and this section was a bit unclear to me.)
For #36460
Change-Id: I11559d81ffb4eba0e4e10e6fa3c01990b11f9180
Reviewed-on: https://go-review.googlesource.com/c/go/+/240622
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Sets Content-Length:0 for nil bodies in PATCH requests, as we already do for POST and PUT requests.
RFC 2616 mentions that unless a method’s Content-Length is forbidden it can send one.
In the wild, we’ve found that Microsoft Azure’s DataLake Gen2 storage API https://docs.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/update deliberately rejects PATCH requests without a Content-Length, yet there is no workaround for setting that header when trying to flush the content of a file which was uploaded in a previous request.
Fixes#40978
Change-Id: Ib0a623b907d827a1c5ee431dca3c41024fa291c5
GitHub-Last-Rev: 12a3903f2b
GitHub-Pull-Request: golang/go#40991
Reviewed-on: https://go-review.googlesource.com/c/go/+/250039
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Due to a typo, this test case was not actually exercising the bug
described in golang/go#33656. Update it to do so. Interestingly, the
comparison is now valid (as it should be) -- I suspect #33656 is
actually fixed.
Fixes#33656
Change-Id: If50a917f6477d8eb4f82f5a2a96bf5d9123ff0d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/241263
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
These exported functions are mostly trivial wrappers, but do make
certain assumptions about how the underlying Checker APIs can be called.
Add some simple tests.
Change-Id: I68e9ae875353c12d118ec961a6f3834385fbbb97
Reviewed-on: https://go-review.googlesource.com/c/go/+/241262
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
With this patch, opt pass can expose more obvious constant-folding
opportunites.
Example:
func test(i int) int {return (i+8)-(i+4)}
The previous version:
MOVD "".i(FP), R0
ADD $8, R0, R1
ADD $4, R0, R0
SUB R0, R1, R0
MOVD R0, "".~r1+8(FP)
RET (R30)
The optimized version:
MOVD $4, R0
MOVD R0, "".~r1+8(FP)
RET (R30)
This patch removes some existing reassociation rules, such as "x+(z-C)",
because the current generic rewrite rules will canonicalize "x-const"
to "x+(-const)", making "x+(z-C)" equal to "x+(z+(-C))".
This patch also adds test cases.
Change-Id: I857108ba0b5fcc18a879eeab38e2551bc4277797
Reviewed-on: https://go-review.googlesource.com/c/go/+/237137
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This patch adds the ARM6464Bitfield auxInt to auxIntType() and
returns its Go type as "arm64Bitfield" type, which is defined
as int16 type.
And the Go type of SymOff auxInt is int32, but some functions
(such as min(), areAdjacentOffsets() and read16/32/64(),etc.)
use SymOff as an input parameter and treat its type as int64,
this patch adds the type conversion for these rules.
Passes toolstash-check -all.
Change-Id: Ib234b48d0a97ef244dd37878e06b5825316dd782
Reviewed-on: https://go-review.googlesource.com/c/go/+/234378
Reviewed-by: Keith Randall <khr@golang.org>
Add a new conversion function to convert aux type to Op type.
Passes toolstash-check -all.
Change-Id: I25d649a5296f6f178d64320dfc5d291e0a597e24
Reviewed-on: https://go-review.googlesource.com/c/go/+/233739
Reviewed-by: Keith Randall <khr@golang.org>
Add a check code for checking whether the "c" value can be
represented as a signed 32 bit integer in some rules.
Passes toolstash-check -all.
Change-Id: I84e4431dc75945985695516d34565b841c8b53e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/233738
Reviewed-by: Keith Randall <khr@golang.org>
CL 249962 added wasm StorepNoWB implementation in assembly, it's now
like all other architectures. This CL adds a general test that the
second param of StorepNoWB must be force to escape.
Fixes#40975
Change-Id: I1eccc7e50a3ec742a1912d65f25b15f9f5ad9241
Reviewed-on: https://go-review.googlesource.com/c/go/+/249761
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Turns out if your failure is in a function with a name like "Reset()"
there will be a lot of hits on the same hashcode. Adding package sensitivity
solves this problem.
In additionm, it turned out that in the case that a logfile was specified
for the GOSSAHASH logging, that it was opened in create mode, which meant
that multiple compiler invocations would reset the file to zero length.
Opening in append mode works better; the automated harness
(github.com/dr2chase/gossahash) takes care of truncating the file before use.
Change-Id: I5601bc280faa94cbd507d302448831849db6c842
Reviewed-on: https://go-review.googlesource.com/c/go/+/246937
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The second argument of StorepNoWB must be forced to escape.
The current Go code does not explicitly enforce that property.
By implementing in assembly, and not using go:noescape, we
force the issue.
Test is in CL 249761. Issue #40975.
This CL is needed for CL 249917, which changes how go:notinheap
works and breaks the previous StorepNoWB wasm code.
I checked for other possible errors like this. This is the only
go:notinheap that isn't in the runtime itself.
Change-Id: I43400a806662655727c4a3baa8902b63bdc9fa57
Reviewed-on: https://go-review.googlesource.com/c/go/+/249962
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
More ergonomic that way. Also change Haspointers to HasPointers
while we are here.
Change-Id: I45bedc294c1a8c2bd01dc14bd04615ae77555375
Reviewed-on: https://go-review.googlesource.com/c/go/+/249959
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
go1.14 drop nacl support, as go1.15 was released, go1.13 is not
supported anymore, nacl is absolutely gone.
Change-Id: I05efb46891ec875b08da8f2996751a8e9cb57d0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/249977
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Current peFile.goarch looks for symbols like "_rt0_386_windows" to
determine GOARCH. But "_rt0_386_windows" is not present in executables
built with cgo.
Use pe.FileHeader.Machine instead. This should work with any Windows
executable, not just with Go built executable.
Fixes#39682
Change-Id: Ie0ffce664f4b8b8fed69b2ecc482425b042a38d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/240957
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
In
type T1 struct { t2 }
type t2 int
func (t2) M()
T1 has method M because it embeds t2, which has M. Classify
the example
func ExampleT1_M
with T1 instead of ignoring it, as is done currently. There is no
other way to provide an example for such a method, since its original
type is unexported.
Continue to ignore examples on methods from embedded types that are
exported, unless in AllMethods mode. Examples for those methods could
be written on the original type.
The change involves removing a check in classifyExamples. The check
isn't necessary to get the above behavior because
reader.collectEmbeddedMethods and sortedFuncs already generate the
appropriate list of methods.
For #40172.
Change-Id: Ibe7d965ecba6426466184e6e6655fc05989e9caf
Reviewed-on: https://go-review.googlesource.com/c/go/+/249557
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Implementing the suggestion made by bcmills on a comment on golang.org/cl/228783.
Change-Id: I314a24a002c65b582ea51610dcc1a54a69afbb8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229705
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Add information that error comes from parsing module proxy responses.
Fixes#38680
Change-Id: Ic318b9cdbca789c1b0d983e083e692a914892fbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/233437
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The documentation refers to a non longer existing flag (-seq).
Remove those references.
Change-Id: I480b6259f9199b47761dc655a90911eabfe07427
Reviewed-on: https://go-review.googlesource.com/c/go/+/249738
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
checkptr has code to recognize &^ expressions, but it didn't take into
account that "p &^ x" gets rewritten to "p & ^x" during walk, which
resulted in false positive diagnostics.
This CL changes walkexpr to mark OANDNOT expressions with Implicit
when they're rewritten to OAND, so that walkCheckPtrArithmetic can
still recognize them later.
It would be slightly more idiomatic to instead mark the OBITNOT
expression as Implicit (as it's a compiler-generated Node), but the
OBITNOT expression might get constant folded. It's not worth the extra
complexity/subtlety of relying on n.Right.Orig, so we set Implicit on
the OAND node instead.
To atone for this transgression, I add documentation for nodeImplicit.
Fixes#40917.
Change-Id: I386304171ad299c530e151e5924f179e9a5fd5b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/249477
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.
Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.
Fixes#40473.
Change-Id: I9b97c494afaae2d5ed6bca4cd428b4132b5f8133
Reviewed-on: https://go-review.googlesource.com/c/go/+/249448
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
