Michael Matloob [Tue, 8 Feb 2022 21:45:17 +0000 (16:45 -0500)]
cmd/go: mention go.work when local path outside modules in go.work
In workspace mode, if a user lists a package or patternthat's inside a
module that's not listed in go.work, mention that the package or pattern
is outside the modules listed in go.work so the user has a better idea
of how to fix the issue.
(Question: it's valid in those flows to add a pattern that points into
the module cache. Should we expand the error to say "package outside
modules listed in go.work file or contained in module cache"? That seems
clunky (and is the uncommon case) which is why I didn't do so in this
case, but it's possible)
Fixes #49632
Change-Id: I3f0ea1b2f566d52a8079b58593fcc5cc095e7a41
Reviewed-on: https://go-review.googlesource.com/c/go/+/384236
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Tue, 8 Feb 2022 00:52:11 +0000 (00:52 +0000)]
runtime: make piController much more defensive about overflow
If something goes horribly wrong with the assumptions surrounding a
piController, its internal error state might accumulate in an unbounded
manner. In practice this means unexpected Inf and NaN values.
Avoid this by identifying cases where the error overflows and resetting
controller state.
In the scavenger, this case is much more likely. All that has to happen
is the proportional relationship between sleep time and estimated CPU
usage has to break down. Unfortunately because we're just measuring
monotonic time for all this, there are lots of ways it could happen,
especially in an oversubscribed system. In these cases, just fall back
on a conservative pace for scavenging and try to wait out the issue.
In the pacer I'm pretty sure this is impossible. Because we wire the
output of the controller to the input, the response is very directly
correlated, so it's impossible for the controller's core assumption to
break down.
While we're in the pacer, add more detail about why that controller is
even there, as well as its purpose.
Finally, let's be proactive about other sources of overflow, namely
overflow from a very large input value. This change adds a check after
the first few operations to detect overflow issues from the input,
specifically the multiplication.
No tests for the pacer because I was unable to actually break the
pacer's controller under a fuzzer, and no tests for the scavenger because
it is not really in a testable state.
However:
* This change includes a fuzz test for the piController.
* I broke out the scavenger code locally and fuzz tested it, confirming
that the patch eliminates the original failure mode.
* I tested that on a local heap-spike test, the scavenger continues
operating as expected under normal conditions.
Fixes #51061.
Change-Id: I02a01d2dbf0eb9d2a8a8e7274d4165c2b6a3415a
Reviewed-on: https://go-review.googlesource.com/c/go/+/383954 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bryan C. Mills [Wed, 9 Feb 2022 16:04:01 +0000 (11:04 -0500)]
runtime/pprof: remove arbitrary sleeps in TestBlockProfile
The "block" helpers in TestBlockProfile previously slept for an
arbitrary duration and assumed that that duration was long enough for
the parent goroutine to have registered as blocking. However —
especially on slow or overloaded builders — the current arbitrary
duration is sometimes not quite long enough.
Rather than increasing the duration to a different arbitrary value
(which would make the test slower but not actually eliminate the
possibility of flakes!), we can use the runtime's own accounting to
detect when the goroutine is actually blocked: we obtain a goroutine
dump from the runtime, and assume that blocking has been registered in
the profile only if the runtime shows the test goroutine in the
appropriate blocked state.
That not only makes the test more reliable, but also makes it
significantly lower-latency when run on a fast machine.
Russ Cox [Wed, 2 Feb 2022 21:41:32 +0000 (16:41 -0500)]
regexp/syntax: reject very deeply nested regexps in Parse
The regexp code assumes it can recurse over the structure of
a regexp safely. Go's growable stacks make that reasonable
for all plausible regexps, but implausible ones can reach the
“infinite recursion?” stack limit.
This CL limits the depth of any parsed regexp to 1000.
That is, the depth of the parse tree is required to be ≤ 1000.
Regexps that require deeper parse trees will return ErrInternalError.
A future CL will change the error to ErrInvalidDepth,
but using ErrInternalError for now avoids introducing new API
in point releases when this is backported.
Fixes #51112.
Change-Id: I97d2cd82195946eb43a4ea8561f5b95f91fb14c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/384616
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Michael Anthony Knyszek [Fri, 21 Jan 2022 06:52:43 +0000 (06:52 +0000)]
runtime: simplify histogram buckets considerably
There was an off-by-one error in the time histogram buckets calculation
that caused the linear sub-buckets distances to be off by 2x.
The fix was trivial, but in writing tests I realized there was a much
simpler way to express the calculation for the histogram buckets, and
took the opportunity to do that here. The new bucket calculation also
fixes the bug.
Fixes #50732.
Change-Id: Idae89986de1c415ee4e148f778e0e101ca003ade
Reviewed-on: https://go-review.googlesource.com/c/go/+/380094 Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Robert Griesemer [Wed, 9 Feb 2022 22:16:05 +0000 (14:16 -0800)]
go/types, types2: rename structuralType/String to coreType/String
This is a pure rename of the respective Go functions/methods
with corresponding adjustments to error messages and tests.
A couple of comments were manually rephrased.
With this change, the implementation and error messages match
the latest spec.
No functionality change.
Change-Id: Iaa92a08b64756356fb2c5abdaca5c943c9105c96
Reviewed-on: https://go-review.googlesource.com/c/go/+/384618
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Tamir Duberstein [Thu, 27 Jan 2022 15:29:04 +0000 (15:29 +0000)]
syscall: use RLIMIT_CPU instead of RLIMIT_NOFILE
The latter is subject to kern.maxfilelimit restrictions on darwin which
are not reflected in the return value. This makes it difficult to
reliably restore the default after the test is complete. RLIMIT_CPU
should hopefully sidestep this problem.
Updates #40564.
Change-Id: Ifb33c7d46f2708130cef366dc245c643a2d5e465
Reviewed-on: https://go-review.googlesource.com/c/go/+/383234
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Bryan Mills <bcmills@google.com>
Robert Griesemer [Wed, 9 Feb 2022 20:43:21 +0000 (12:43 -0800)]
spec: document behavior of generic type switch cases
Fixes #51110.
Change-Id: I11370417f1ef435b05dfab18eeabc2c3c1b7b8a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/384674
Trust: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com> Reviewed-by: Dan Scales <danscales@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Wed, 9 Feb 2022 02:40:28 +0000 (18:40 -0800)]
spec: the type of a constant cannot be a type parameter
Add corresponding rules and a couple of examples.
Fixes #50202.
Change-Id: I4287b5e2d0fd29a0c871795e07f1bb529c9c6004
Reviewed-on: https://go-review.googlesource.com/c/go/+/384240
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Griesemer [Tue, 8 Feb 2022 18:41:27 +0000 (10:41 -0800)]
spec: use "core type" rather than "structural type"
This change in terminology prevents potential confusion
that migth be caused by associating "structural type"
with "structural typing"; the two are not connected.
Also, adjusted introductory paragraph of section on
constraint type inference: type inference goes in both
directions, from type parameter to core type and vice
versa. The previous description was not quite accurate.
Change-Id: If4ca300f525eea660f68486302619aa6ad5dbc2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/384238
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Eric Lagergren [Wed, 9 Feb 2022 06:38:28 +0000 (22:38 -0800)]
crypto/aes: fix key size typo
AES-196 does not exist, but AES-192 does.
Signed-off-by: Eric Lagergren <eric@ericlagergren.com>
Change-Id: I8c9ac67735e99e5b2ee7fb9824029c1164221153
Reviewed-on: https://go-review.googlesource.com/c/go/+/384374
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Bryan C. Mills [Tue, 8 Feb 2022 22:09:40 +0000 (17:09 -0500)]
net: fix a race in TestLookupContextCancel
If the actual DNS lookup in LookupIPAddr completes quickly enough,
it may succeed even if the passed-in Context is already canceled.
That would (rarely) cause TestLookupContextCancel to fail due to an
unexpectedly-nil error.
This change uses the existing testHookLookupIP hook to delay the
cancellation until the lookup has started (to try to provoke the code
path for which the test was added), and then block the lookup result
until LookupIPAddr has noticed it.
Bryan C. Mills [Tue, 8 Feb 2022 17:23:50 +0000 (12:23 -0500)]
runtime/debug: replace (*BuildInfo).Marshal methods with Parse and String
Since a String method cannot return an error, escape fields that may
contain unsanitized values, and unescape them during parsing.
Add a fuzz test to verify that calling the String method on any
BuildInfo returned by Parse produces a string that parses to the same
BuildInfo. (Note that this doesn't ensure that String always produces
a parseable input: we assume that a user constructing a BuildInfo
provides valid paths and versions, so we don't bother to escape those.
It also doesn't ensure that ParseBuildInfo accepts all inputs that
ought to be valid.)
Meng Zhuo [Wed, 9 Feb 2022 02:39:15 +0000 (10:39 +0800)]
runtime: AES maphash scramble 3 times on 386
Issue #43130 shows flaky hash not inbalanced on 386 platform,
which is using AES hashing instead of wyhash.
This CL increase the scramble times to 3 that amd64 using right now.
Robert Griesemer [Tue, 8 Feb 2022 04:59:59 +0000 (20:59 -0800)]
doc/go1.18: document behavior of go/types predicates for extended interfaces
For #47694.
Change-Id: Ic27193b65ef4b3c0c932107b8731b5f8d3190ad5
Reviewed-on: https://go-review.googlesource.com/c/go/+/383918
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Mui [Tue, 8 Feb 2022 19:46:58 +0000 (14:46 -0500)]
cmd/dist: test cgo internal linking on darwin-arm64
CL 383554 disables testing cgo internal linking on all ARM64 but
Windows, because it doesn't work with newer GCC. But
- darwin-arm64 works, and it does not use GCC
- we don't support cgo internal linking on windows-arm64 anyway.
This CL fixes the condition.
Change-Id: I9eb7b81ef75e482f5e95d2edae4863ba21396432
Reviewed-on: https://go-review.googlesource.com/c/go/+/384269
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sean Liao [Tue, 8 Feb 2022 21:34:13 +0000 (22:34 +0100)]
net/smtp: match actual behavior for Auth.Start
Looking at history, it appears to never have worked as documented.
Fixes #48759
Change-Id: I066307c28e3ed1875c1c4049bade62e2818dd400
Reviewed-on: https://go-review.googlesource.com/c/go/+/383998 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
Robert Griesemer [Tue, 8 Feb 2022 02:37:02 +0000 (18:37 -0800)]
go/types, types2: AssertableTo is undefined for generalized interfaces
Document that AssertableTo is undefined (at least for 1.18) if
the first argument is a generalized interface; i.e., an interface
that may only be used as a constraint in Go code.
Still, implement it as we might expect it to be defined in the
future, to prevent problems down the road due to Hyrum's Law.
While at it, also removed the internal flag forceStrict and its
one use in Checker.assertableTo; forceStrict was never enabled
and if it would have been enabled, the behavior would not have
been correct.
Change-Id: Ie4dc9345c88d04c9640f881132154a002db22643
Reviewed-on: https://go-review.googlesource.com/c/go/+/383917
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Ian Lance Taylor [Tue, 8 Feb 2022 01:07:44 +0000 (17:07 -0800)]
runtime: restore old mp.fastrand initialization
CL 337350 changed mp.fastrand from a [2]uint32 to a uint64 and changed
the initialization to a single call of int64Hash. However, int64Hash
returns uintptr, so 32-bit systems this always left the most
significant 32 bits of mp.fastrand initialized to 0. The new code also
did not protect against initializing mp.fastrand to 0, which on a
system that does not implement math.Mul64 (most 32-bit systems) would
lead fastrand to always return 0.
This CL restores the mp.fastrand initialization to what it was before
CL 337350, adjusted for the change from [2]uint32 to uint64.
Change-Id: I663b415d9424d967e8e665ce2d017604dcd5b204
Reviewed-on: https://go-review.googlesource.com/c/go/+/383916
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Tue, 8 Feb 2022 17:47:46 +0000 (09:47 -0800)]
spec: explicitly define integer, floating-point, and complex types
The terms "integer type", "floating-point type", and "complex type"
are used frequently in the spec but are not explicitly (only indirectly)
defined.
Slightly rephrased the section on numeric types and introduce these
terms explicitly. Add links to this section.
Change-Id: I3fb888933bece047da8b356b684c855618e9aee4
Reviewed-on: https://go-review.googlesource.com/c/go/+/384157
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Tue, 8 Feb 2022 21:11:09 +0000 (13:11 -0800)]
builtin: clarify that interface types do not implement comparable
Fixes #51083
Change-Id: Ic9207ae4104b06749925186e0eb4f18edf1b5007
Reviewed-on: https://go-review.googlesource.com/c/go/+/384235
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Bryan C. Mills [Tue, 8 Feb 2022 20:07:21 +0000 (15:07 -0500)]
runtime: skip TestGdbBacktrace flakes matching a known GDB internal error
TestGdbBacktrace occasionally fails due to a GDB internal error.
We have observed the error on various linux builders since at least
October 2020, and it has been reported upstream at least twice.¹²
Since the bug is external to the Go project and does not appear to be
fixed upstream, this failure mode can only add noise.
Daniel Martí [Tue, 8 Feb 2022 14:49:30 +0000 (14:49 +0000)]
misc/reboot: don't use symlinks when copying GOROOT/src
go:embed disallows using symlinked files by design.
crypto/elliptic is the first std package to use it as of CL 380475,
and unfortunately that broke the TestRepeatBootstrap long test.
The reason it uses symlinks is for speed; it wants to copy GOROOT/src,
but regular files aren't going to be modified in any way,
so a symlink, if supported, means not needing to copy the contents.
Replace the symlink attempt with hard links,
which will mean regular files remain as such, fixing go:embed.
It's worth noting that on many systems hard links won't work,
as the temporary filesystem tends to be separate,
but it doesn't hurt to try.
In my system, where /tmp is tmpfs, the test now copies more bytes.
With the added Logf, I can see overlayDir goes from ~30ms to ~100ms.
This makes sense, as GOROOT/src currently weighs around 100MiB.
To alleviate that slow-down, stop copying testdata directories,
as they currently weigh around 20MiB and aren't needed for the test.
This gets overlayDir on my system down to an acceptable ~70ms.
I briefly considered teaching overlayDir what files can be symlinks,
but that seemed fairly complex long-term, as any file could be embedded.
While here, start using testing.T.TempDir and fs.WalkDir.
Daniel Martí [Mon, 24 Jan 2022 15:26:33 +0000 (15:26 +0000)]
crypto/elliptic: use go:embed for the precomputed p256 table
go.dev/cl/339591 changed the code generation to use a constant string,
so that the ~88KiB table can be marked read-only.
The compiled code became a lot better, but unfortunately,
the generated Go source became significantly more inefficient.
The numbers below compare "gofmt -l" and "go tool compile" of said file,
where "old" is the file as of Go 1.17, and "new" as of master in 2022/01/19:
name old time/op new time/op delta
Gofmt 22.8ms ± 6% 898.5ms ± 3% +3837.32% (p=0.000 n=8+8)
GoToolCompile 26.9ms ± 2% 371.1ms ± 2% +1278.36% (p=0.000 n=7+8)
name old user-time/op new user-time/op delta
Gofmt 25.7ms ±65% 897.1ms ± 3% +3383.86% (p=0.000 n=8+8)
GoToolCompile 35.1ms ±26% 367.2ms ± 3% +945.80% (p=0.000 n=8+8)
name old sys-time/op new sys-time/op delta
Gofmt 6.42ms ±276% 7.23ms ±38% ~ (p=0.412 n=8+6)
GoToolCompile 9.20ms ±100% 13.90ms ±53% ~ (p=0.105 n=8+8)
name old peak-RSS-bytes new peak-RSS-bytes delta
Gofmt 9.11MB ± 7% 22.79MB ± 1% +150.23% (p=0.000 n=8+8)
GoToolCompile 25.1MB ± 2% 68.6MB ± 2% +173.57% (p=0.000 n=8+8)
"+" operators are binary expressions at the syntax tree level,
which are represented by packages like go/ast as roughly:
struct {
X Expr
Op Token
Y Expr
}
Since each node is a pointer, chains of "+" operators act like linked lists.
The generated code has about 14k lines, and 8 "+" operators per line,
meaning that we end up with a linked list with over 11k elements.
This explains the slow-down in gofmt; the printer must walk said list,
and it does so more than once to work out how to format it.
It seems like the compiler is similarly affected by the huge length.
To remedy the effect of the linked list, use go:embed instead.
This results in the same string variable with the binary table,
but it greatly reduces the amount of syntax and its cost above.
We still keep the generator around, but modified so it produces the
binary file to be embedded rather than a large Go file.
Finally, we update go/build/deps_test.go to allow crypto/elliptic to
depend on embed; it's a tiny package and crypto/elliptic was already
manually embedding assets via code generation.
The change to deps_test.go was briefly discussed in the issue below.
Robert Findley [Tue, 1 Feb 2022 22:13:53 +0000 (17:13 -0500)]
go/parser: fix incorrect resolution of receiver type parameters
Declare receiver type parameters in the function scope, but don't
resolve them (for now), as ast.Object.Decl is not documented to hold
*ast.Idents. This avoids incorrect resolution of identifiers to names
outside the function scope.
Also make tracing and error reporting more consistent.
For golang/go#50956
Change-Id: I8cd61dd25f4c0f6b974221599b00e23d8da206a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/382247
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Mon, 7 Feb 2022 19:28:14 +0000 (11:28 -0800)]
go/types, types2: ensure we have an interface before checking constraints
Due to delayed computation of constraint type sets it is possible that
a type argument is checked against a constraint before that constraint
has been wrapped into an implicit interface (if needed).
When the type checker is about to check whether a type argument
implements a constraint, it's always safe to force wrapping of
a constraint in an implicit interface (if necessary) before doing
the implements check.
Also, use a better position for tracing output related to constraint
type set computation.
Fixes #51048.
Change-Id: I52fecbf57814f09b62762452d7e17c2a230fdd59
Reviewed-on: https://go-review.googlesource.com/c/go/+/383834
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Ian Lance Taylor [Mon, 7 Feb 2022 03:33:03 +0000 (19:33 -0800)]
runtime: don't assume that 0.25 * 100 is representable as int
For the gofrontend, it's not; there are some trailing one bits,
which is permitted by the spec.
Change-Id: I58f5a5347068bb488306ec8e73da7c59376f1ae1
Reviewed-on: https://go-review.googlesource.com/c/go/+/383635
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Ian Lance Taylor [Mon, 7 Feb 2022 18:02:30 +0000 (10:02 -0800)]
doc/go1.18: mention new x/exp generic packages
Change-Id: I119cfb1a0da9af89ced78935b8fcdfdb8d9b4ab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/383794
Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Bryan C. Mills [Mon, 7 Feb 2022 16:33:30 +0000 (11:33 -0500)]
misc/cgo: fix aliasing bugs in parallel tests that append to shared slices
These tests use a slice to represent the base C compiler command (with
flags). Appending to that slice can cause subtle aliasing bugs, such
as commands that silently corrupt the arguments of other concurrent
commands in parallel tests.
In this change, we explicitly reduce the capacity of the command slice
to force appends to it to always allocate unique new slices.
Benny Siegert [Sat, 5 Feb 2022 16:23:26 +0000 (17:23 +0100)]
cmd/dist: skip internal linking tests on arm64
The previous workaround for issue #39466 only disabled this test for
Linux. However, the issue manifests for all arm64 systems with gcc 9.4
and above. The new netbsd-arm64 builder uses NetBSD-current with gcc
10.3, so it fails in the same way.
Updates #39466
Change-Id: I276a99a5e60914e5c22f74a680e461bea17cfe92
Reviewed-on: https://go-review.googlesource.com/c/go/+/383554
Trust: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Sat, 5 Feb 2022 05:14:13 +0000 (21:14 -0800)]
runtime: change sys.PtrSize to goarch.PtrSize in comments
The code was updated, the comments were not.
Change-Id: If387779f3abd5e8a1b487fe34c33dcf9ce5fa7ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/383495
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Fri, 4 Feb 2022 00:07:28 +0000 (16:07 -0800)]
go/types, types2: always use missingMethodReason in checker.Implements
Remove special case where we don't have a *Checker and always use
Checker.missingMethodReason in Checker.implements.
Look for zero methods rather than empty interface to exit early
from Checker.missingMethod, and remove the extra test in
Checker.implements.
With this change we get consistent and more detailed error messages
from all places where we do a form of the "implements" test.
To make this possible, allow for the receiver to be nil in
- Checker.sprintf
- Checker.missingMethodReason
- Checker.interfacePtrError
- Checker.funcString
Allowing Checker.sprintf with nil Checker permits further simplifying
in a couple of places.
Change-Id: I0ea7178c9efbcd4a25ded2a66e2b058db52dc4d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/383054
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 3 Feb 2022 23:03:02 +0000 (15:03 -0800)]
go/types, types2: simplify Checker.typeAssertion, use same code in both type checkers
- Remove the xtyp argument from the Checker.typeAssertion parameter
list; it was confusing and not needed. Adjusted call sites.
- Simplify logic in Checker.typeAssertion.
- Use the same code in both types2 and go/types, specifically
use the same error positions.
- Adjust error messages as needed.
This removes another subtle discrepancy between types2 and go/types.
The go/types error messages don't have the have/want appendix for
the affected error messages yet because we don't use case folding
in lookups yet.
Change-Id: Id39f5c473da36c9baad60082f85cf1f34dc26c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/383014
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 3 Feb 2022 05:36:50 +0000 (21:36 -0800)]
go/types, types2: simplify missingMethodReason
Added a funcString helper so we don't need to rewrite strings
with strings.Replace.
Use compiler format for error message about wrong method type;
this removes another unnecessary variation.
Simplify conditions for pointer-to-interface related error:
if one of the involved types is an interface pointer, it can't
have any methods.
Rewrite logic so we don't need all the else-if branches.
Adjusted a test case for types2 accordingly. The go/types version
of this test case has a different error because the implementation
of Checker.typeAssertion is different in the two type checkers
(the types2 version gives errors closer to the 1.17 compiler).
Change-Id: I19e604d7063c3e31e8290bd78384a9f38bb0d740
Reviewed-on: https://go-review.googlesource.com/c/go/+/382694
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 3 Feb 2022 00:31:26 +0000 (16:31 -0800)]
go/types, types2: fix bug in types2.MissingMethod
Because Checker.missingMethod also looks up methods matching
matching case-folded names, when Checker.missingMethod returns
an alternative method, that method does not automatically have
the wrong type. It may be a method with a different name.
Adjust types2.MissingMethod to check the alternative method
name before reporting a wrong type.
Add API test that verifies (now correct) behavior for this
case.
Ported the code also to go/types, though it was not a bug
there yet because looking up with case-folding is not yet
enabled.
Change-Id: Iaa48808535c9265a9879338ea666c6c021e93a2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/382634
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Wed, 2 Feb 2022 23:04:58 +0000 (15:04 -0800)]
go/types, types2: remove superflous type parameter length checks
There is no need to check for length equality of type parameter
lists in Checker.missingMethod: the Identical predicate does
this check. Furthermore, we don't have methods with their own
type parameters. Remove the unnecessary (duplicate) code.
Also, update doc string on missingMethod and rename the 2nd
result parameter for clarity, and clarify internal comments.
For go/types, include the same case-folding code as for types2
but leave it disabled or now. Adjust any other differences in
the missingMethod implementation.
With this change, the types2 and go/types code of missingMethod
is identical again except for the disabled case-folding lookup.
No functionality changes.
Preparation for fixing some of the outstanding error reporting issues.
Change-Id: I4778d006c17f4e084ecc2cac7386c68e86aa49eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/382614
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Wed, 2 Feb 2022 21:18:56 +0000 (13:18 -0800)]
go/types, types2: use same method lookup code in both type checkers
types2 has some code to handle case-folding when doing method lookups
which was missing in go/types. This change is a first step to match
the implementations. Specifically:
In types2:
- remove the lookupMethodFold names in favor of just lookupMethod,
but with the foldCase flag (e.g., instead if lookupMethodFold,
we just use lookupMethod)
- rename checkFold to foldCase everywhere
- add foldCase parameter where it was missing
- moved foldCase paremeter to the end in lookupFieldOrMethod
- no functionality changes
In go/types:
- match function signatures with types2 use
- always provide false as argument for foldCase for now
- no functionality changes
Preparation for fixing some of the outstanding error reporting issues.
Change-Id: If129a5feb89ddf96a3596e8d73b23afa591875a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/382461
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Tue, 1 Feb 2022 05:25:14 +0000 (21:25 -0800)]
go/types, types2: better error messages for comparisons
Refactor Checker.comparison such that its logic is easier to reason
about and so that special cases can be handled more directly.
Use the appropriate operand (of 1st or 2nd operand) for error
reporting (position and type), rather than always using the
first operand.
Use an extra parameter to indicate a switch case
comparison; in this case the error is always reported at
the position of the first operand. (The error messages are
not yet adjusted for switches; see next CL.)
Introduce a new kindString function which is used to print simplified
types in error messages (related to comparisons only): instead of
printing the details of a struct type, we just print "struct" where
the details are not relevant. This matches the 1.17 compiler behavior.
Added a "reportf" parameter to the internal comparable function so we
can report an error cause in addition to the boolean result. Rather
than passing a *string for cause, we pass a function to record the
cause so that we can use the *Checker context for printing (needed
for proper type qualification). This mechanism reports the same
details now as the 1.17 compiler.
Adjusted various tests as needed added new test files.
Fixes #50918.
Change-Id: I1f0e7af22f09db4d31679c667c71a9038a8dc9d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/381964
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Brandon Bennett [Fri, 4 Feb 2022 19:45:18 +0000 (12:45 -0700)]
cmd/go: preserve LIBRARY_PATH and C_INCLUDE_PATH for script tests
In bespoke build environments default libraries may be specificied with
LIBRARY_PATH, C_INCLUDE_PATH enviroment variables to overide the system
(i.e glibc). Allow them though to allow cgo testing of these enviroments.
Kevin Burke [Fri, 4 Feb 2022 19:27:17 +0000 (11:27 -0800)]
internal/nettrace: fix spelling error
Change-Id: Ibf51c0687197c0d791916b21cba7f8408aa5300a
Reviewed-on: https://go-review.googlesource.com/c/go/+/383216 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Kevin Burke <kevin@burke.dev>
Robert Findley [Wed, 2 Feb 2022 23:40:27 +0000 (18:40 -0500)]
go/parser, go/types: don't parse type parameters on methods
The go/parser package is updated to report an error on method type
parameters, and to not store them in the AST. Tests are updated
accordingly, and error messages are normalized accross go/parser and the
compiler syntax package.
Before this CL, go/parser would parse type parameters on method
declarations and interface methods, leaving it to go/types to complain.
There are several problems with this:
- Interface Methods and Method declarations do not have type parameters
in the spec. We try to align the parser with the productions in the
spec.
- Parsing method type parameters means that downstream libraries
(go/doc, go/format, etc.) theoretically need to handle them, even
though they are not part of the language.
- Relatedly, go/types has inconsistent handling of method type
parameters due to support being removed, leading to the crasher in
#50427.
It is more consistent and safer to disallow type parameters on methods
in the parser.
Fixes #50427
Change-Id: I555766da0c76c4cf1cfe0baa9416863088088b4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/382538
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Mon, 31 Jan 2022 21:37:40 +0000 (16:37 -0500)]
runtime: regression test for issue 50936
Add a regression test for issue 50936 which coerces the runtime into
frequent execution of the cgocall dropg/execute curg assignment race by
making many concurrent cgo calls eligible for P retake by sysmon. This
results in no P during exitsyscall, at which point they will update curg
and will crash if SIGPROF arrives in between updating mp.curg and
mp.curg.m.
This test is conceptually similar to the basic cgo callback test in
aprof.go but with additional concurrency and a sleep in C.
On my machine this test fails ~5% of the time prior to CL 382079.
For #50936.
Change-Id: I21b6c7f2594f9a615a64580ef70a88b692505678
Reviewed-on: https://go-review.googlesource.com/c/go/+/382244
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Bryan C. Mills [Wed, 2 Feb 2022 15:19:30 +0000 (10:19 -0500)]
runtime: skip TestSegv failures with "morestack on g0" on darwin/amd64
This failure mode has been present since at least 2020-06-08. We have
enough information to diagnose it, and further failures don't seem to
be adding any new information at this point: they can only add noise,
both on the Go project's builders and in users' own modules (for
example, when run as part of 'go test all').
Bryan C. Mills [Thu, 13 Jan 2022 20:38:14 +0000 (15:38 -0500)]
cmd/go/internal/modfetch: do not short-circuit canonical versions
Since at least CL 121857, the conversion logic in
(*modfetch).codeRepo.Stat has had a short-circuit to use the version
requested by the caller if it successfully resolves and is already
canonical.
However, we should not use that version if it refers to a branch
instead of a tag, because branches (unlike tags) usually do not refer
to a single, stable release: a branch named "v1.0.0" may be for the
development of the v1.0.0 release, or for the development of patches
based on v1.0.0, but only one commit (perhaps at the end of that
branch — but possibly not even written yet!) can be that specific
version.
We already have some logic to prefer tags that are semver-equivalent
to the version requested by the caller. That more general case
suffices for exact equality too — so we can eliminate the
special-case, fixing the bug and (happily!) also somewhat simplifying
the code.
Robert Griesemer [Fri, 28 Jan 2022 02:55:29 +0000 (18:55 -0800)]
go/types, types2: shorter list for 2nd phase of function type inference
In the 2nd phase of function argument type inference we only
consider parameters with types that are single type parameters.
Thus there is no need to collect anything else in the first
phase.
This matches the algorithm description in the forthcoming spec
more closely.
Change-Id: Ie5c29f30ff43b1e37d719ecbe1688b50ed2177f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/381554
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Bryan C. Mills [Wed, 2 Feb 2022 22:09:28 +0000 (17:09 -0500)]
net: remove an arbitrary timeout in TestUDPReadSizeError
Looking at the condition actually exercised by the test it seems
unnecessary: assuming that the Write succeeds (checked earlier in the
test), the Read must have a nonzero number of bytes available to read
immediately. (That is not the case in TestUDPZeroByteBuffer, from
which this test appears to have been derived.)
Ian Lance Taylor [Wed, 2 Feb 2022 21:22:18 +0000 (13:22 -0800)]
constraints: remove package
It has moved to golang.org/x/exp/constraints. Perhaps it will move
back to the standard library in a future release.
For golang/go#45458
Fixes golang/go#50792
Change-Id: I93aa251a7afe7b329a3d3faadc0c5d6388b1f0e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/382460
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Filippo Valsorda [Sat, 30 Oct 2021 04:27:51 +0000 (00:27 -0400)]
crypto/ecdsa,crypto/elliptic: update docs and spec references
crypto/ecdsa was long overdue a cleanup. Bump the FIPS 186 version, and
make sure we consistently reference that and SEC 1, not the paywalled
ANSI standard.
Robert Findley [Wed, 2 Feb 2022 18:47:31 +0000 (13:47 -0500)]
go/types, types2: add a const to control recursion panics in unification
Add a panicAtUnificationDepthLimit const to replace the use of the debug
const to control whether to panic when the unification recursion depth
is reached. Our tests should pass when debug==true.
Change-Id: I58847f49d66010bd4ca01c0408ec10acac95cba6
Reviewed-on: https://go-review.googlesource.com/c/go/+/382534
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Fri, 28 Jan 2022 23:48:32 +0000 (15:48 -0800)]
cmd/go: document that GOENV=off disables the default config file
Fixes #46840
Change-Id: Icc5475e312003e316dc039413a35089485163ba6
Reviewed-on: https://go-review.googlesource.com/c/go/+/381894
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Trust: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bryan C. Mills [Tue, 1 Feb 2022 16:25:13 +0000 (11:25 -0500)]
cmd/go: fix relative-path normalization in go.work files
We had been statting 'dir' instead of 'absDir', which statted the
wrong directory if 'dir' was made relative to the location of the
go.work file and that location was different from the current working
directory.
While we're here, I also noticed some other dir-vs.-absDir
discrepancies. The haveDirs map had assumed relative, slash-separated
filesystem paths, but absolute paths on Windows are not
slash-separated and we do retain those. Moreover, if users hand-edit
the go.work file they may introduce non-canonical paths.
I have changed the haveDirs logic to retain absolute paths in their
platform-specific forms, and to call DropUse with the original path
from the go.work file instead of the newly-resolved path.
Robert Griesemer [Tue, 1 Feb 2022 17:07:49 +0000 (09:07 -0800)]
doc/go1.18: explain changes to compiler error messages
For #47694.
For #50954.
Change-Id: I3789e145d64025fe2bf787fa5d01a21832903586
Reviewed-on: https://go-review.googlesource.com/c/go/+/381968
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dan Scales [Wed, 26 Jan 2022 18:09:39 +0000 (10:09 -0800)]
go/types, types2: tweak missingMethodReason logic to improve message
This makes the error case pointed out in the issue like the current
message in Go 1.17 or -G=0 mode. The priority is to point out the
similar but wrong method name, rather than a difference in type.
Made changes to both cmd/compile/internal/types2 and go/types.
Added in a missing tab in an error message in go/types.
At the same time, removed the extra "at info" on the have lines (and
pointer receiver lines) of error messages, as requested in #50907.
Fixes #50816
Fixes #50907
Change-Id: I04f8151955bdb6192246cbcb59adc1c4b8a2c4e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381774 Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Roland Shoemaker [Sun, 30 Jan 2022 19:21:28 +0000 (11:21 -0800)]
internal/fuzz: properly handle duplicates during cache loading
When loading the corpus, if the cache contained an entry which was a
duplicate of an entry added using f.Add, coordinator.addCorpusEntries
would return early, ignoring everything after this entry in the cache.
Instead, skip duplicates as intended, and continue to load the rest of
the cache.
Robert Griesemer [Fri, 28 Jan 2022 23:34:40 +0000 (15:34 -0800)]
spec: add section on comparable constraint
For #50646.
Fixes #50791.
Change-Id: I8fec25ae3f0280c5b5a778011d23842b886ba79e
Reviewed-on: https://go-review.googlesource.com/c/go/+/381896
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 28 Jan 2022 23:17:46 +0000 (15:17 -0800)]
cmd/go: update go work edit -json types to final version
For #45713
Change-Id: Ia55a96702b99cccaf5d96c2125ee513700658444
Reviewed-on: https://go-review.googlesource.com/c/go/+/381874
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Trust: Michael Matloob <matloob@golang.org>
Adding _TraceJumpStack to cgo traceback exposed a crashing condition.
This CL was primarily a cleanup, so we revert it entirely for now
and follow-up with the VDSO and libcall parts later.
Fixes #50936.
Change-Id: Ie45c9caaa8e2ef5bc9498ba65c36c887ca821bf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/382079 Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Thu, 27 Jan 2022 06:48:44 +0000 (22:48 -0800)]
go/types, types2: fix implements and identical predicates
- Use the correct predicate in Checker.implements: for interfaces
we cannot use the API Comparable because it always returns true
for all non-type parameter interface types: Comparable simply
answers if == and != is permitted, and it's always been permitted
for interfaces. Instead we must use Interface.IsComparable which
looks at the type set of an interface.
- When comparing interfaces for identity, we must also consider the
whether the type sets have the comparable bit set.
With this change, `any` doesn't implement `comparable` anymore. This
only matters for generic functions and types, and the API functions.
It does mean that for now (until we allow type-constrained interfaces
for general non-constraint use, at some point in the future) a type
parameter that needs to be comparable cannot be instantiated with an
interface anymore.
For #50646.
Change-Id: I7e7f711bdcf94461f330c90509211ec0c2cf3633
Reviewed-on: https://go-review.googlesource.com/c/go/+/381254
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Bryan C. Mills [Fri, 28 Jan 2022 22:22:32 +0000 (17:22 -0500)]
cmd/go: rewrite TestScript/cgo_stale_precompiled to be agnostic to staleness
The configuration set by x/build/cmd/releasebot causes runtime/cgo to
be stale in the darwin/amd64 release (see #36025, #35459).
That staleness is mostly benign because we can reasonably assume that
users on macOS will either disable CGO entirely or have a C compiler
installed to rebuild (and cache) the stale packages if needed.
Ian Lance Taylor [Fri, 28 Jan 2022 21:15:16 +0000 (13:15 -0800)]
doc/go1.18: mention new debug/dwarf DataBitOffset fields
For #46784
For #47694
For #50685
Change-Id: I5351b56722d036a520d1a598ef7af95c5eb44c35
Reviewed-on: https://go-review.googlesource.com/c/go/+/381778
Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Ian Lance Taylor [Fri, 28 Jan 2022 22:32:56 +0000 (14:32 -0800)]
doc/go1.18: in workspace mode doc, link to "go work" docs
For #45713
For #47694
Change-Id: I6f615c07749fca49c19f2ae22f79971c29aa8183
Reviewed-on: https://go-review.googlesource.com/c/go/+/381779
Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>