Some tests in go/types can still not use the typecheck helper functions
because they need a specific fileset for position information.
(We could use a single global file set for all tests to make this work.)
Change-Id: I73552b08a00f08d809c319d3d2328acee9532619
Reviewed-on: https://go-review.googlesource.com/c/go/+/461694
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Add the code to generate initorder.go but do not enable the generation
of that file for now because the generated use uses error_ which has
implications for gopls use (error_ produces a single error instead of
pultiple \t-indented follow-on errors).
Change-Id: I5cd8acdeb8845dbb4716f19cf90d88191dd4216c
Reviewed-on: https://go-review.googlesource.com/c/go/+/461692
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
The linux/sparc64 port is incomplete—it doesn't work, and it doesn't
have a builder. Now that dist supports broken ports, mark it as such.
The incomplete map was created to hide ports that aren't functional
from dist list output. Now that we have the broken port concept, it
seems largely redundant, so remove it for now.
For #56679.
Updates #28944.
Change-Id: I34bd23e913ed6d786a4d0aa8d2852f2b926fe4b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/458516
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
It's empty so far. The next CL adds linux/sparc64.
Also add -force flag to the bootstrap.bash script
so that it's possible to use it with broken ports.
For #56679.
Change-Id: I09c733d0df0a68df34fb808eae29be010a6da461
Reviewed-on: https://go-review.googlesource.com/c/go/+/458515
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This hook was added for the Go build system (x/build) to be able to set
the run flag value, but it's no longer used anywhere. Remove it for now.
Updates #46054.
Change-Id: I64e7d68d2b270303f3bd54f73079600f209e350a
Reviewed-on: https://go-review.googlesource.com/c/go/+/455519
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change makes it easier for clients to debug mutual TLS connection failures. Currently, there are a few situations where invalid client auth leads to a generic "bad certificate" alert. 3 specific situations have a more appropriate TLS alert code, based on the alert descriptions in the appendix of both RFC5246 and RFC8446.
1. The server is configured to require client auth, but no client cert was provided; the appropriate alert is "certificate required". This applies only to TLS 1.3, which first defined the certificate_required alert code.
2. The client provided a cert that was signed by an authority that is not in the server's trusted set of CAs; the appropriate alert is "unknown certificate authority".
3. The client provided an expired (or not yet valid) cert; the appropriate alert is "expired certificate".
Otherwise, we still fall back to "bad certificate".
Fixes#52113
Change-Id: I7d5860fe911cad8a1615f16bfe488a37e936dc36
GitHub-Last-Rev: 34eeab587b
GitHub-Pull-Request: golang/go#53251
Reviewed-on: https://go-review.googlesource.com/c/go/+/410496
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
With benchinit, we see a noticeable improvement in init times:
name old time/op new time/op delta
GoTypes 83.4µs ± 0% 43.7µs ± 1% -47.57% (p=0.029 n=4+4)
name old alloc/op new alloc/op delta
GoTypes 26.5kB ± 0% 18.8kB ± 0% -29.15% (p=0.029 n=4+4)
name old allocs/op new allocs/op delta
GoTypes 238 ± 0% 154 ± 0% -35.29% (p=0.029 n=4+4)
Port the same change to cmd/compile/internal/types and types2.
Updates #26775.
Change-Id: Ia1f7c4a4ce9a22d66e2aa9c9b9c341036993adca
Reviewed-on: https://go-review.googlesource.com/c/go/+/460544
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Avoid unnecessary allocations when calling reflect.TypeOf;
we can use nil pointers, which fit into an interface without allocating.
This saves about 1% of CPU time.
The builtin types are limited to typeIds between 0 and firstUserId,
and since firstUserId is 64, builtinIdToType does not need to be a map.
We can simply use an array of length firstUserId, which is simpler.
This saves about 1% of CPU time.
idToType is similar to firstUserId in that it is a map keyed by typeIds.
The difference is that it can grow with the user's types.
However, each added type gets the next available typeId,
meaning that we can use a growing slice, similar to the case above.
nextId then becomes the current length of the slice.
This saves about 1% of CPU time.
typeInfoMap is stored globally as an atomic.Value,
where each modification loads the map, makes a whole copy,
adds the new element, and stores the modified copy.
This is perfectly fine when the user registers types,
as that can happen concurrently and at any point in the future.
However, during init time, we sequentially register many types,
and the overhead of copying maps adds up noticeably.
During init time, use a regular global map instead,
which gets replaced by the atomic.Value when our init work is done.
This saves about 2% of CPU time.
Finally, avoid calling checkId in bootstrapType;
we have just called setTypeId, whose logic for getting nextId is simple,
so the extra check doesn't gain us much.
This saves about 1% of CPU time.
Using benchinit, which transforms GODEBUG=inittrace=1 data into Go
benchmark compatible output, results in a nice improvement:
name old time/op new time/op delta
EncodingGob 175µs ± 0% 162µs ± 0% -7.45% (p=0.016 n=5+4)
name old alloc/op new alloc/op delta
EncodingGob 39.0kB ± 0% 36.1kB ± 0% -7.35% (p=0.016 n=5+4)
name old allocs/op new allocs/op delta
EncodingGob 588 ± 0% 558 ± 0% -5.10% (p=0.000 n=5+4)
Updates #26775.
Change-Id: I28618e8b96ef440480e666ef2cd5c4a9a332ef21
Reviewed-on: https://go-review.googlesource.com/c/go/+/460543
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Per benchinit, this makes a big difference to init times:
name old time/op new time/op delta
InternalProfile 185µs ± 1% 6µs ± 1% -96.51% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
InternalProfile 101kB ± 0% 4kB ± 0% -95.72% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
InternalProfile 758 ± 0% 25 ± 0% -96.70% (p=0.008 n=5+5)
The fixed 0.2ms init cost is saved for any importer of net/http/pprof,
but also for cmd/compile, as it supports PGO now.
A Go program parsing profiles might not even need to compile these
regular expressions at all, if it doesn't encounter any legacy files.
I suspect this will be the case with most invocations of cmd/compile.
Updates #26775.
Change-Id: I8374dc64459f0b6bb09bbdf9d0b6c55d7ae1646e
Reviewed-on: https://go-review.googlesource.com/c/go/+/460545
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Go 1.20 will require Go 1.17 to bootstrap, so we can stop worrying about
older Go bootstrap versions. https://go.dev/issues/44505 fixed most of
those TODOs, but this one was presumably forgotten about.
Change-Id: I0c19ec4eec65cd807e7db9a57c5969845d915c07
Reviewed-on: https://go-review.googlesource.com/c/go/+/461155
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The Value type implements Aux interface because it is being used as a
"avoid clobbering flags" marker by amd64, x86 and s390x SSA parts.
Create a boolean that implements the Aux interface. Use it as the marker
instead. We no longer need Value to implement Aux.
Resolves a TODO.
See CL 275756 for more info.
Change-Id: I8a1eddf7e738b8aa31e82f3c4c590bafd2cdc56b
Reviewed-on: https://go-review.googlesource.com/c/go/+/461156
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Currently runtime.Breakpoint generates SIGSEGV in s390x.
The solution to this is add new asm instruction BRRK of
type FORMAT_E for the breakpoint exception.
Fixes#52103
Change-Id: I8358a56e428849a5d28d5ade141e1d7310bee084
Reviewed-on: https://go-review.googlesource.com/c/go/+/457456
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
With GOAMD64=V3 the canonical isPowerOfTwo function:
func isPowerOfTwo(x uintptr) bool {
return x&(x-1) == 0
}
Used to compile to:
temp := BLSR(x) // x&(x-1)
flags = TEST(temp, temp)
return flags.zf
However the blsr instruction already set ZF according to the result.
So we can remove the TEST instruction if we are just checking ZF.
Such as in multiple pieces of code around memory allocations.
This make the code smaller and faster.
Change-Id: Ia12d5a73aa3cb49188c0b647b1eff7b56c5a7b58
Reviewed-on: https://go-review.googlesource.com/c/go/+/448255
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
make\(\[\][a-zA-Z0-9]+, 0\) is seen 52 times in the go source.
And at least 391 times on internet:
https://grep.app/search?q=make%5C%28%5C%5B%5C%5D%5Ba-zA-Z0-9%5D%2B%2C%200%5C%29®exp=true
This used to compile to calling runtime.makeslice.
However we can copy what we do for []T{}, just use a zerobase pointer.
On my machine this is 10x faster (from 3ns to 0.3ns).
Note that an empty loop also runs in 0.3ns,
so this really is free when you count superscallar execution.
Change-Id: I1cfe7e69f5a7a4dabbc71912ce6a4f8a2d4a7f3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/454036
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
The number 104 appears to date back to the
first implementation of split stacks in
https://go.googlesource.com/go/+/b987f7a757f53f460973622a36eebb696f9b5060.
That change introduces a 104 byte stack guard.
it doesn't makes any sense today.
Change-Id: I73069f6d1a827653af63e616f0119fbac809882e
GitHub-Last-Rev: bcf9000590
GitHub-Pull-Request: golang/go#56594
Reviewed-on: https://go-review.googlesource.com/c/go/+/448036
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Convert the scheduling pass from scheduling backwards to scheduling forwards.
Forward scheduling makes it easier to prioritize scheduling values as
soon as they are ready, which is important for things like nil checks,
select ops, etc.
Forward scheduling is also quite a bit clearer. It was originally
backwards because computing uses is tricky, but I found a way to do it
simply and with n lg n complexity. The new scheme also makes it easy
to add new scheduling edges if needed.
Fixes#42673
Update #56568
Change-Id: Ibbb38c52d191f50ce7a94f8c1cbd3cd9b614ea8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/270940
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is the second round to look for spelling mistakes. This time the
manual sifting of the result list was made easier by filtering out
capitalized and camelcase words.
grep -r --include '*.go' -E '^// .*$' . | aspell list | grep -E -x '[A-Za-z]{1}[a-z]*' | sort | uniq
This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.
Change-Id: Ie8a2092aaa7e1f051aa90f03dbaf2b9aaf5664a9
GitHub-Last-Rev: fc2bd6e0c5
GitHub-Pull-Request: golang/go#57737
Reviewed-on: https://go-review.googlesource.com/c/go/+/461595
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This useConstraintTypeInference flag was debugging purposes only and
is not needed anymore. It's already gone in go/types.
Also, adjust/fix some comments.
Change-Id: I713be5759f05c618fcf26e16cf53dfb3626bba93
Reviewed-on: https://go-review.googlesource.com/c/go/+/461690
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I63181f6540fc1bfcfc988a16bf9fafbd3575cfdf
GitHub-Last-Rev: d90528730a
GitHub-Pull-Request: golang/go#57909
Reviewed-on: https://go-review.googlesource.com/c/go/+/462284
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Change-Id: Ie46bc3cbfb2622b5eb70618557ff5398866f5607
GitHub-Last-Rev: a665ef84dd
GitHub-Pull-Request: golang/go#57813
Reviewed-on: https://go-review.googlesource.com/c/go/+/462044
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Change-Id: Ibd519ed6419f8f21c89a111a0326d0788aca9d19
GitHub-Last-Rev: 45e3224f9a
GitHub-Pull-Request: golang/go#57819
Reviewed-on: https://go-review.googlesource.com/c/go/+/462046
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
The pconn write loop closes a request's body after sending the
request, but in the case where the write loop exits with an
unsent request in writech the body is never closed.
Close the request body in this case.
Fixes#49621
Change-Id: Id94a92937bbfc0beb1396446f4dee32fd2059c7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461675
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In CL 408734 we introduced a fall back to base offset 0 if reading a
directory entry at the computed base offset failed. We have now found
a file in the wild for which the computed base offset is incorrect,
but happens to refer to a valid directory entry. In this CL, we change
the fallback such that if the first directory header relative to base
offset 0 is valid, we just use base offset 0.
Change-Id: Ia9ace20c1065d1f651035f16f7d91d741ab1dbf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/461598
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Current RISCV64 assembler do not check the invalid shift amount. This CL
adds the check to avoid generating invalid instructions.
Fixes#57755
Change-Id: If33877605e161baefd98c50db1f71641ca057507
Reviewed-on: https://go-review.googlesource.com/c/go/+/461755
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
We need to make sure that when we get the stack pointer, we get it
at the right time.
V = GetCallerSP
Call()
W = GetCallerSP
If Call causes a stack growth, then we will be in a situation
where V != W. So it matters when GetCallerSP operations get scheduled.
Add a memory argument to GetCallerSP so it can't be reordered with
things like calls.
Change-Id: I6cc801134c38e358c5a1ec0c09d38379a16a4184
Reviewed-on: https://go-review.googlesource.com/c/go/+/453515
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Martin Möhrmann <martin@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Seeing the Node.js version that was used during a particular test run
should be helpful during the upcoming migration from Node.js 14 to 18.
Add minimal support for that.
For golang/go#57614.
Change-Id: Id55ba25a7ee4a803788316d4a646cd4b6f4297e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/460655
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Richard Musiol <neelance@gmail.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
The SPanchored opcode is identical to SP, except that it takes a memory
argument so that it (and more importantly, anything that uses it)
must be scheduled at or after that memory argument.
This opcode ensures that a LEAQ of a variable gets scheduled after the
corresponding VARDEF for that variable.
This may lead to less CSE of LEAQ operations. The effect is very small.
The go binary is only 80 bytes bigger after this CL. Usually LEAQs get
folded into load/store operations, so the effect is only for pointerful
types, large enough to need a duffzero, and have their address passed
somewhere. Even then, usually the CSEd LEAQs will be un-CSEd because
the two uses are on different sides of a function call and the LEAQ
ends up being rematerialized at the second use anyway.
Change-Id: Ib893562cd05369b91dd563b48fb83f5250950293
Reviewed-on: https://go-review.googlesource.com/c/go/+/452916
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Martin Möhrmann <martin@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
The standard way to generate code in a Go package is via //go:generate
directives, which are invoked by the developer explicitly running:
go generate import/path/of/said/package
Switch to using that approach here.
This way, developers don't need to learn and remember a custom way that
each particular Go package may choose to implement its code generation.
It also enables conveniences such as 'go generate -n' to discover how
code is generated without running anything (this works on all packages
that rely on //go:generate directives), being able to generate multiple
packages at once and from any directory, and so on.
Change-Id: I0e5b6a1edeff670a8e588befeef0c445613803c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/460135
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Allow GODEBUG users to report how many times a setting
resulted in non-default behavior.
Record non-default-behaviors for all existing GODEBUGs.
Also rework tests to ensure that runtime is in sync with runtime/metrics.All,
and generate docs mechanically from metrics.All.
For #56986.
Change-Id: Iefa1213e2a5c3f19ea16cd53298c487952ef05a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/453618
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for #56986.
Fixes#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
modindex defines its own type Package that must be exactly the
same as build.Package (there are conversions between them).
The normal reason to do this is to provide a different method set,
but there aren't any different methods. And if we needed to do that,
we could write
type Package build.Package
instead of repeating the struct definition. Remove the type entirely
in favor of direct use of build.Package.
This makes the modindex package not break when fields are
added to go/build.Package.
Change-Id: I8ffe9f8832bbc62be93a72e6a09d807ddbce216b
Reviewed-on: https://go-review.googlesource.com/c/go/+/462255
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
test_shuffle was added in CL 310033. It takes about 4½ seconds on my
workstation prior to this CL, most of which is spent linking and
running test binaries in 'go test'.
We can reduce that time somewhat (to 3¾ seconds) by simply running the
test fewer times (cases of 'off', 'on', positive, zero, and negative
values seem sufficient), but we should also avoid that linking
overhead at all in short mode.
Fixes#57709.
Change-Id: I908a70435ccfb1ca16ed23aec17512bf2b267b21
Reviewed-on: https://go-review.googlesource.com/c/go/+/461455
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
The mustBlock helper returns a function that verifies that the blocked
operation does eventually complete. However, it used an arbitrary
10-second timeout in place of “eventually”.
Since the test is checking a synchronization library, bugs are likely
to manifest as deadlocks. It may be useful to log what operation is in
flight if such a deadlock occurs; however, since we can't bound how
long a “reasonable” operation should take, the log message should only
be informational — it should not cause the test to fail.
While we're here, let's also set a better example by not leaking
time.After timers in the tests..
Fixes#57592.
Change-Id: I4e74e42390679bffac7a286824acb71b08994c17
Reviewed-on: https://go-review.googlesource.com/c/go/+/461000
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
This uses the new Cancel and WaitDelay fields of os/exec.Cmd
(added in #50436) to interrupt or kill the 'hg serve' command
when its incoming http.Request is canceled.
This should keep the vcweb hg handler from getting stuck if 'hg serve'
hangs after the request either completes or is canceled.
Fixes#57597 (maybe).
Change-Id: I53cf58e8ab953fd48c0c37f596f99e885a036d9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/460997
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This test is flaky, apparently due to a typo'd operator in CL 21447
that causes it to compare “same port OR IP” instead of
“same port AND IP”.
If we merely fixed the comparison, the test would hopefully stop being
flaky itself, but we would still be left with another problem:
repeatedly dialing a port that we believe to be unused can interfere
with other tests, which may open the previously-unused port and then
attempt a single Dial and expect it to succeed. Arbitrary other Dial
calls for that port may cause the wrong connection to be accepted,
leading to spurious test failures.
Moreover, the test can be extremely expensive for the amount of data
we hope to get from it, depending on the system's port-reuse
algorithms and dial implementations. It is already scaled back by up
to 1000x on a huge number of platforms due to latency, and may even be
ineffective on those platforms because of the arbitrary 1ms Dial
timeout. And the incremental value from it is quite low, too: it tests
the workaround for what is arguably a bug in the Linux kernel, which
ought to be fixed (and tested) upstream instead of worked around in
every open-source project that dials local ports.
Instead of trying to deflake this test, let's just get rid of it.
Fixes#18290.
Change-Id: I8a58b93d67916a33741c9ab29ef99c49c46b32c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/460657
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
These tests were only run on GOARCH=amd64, but the rationale given in
CL 11858043 was GC precision on 32-bit platforms. Today, we have far
more 64-bit platforms than just amd64, and I believe that GC precision
on 32-bit platforms has been substantially improved as well.
The GOARCH restriction seems unnecessary.
Updates #57166.
Updates #5368.
Change-Id: I45c608b6fa721012792c96d4ed94a6d772b90210
Reviewed-on: https://go-review.googlesource.com/c/go/+/456120
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Austin Clements <austin@google.com>
- Use testenv.Command instead of exec.Command to try to get more
useful timeout behavior.
- Parallelize tests that appear not to require global state.
(And add explanatory comments for a few that are not
parallelizable for subtle reasons.)
- Consolidate some “Helper” tests with their parent tests.
- Use t.TempDir instead of os.MkdirTemp when appropriate.
- Factor out subtests for repeated test helpers.
For #36107.
Updates #22315.
Change-Id: Ic24b6957094dcd40908a59f48e44c8993729222b
Reviewed-on: https://go-review.googlesource.com/c/go/+/458015
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
- Consolidate the two test bodies as one helper function.
- Eliminate arbitrary timeout.
- Shorten arbitrary sleeps in short mode.
- Simplify goroutines.
- Mark the tests as parallel.
Fixes#36107.
Updates #24164.
Change-Id: I14fe4395963a7256cb6d2d743d348a1ade077d5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/457336
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Previously if the default go toolchain (from "where go") contained spaces, then running make.bat would fail.
Fixes it to correctly treat paths with spaces. This is especially useful given that the default Go install path is under "C:\Program Files".
Fixes#57918
Change-Id: Icacf8dd5178b608225f02e4a11a8753d78bed262
GitHub-Last-Rev: 4cd8a790e4
GitHub-Pull-Request: golang/go#57836
Reviewed-on: https://go-review.googlesource.com/c/go/+/462275
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
CL 444277 fixed Time.UnmarshalText and Time.UnmarshalJSON to properly
unmarshal timestamps according to RFC 3339 instead of according
to Go's bespoke time syntax that is a superset of RFC 3339.
However, this change seems to have broken an AWS S3 unit test
that relies on parsing timestamps with single digit hours.
It is unclear whether S3 emits these timestamps in production or
whether this is simply a testing artifact that has been cargo culted
across many code bases. Either way, disable strict parsing for now
and re-enable later with better GODEBUG support.
Updates #54580
Change-Id: Icced2c7f9a6b2fc06bbd9c7e90f90edce24c2306
Reviewed-on: https://go-review.googlesource.com/c/go/+/462286
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Import x/tools as of CL 462596 (070db2996ebe, Jan 18 2022),
to bring in two vet analysis fixes (printf and loopclosure).
For #57911.
Fixes#57903.
Fixes#57904.
Change-Id: I82fe4e9bd56fb8e64394ee8618c155316942a517
Reviewed-on: https://go-review.googlesource.com/c/go/+/462555
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Tests must not write to GOROOT: it might not writable (for example, if
it is owned by root and the user is non-root), and in general we can't
assume that the configuration in which the test is run matches the
configuration with which the installed tools were built.
In this specific case, CL 454836 (for #57007) installs 'cmd' with
CGO_ENABLED=0, but most builders still run the tests with CGO_ENABLED
unset.
Updates #57007.
Change-Id: I2795fcd3ff61c164dc730b62f697f307ab3a167b
Reviewed-on: https://go-review.googlesource.com/c/go/+/461689
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Follow-up on CL 461687 which missed the go/types change.
(As an aside, we cannot yet generate this change because
go/types uses a positioner and types2 just uses a syntax.Pos.)
Change-Id: I28113a2efdc3ddd30cb9a80d2cb2c802ff035ec2
Reviewed-on: https://go-review.googlesource.com/c/go/+/461688
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
