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>
Robert Griesemer [Fri, 28 Jan 2022 05:26:27 +0000 (21:26 -0800)]
go/types, types2: delete TypeList.String
This method is unused and was not discussed in the API
proposals. Note that all error output goes through the
local sprintf which handles arguments specially.
Fixes #50760.
Change-Id: Iae66b0253cc0ece037d3d280951dc2d223c119fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/381634
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 27 Jan 2022 18:52:22 +0000 (10:52 -0800)]
go/types, types2: use Checker.implements in operand.assignableTo
Now that we have the detailed error reporting in Checker.implements
we don't need it anymore in operand.assignableTo and can simply call
Checker.implements. This also more directly matches the spec.
For #50646.
Change-Id: Ic44ced999c75be6cc9edaab01177ee0495147ea1
Reviewed-on: https://go-review.googlesource.com/c/go/+/381435
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 27 Jan 2022 18:28:46 +0000 (10:28 -0800)]
go/types, types2: better error reporting for Checker.implements
This CL copies (and adjusts as needed) the logic for error reporting
from operand.assignableTo to Checker.implements in the case of a missing
method failure and assignment to an interface pointer.
Preparation for using Checker.implements in operand.assignableTo
rather than implementing the same logic twice.
This also leads to better errors from Checker.implements as it's
using the same logic we already use elsewhere.
For #50646.
Change-Id: I199a1e02cf328b222ae52c10131db871539863bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/381434
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Tue, 30 Nov 2021 17:29:45 +0000 (09:29 -0800)]
spec: add section on type inference
Change-Id: Ic338788d6410ed0d09ad129811377ee9ce5ed496
Reviewed-on: https://go-review.googlesource.com/c/go/+/367954
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Than McIntosh [Tue, 25 Jan 2022 14:34:35 +0000 (09:34 -0500)]
debug/dwarf: fix problems with handling of bit offsets for bitfields
This patch reworks the handling of the DWARF DW_AT_bit_offset and
DW_AT_data_bit_offset attributes to resolve problems arising from
a previous related change (CL 328709).
In CL 328709 the DWARF type reader was updated to look for and use
the DW_AT_data_bit_offset attribute for structure fields, handling
the value of the attribute in the same way as for DW_AT_bit_offset.
This caused problems for clients, since the two attributes have very
different semantics.
This CL effectively reverts CL 328709 and moves to a scheme in which
we detect and report the two attributes separately/independently.
This patch also corrects a problem in the DWARF type reader in the
code that detects and fixes up the type of struct fields corresponding
to zero-length arrays; the code in question was testing the
DW_AT_bit_offset attribute value but assuming DW_AT_data_bit_offset
semantics, meaning that it would fail to fix up cases such as
typedef struct another_struct {
unsigned short quix;
int xyz[0];
unsigned x:1;
long long array[40];
} t;
The code in question has been changed to avoid using BitOffset and
instead consider only ByteOffset and BitSize.
Fixes #50685.
Updates #46784.
Change-Id: Ic15ce01c851af38ebd81af827973ec49badcab6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/380714
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Robert Findley [Thu, 27 Jan 2022 01:50:51 +0000 (20:50 -0500)]
go/types: update interface receivers after substituting
Interface method receivers are synthetic: they record either the
interface type or the the defined type for which they are the RHS of the
type declaration. When instantiating, we need to update these receivers
accordingly.
Fixes #50839
Change-Id: Icd8e1a2817b0135059d25d034b01b0ff5207641f
Reviewed-on: https://go-review.googlesource.com/c/go/+/381174
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>
For a composite literal expression like []T{{f: 1}}, we allow T to be
a pointer to struct type, so it's consistent to allow T to also be a
type parameter whose structural type is a pointer to struct type.
Fixes #50833.
Change-Id: Ib0781ec4a4f327c875ea25b97740ff2c0c86b916
Reviewed-on: https://go-review.googlesource.com/c/go/+/381075
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Roland Shoemaker [Mon, 1 Nov 2021 17:03:36 +0000 (10:03 -0700)]
internal/fuzz: centralize corpus entry addition
Adds an addCorpusEntry method to coordinator which manages checking for
duplicate entries, writing entries to the cache directory, and adding
entries to the corpus. Also moves readCache to be a method on the
coordinator.
Dan Scales [Thu, 27 Jan 2022 01:58:08 +0000 (17:58 -0800)]
cmd/compile: add missing shape check in (*Tsubster).tinter
Add a missing shape check in (*Tsubster).tinter when substituting on a
generic type which is an empty interface, analogous to same check in
(*Tsubster).tstruct. Empty structs/interfaces that have rparams (i.e.
are a generic type or a shape type) need to get a new type of their
rparams - they will be different even though they don't have any
fields/methods. Without this shape check, we were not correctly
completing the Token[int] type during substitution in the example in the
issue. This issue only happens for a generic type which is an empty
interface (i.e. doesn't actually use the type param, hence quite unusual).
Added the test case already created by Keith.
Fixes #50841
Change-Id: Ia985b9f52c0e87ed0647b46373e44c51cb748ba4
Reviewed-on: https://go-review.googlesource.com/c/go/+/381175
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Matthew Dempsky [Wed, 26 Jan 2022 21:26:45 +0000 (13:26 -0800)]
cmd/compile: support structural typing in unified IR
This CL updates unified IR to look at the structural type of a
composite literal type, rather than merely the underlying type, to
determine if it's a structure. This fixes a number of currently
failing regress test cases.
Updates #50833.
Change-Id: I11c040c77ec86c23e8ffefcf1ce1aed548687dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381074
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Robert Griesemer [Wed, 26 Jan 2022 22:01:52 +0000 (14:01 -0800)]
Revert "spec: document the underlying type of comparable"
This reverts CL 380854.
Per the conluding discussions on #50791. A follow-up will
document `comparable` more thoroughly.
For #50791.
Change-Id: I15db9051784a012f713e28d725c3b8bbfeb40569
Reviewed-on: https://go-review.googlesource.com/c/go/+/381076
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
This method isn't available in go/types, and its use by unified IR is
non-essential. This CL refactors reader2.go to avoid using it and then
removes the method.
Change-Id: I813c93a062c43292bb6760686ef91df5219534a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/380834
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org>
Bryan C. Mills [Wed, 26 Jan 2022 17:45:12 +0000 (12:45 -0500)]
cmd/api: reduce parallel 'go list' invocations to a constant
'go list' has its own internal parallelism, so invoking in in parallel
can produce up to quadratic peak memory usage.
Running 'go list' is also very I/O-intensive, so the higher
parallelism does substantially improve latency; unfortunately, we lack
a good way to balance latency against memory footprint, so we need to
sacrifice some latency for reliability.
Robert Griesemer [Wed, 26 Jan 2022 03:12:35 +0000 (19:12 -0800)]
spec: document the underlying type of comparable
For #50791.
Change-Id: I7f135bb6626128a3cee9fd71c57535c1fc83ac7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/380854
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Wed, 26 Jan 2022 14:18:14 +0000 (09:18 -0500)]
cmd/go: avoid recording GOROOT_FINAL in precompiled C archives
C archives for packages in GOROOT are shipped along with binary
releases of the Go toolchain. Although we build the toolchain with
GOROOT_FINAL set, we don't know actually know where the release will
be installed: the user's real GOROOT can differ arbitrarily from our
GOROOT_FINAL.
(In the specific case of toolchains installed through golang.org/dl
wrappers, the release's GOROOT_FINAL is /usr/local/go but the actual
GOROOT to which the release is installed is
$HOME/sdk/$(go env GOVERSION).)
Bryan C. Mills [Wed, 26 Jan 2022 14:24:25 +0000 (09:24 -0500)]
cmd/go: refactor TestScript/build_issue48319 to check a more general property
The test previously checked that the DWARF DW_AT_comp_dir attribute
matched GOROOT_FINAL. However, on further consideration, we believe
that DW_AT_comp_dir should not actually match GOROOT_FINAL: the DWARF
spec says that DW_AT_comp_dir records “the current working directory
of the compilation command that produced this compilation unit”, but
the actual working directory of the compilation command proper is a
throwaway directory in the build cache — it is neither stable nor
meaningful.
However, the test was getting at a real issue that we do care about:
namely, that the binary produced by a 'go build' command with cgo
enabled should not reuse a dependency that embeds a stale
GOROOT_FINAL.
This change refactors the test to verify the latter property instead
of checking DW_AT_comp_dir specifically.
Robert Findley [Tue, 25 Jan 2022 22:31:52 +0000 (17:31 -0500)]
all: update vendored golang.org/x/tools
Update the vendored x/tools to pick up CL 380014, which updates the
ifaceassert vet analyzer to remove spurious errors for assertions
involving interfaces with type parameters.
This also picks up some superficial changes related to refactoring of
the x/tools/internal/typeparams API.
The following commands were used:
go get -d golang.org/x/tools@master
go mod tidy
go mod vendor
Fixes #50658
Change-Id: I2f612fd186a1a260cab21860b192c9f6dc3f560f
Reviewed-on: https://go-review.googlesource.com/c/go/+/380777
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Tim King <taking@google.com>
Robert Findley [Mon, 24 Jan 2022 15:37:59 +0000 (10:37 -0500)]
go/types, types2: make each method instantiation independently lazy
Method signatures can introduce a significant number of edges into the
type graph. One can imagine a generic type with many methods, each of
which may use other instantiated types, etc. For performance, when type
checking generic code, we should avoid unnecessary instantiation of
methods wherever possible.
This CL achieves this by making method instantiation lazy at the
individual method level. It abstracts method access into a methodList
type, which may be either eager or lazy. In the lazy case, methods are
only instantiated when they are accessed via the Named.Method,
MethodSet, or LookupFieldOrMethod APIs. Factoring out a methodList type
makes it easier to verify that we're not leaking the methods slice
anywhere, and as a side benefit reduces the size of *Named types in the
case where there are no methods. The effective memory footprint of Named
types with methods increases by a pointer (to hold the slice of guards),
and the footprint of instantiated named types increases additionally by
a sync.Once per method. We estimate that this memory increase is more
than offset by the reduction in the number of instantiated methods.
This also simplifies the code. Previously we had to work around the fact
that named type expansion could occur before all signatures were set-up,
by stashing the instantiated receiver into a partially filled-out *Func.
With fully lazy methods, we can rely on the invariant that any use of
methods in valid code can only occur after all signatures can be type
checked. This means that we can fully instantiate the *Func, and don't
need to deal with partially instantiated stubs.
Finally, this CL fixes a bug (issue #50619), where traversing
Method->Receiver Type->Method did not get us back where we started. This
is fixed by not instantiating a new method if t is already the receiver
base of the original method.
A test is added to explicitly verify the invariant above, and more test
cases are added for the behavior of Info with respect to generic code.
Fixes #50619
Change-Id: I5b6d2bdc4404c9f5dcb583a29cb64e8af9794c54
Reviewed-on: https://go-review.googlesource.com/c/go/+/380499
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>
Dan Scales [Tue, 25 Jan 2022 17:50:03 +0000 (09:50 -0800)]
test: add a new test absdiff3.go which uses function callback
We have disallowed having a typeparam on the right-hand-side of a type
declaration. So, we disabled much of the test absdiff.go. I recently
wrote a new test absdiff2.go to use a structure containing the type
param type, so I could attach a method properly and run the full test.
As a contrast, I thought I would create absdiff3.go, where the Abs
functionality is passed in as a function callback (but derived from a
generic function). This is simpler, and more inline with some of the
guidelines that Ian has been proposing (use passed-in functions rather
than requiring methods, when possible, for greater ease-of-use).
Only adds a new test absdiff3.go. (And fixes a comment in absdiff2.go.)
Change-Id: I6dd185b50a3baeec31f689a892319963468a7201
Reviewed-on: https://go-review.googlesource.com/c/go/+/380774 Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Dan Scales [Mon, 24 Jan 2022 22:38:01 +0000 (14:38 -0800)]
cmd/compile: new absdiff.go test, fix problem with g.curDecl
Added a new absdiff2.go test case, which works fully without using a
typeparam on the right-hand-side of a type declaration (which is
disallowed). Fixed an issue that the test revealed, which is that we
need to set g.curDecl properly for the "later" functions which are
deferred until after all declarations are initially processed. Also,
g.curDecl may be non-nil in typeDecl for local type declaration. So, we
adjust the associate assertion, and save/restore g.curDecl
appropriately.
Fixes #50790
Change-Id: Ieed76a7ad0a83bccb99cbad4bf98a7bfafbcbbd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/380594 Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Robert Findley [Mon, 24 Jan 2022 20:42:52 +0000 (15:42 -0500)]
go/types, types2: pass the seen map through _TypeSet.IsComparable
While checking comparability of type parameters, we recurse through
_TypeSet.IsComparable, but do not pass the cycle-tracking seen map,
resulting in infinite recursion in some cases.
Refactor to pass the seen map through this recursion.
Fixes #50782
Change-Id: I2c2bcfed3398c11eb9aa0c871da59e348bfba5f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/380504
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 [Fri, 21 Jan 2022 17:05:51 +0000 (09:05 -0800)]
go/types, types2: reorder object processing to avoid broken aliases
By processing non-alias type declarations before alias type declaration,
and those before everything else we can avoid some of the remaining
errors which are due to alias types not being available.
For #25838.
For #50259.
For #50276.
For #50729.
Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/380056
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Fri, 21 Jan 2022 02:56:57 +0000 (18:56 -0800)]
go/types, types2: report an error when using a broken alias
The type checker doesn't have a general mechanism to "use" the type
of a type alias whose type depends on a recursive type declaration
which is not yet completely type-checked. In some cases, the type of
a type alias is needed before it is determined; the type is incorrect
(invalid) in that case but no error is reported. The type-checker is
happy with this (incorrect type), but the compiler may crash under
some circumstances.
A correct fix will likely require some form of forwarding type which
is a fairly pervasive change and may also affect the type checker API.
This CL introduces a simple side table, a map of broken type aliases,
which is consulted before the type associated with a type alias is
used. If the type alias is broken, an error is reported.
This is a stop-gap solution that prevents the compiler from crashing.
The reported error refers to the corresponding issue which suggests
a work-around that may be applicable in some cases.
Also fix a minor error related to type cycles: If we have a cycle
that doesn't start with a type, don't use a compiler error message
that explicitly mentions "type".
Fixes #50259.
Fixes #50276.
Fixes #50779.
For #50729.
Change-Id: Ie8e38f49ef724e742e8e78625e6d4f3d4014a52c
Reviewed-on: https://go-review.googlesource.com/c/go/+/379916
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Sat, 15 Jan 2022 01:42:20 +0000 (17:42 -0800)]
go/types, types2: consider type parameters for cycle detection
In validType, when we see an instantiated type, proceed as with
non-generic types but provide an environment in which to look up
the values (the corresponding type arguments) of type parameters
of the instantiated type. For each type parameter for which there
is a type argument, proceed with validating that type argument.
This corresponds to applying validType to the instantiated type
without actually instantiating the type (and running into infinite
instantiations in case of invalid recursive types).
Also, when creating a type instance, use the correct source position
for the instance (the start of the qualified identifier if we have an
imported type).
Fixes #48962.
Change-Id: I196c78bf066e4a56284d53368b2eb71bd8d8a780
Reviewed-on: https://go-review.googlesource.com/c/go/+/379414
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Sat, 15 Jan 2022 01:34:59 +0000 (17:34 -0800)]
go/types, types2: remove special case for external types in validType
Because validType doesn't modify global state anymore, there's
no need to ignore imported types. When we start tracking type
parameters, we need to include imported types because they may
contribute to cycles that invalidate a type.
This CL effectively reverts CL 202483 (issue #35049, which
doesn't apply anymore because we don't change the state of
imported objects).
Preparation for fixing issue #48962.
For #35049.
For #48962.
Change-Id: I06f15575ad197375c74ffd09c222250610186b15
Reviewed-on: https://go-review.googlesource.com/c/go/+/378675
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Sat, 15 Jan 2022 01:01:43 +0000 (17:01 -0800)]
go/types, types2: move validType code into its own file
The validType check is independent of the work of declaring objects.
Move it into a separate file for better separation of concerns and
code organization.
No other changes - this is purely a code move.
Preparation for fixing issue #48962.
Change-Id: Ib08db2d009c4890882d0978b278e965ca3078851
Reviewed-on: https://go-review.googlesource.com/c/go/+/378674
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Fri, 21 Jan 2022 21:52:56 +0000 (16:52 -0500)]
runtime: replace TestFutexsleep with TestTimediv
TestFutexsleep was originally created in CL 7876043 as a
regression test for buggy division logic in futexsleep. Several months
later CL 11575044 moved this logic to timediv (called by futexsleep).
This test calls runtime.Futexsleep, which temporarily disables
asynchronous preemption. Unfortunately, TestFutexSleep calls this from
multiple goroutines, creating a race condition that may result in
asynchronous preemption remaining disabled for the remainder of the
process lifetime.
We could fix this by moving the async preemption disable to the main
test function, however this test has had a history of flakiness. As an
alternative, this CL replaces the test wholesale with a new test for
timediv, covering the overflow logic without the difficulty of dealing
with futex.
Fixes #50749.
Change-Id: If9e1dac63ef1535adb49f9a9ffcaff99b9135895
Reviewed-on: https://go-review.googlesource.com/c/go/+/380058
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> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Dan Scales [Wed, 19 Jan 2022 22:46:58 +0000 (14:46 -0800)]
cmd/compile: distinguish bound calls/field access in getInstInfo
Given we have support for field access to type params with a single
structural type, we need to distinguish between methods calls and field
access when we have an OXDOT node on an expression which is a typeparam
(or correspondingly a shape). We were missing checks in getInstInfo,
which figures out the dictionary format, which then caused problems when
we generate the dictionaries. We don't need/want dictionary entries for
field access, only for bound method calls. Added a new function
isBoundMethod() to distinguish OXDOT nodes which are bound calls vs.
field accesses on a shape.
Removed isShapeDeref() - we can't have field access or method call on a
pointer to variable of type param type.
Fixes #50690
Change-Id: Id692f65e6f427f28cd2cfe474dd30e53c71877a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/379674
Trust: Dan Scales <danscales@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Austin Clements [Mon, 24 Jan 2022 14:27:31 +0000 (09:27 -0500)]
runtime: call fflush before exiting in C test
Very, very rarely TestVectoredHandlerDontCrashOnLibrary fails because
the C subprocess exits with a 0 status code and no output. This
appears to happen because C does not actually guarantee that stdout
will be flushed on exit and somehow, very rarely, it is not flushed.
Add explicit fflushes to fix this. This reduces the failure rate of
TestVectoredHandlerDontCrashOnLibrary from 0.0013% to 0% in 250,000
iterations.
Ian Lance Taylor [Fri, 21 Jan 2022 22:30:39 +0000 (14:30 -0800)]
spec: minor formatting and link cleanups
Mostly from CL 367954.
Change-Id: Id003b0f785a286a1a649e4d6e8c87d0418a36545
Reviewed-on: https://go-review.googlesource.com/c/go/+/379920
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>
Michael Matloob [Fri, 21 Jan 2022 21:05:02 +0000 (16:05 -0500)]
cmd/go: evaluate root symlink in matchPackages
This fixes checks for crossing module boundaries when the root of
the module is a symlink. We're comparing paths by string, so we need
to follow the symlink to get the proper path to compare.
Change-Id: Idf5f0dd5c49bcae5fffb5372e99a7fab89169a9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/380057
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 Pratt [Fri, 21 Jan 2022 16:07:52 +0000 (11:07 -0500)]
runtime/pprof: TestLabelSystemstack parallelLabelHog.func1 must be labeled
The closure in parallelLabelHog should be labeled in a addition to
parallelLabelHog itself. Generally samples on that goroutine land on
labelHog, but there is a small portion of the closure outside of
labelHog.
Robert Griesemer [Fri, 21 Jan 2022 00:20:33 +0000 (16:20 -0800)]
Revert "doc/go1.18: document type parameter name restriction"
This reverts CL 376414.
For #47694.
For #50481.
Change-Id: Ie73961046e52e6e5d3262ef0aeaa24bec7eaa937
Reviewed-on: https://go-review.googlesource.com/c/go/+/379835
Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Dan Scales [Tue, 18 Jan 2022 20:55:17 +0000 (12:55 -0800)]
cmd/compile: make sure multiple blank typeparams remain unique
In a method declaration "func (f *Foo[_, _]) String() string { ... }",
the two blank typeparams have the same name, but our current design with
types1 needs unique names for type params. Similarly, for export/import,
we need unique names to keep the type params straight in generic types
and connect the proper type param with the proper constraint. We make
blank type params unique by changing them to $1, $2, etc in noder.typ0()
via typecheck.TparamExportName(). We then revert $<num> back to _ during
type2 import via typecheck.TparamName(). We similarly revert
during gcimporter import. We don't need/want to revert in the types1
importer, since we want unique names for type params.
Rob Findley has made a similar change to x/tools (and we tried to make
the source code changes similar for the gcimporter and types2 importer
changes).
Fixes #50419
Change-Id: I855cc3d90d06bcf59541ed0c879e9a0e4ede45bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/379194 Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Ian Lance Taylor [Wed, 19 Jan 2022 02:46:00 +0000 (18:46 -0800)]
runtime: remove -tags=threadprof in tests
Use an enviroment variable rather than a build tag to control starting
a busy loop thread when testprogcgo starts. This lets us skip another
build that invokes the C compiler and linker, which should avoid
timeouts running the runtime tests.
Fixes #44422
Change-Id: I516668d71a373da311d844990236566ff63e6d72
Reviewed-on: https://go-review.googlesource.com/c/go/+/379294
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Robert Findley [Thu, 20 Jan 2022 03:00:42 +0000 (22:00 -0500)]
go/types, types2: use Identical rather than unification in missingMethod
Now that we instantiate methods on instantiated types, there is no need
to use unification to match signatures inside of missingMethod.
Generally, we should never encounter uninstantiated signatures within
statements. If we do encounter signatures that contain type parameters,
it is because the signatures are themselves defined or instantiated
using type parameters declared in the function scope (see example
below). The current unification logic would not handle this.
type S[T any] struct{}
func (S[T]) m(T)
func _[P any]() bool {
var v interface{m(int)}
_, ok = v.(S[P])
return ok
}
Change-Id: I754fb5535bba2fc7a209dc7419fd4015c413c9a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/379540
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>
Dan Scales [Mon, 17 Jan 2022 21:24:06 +0000 (13:24 -0800)]
cmd/compile: add early a CONVIFACE normally created in the order phase
Most CONVIFACEs are created in the transform phase (or old typechecker,
in -G=0 mode). But if the main result of a multi-value assignment (map,
channel, or dot-type) must be converted to an interface during the
assignment, that CONVIFACE is not created until (*orderState).as2ok in
the order phase (because the AS2* ops and their sub-ops are so tightly
intertwined). But we need to create the CONVIFACE during the
stenciling/transform phase to enable dictionary lookups. So, in
transformAssign(), if we are doing a special multi-value assignment
involving a type-param-derived type, assign the results first to temps,
so that we can manifest the CONVIFACE during the transform in assigning
the first temp to lhs[0].
Added a test for both AS2RECV (channel receives) and AS2MAPR (maps). I
don't think we can have a type assertion on a type-param-derived type.
Fixes #50642
Change-Id: I4d079fc46c93d8494d7db4ea8234d91522edb02a
Reviewed-on: https://go-review.googlesource.com/c/go/+/379054 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Wed, 19 Jan 2022 19:25:12 +0000 (14:25 -0500)]
runtime/pprof: compare all samples vs rusage in TestCPUProfileMultithreadMagnitude
TestCPUProfileMultithreadMagnitude compares pprof results vs OS rusage
to verify that pprof is capturing all CPU usage. Presently it compares
the sum of cpuHog1 samples vs rusage. However, background usage from the
scheduler, GC, etc can cause additional CPU usage causing test failures
if rusage is too far off from the cpuHog1 samples.
That said, this test doesn't actually need to care about cpuHog1
samples. It simply cares that pprof samples match rusage, not what the
breakdown of usage was. As a result, we can compare the sum of _all_
pprof samples vs rusage, which should implicitly include any background
CPU usage.
Fixes #50097.
Change-Id: I649a18de5b3dcf58b62be5962fa508d14cd4dc79
Reviewed-on: https://go-review.googlesource.com/c/go/+/379535
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Pratt [Tue, 7 Dec 2021 17:01:04 +0000 (12:01 -0500)]
runtime/pprof: assert that labels never appear on unexpected samples
This makes TestLabelSystemstack much more strict, enabling it to detect
any misplacement of labels.
Unfortunately, there are several edge cases where we may not have an
obviously correct stack trace, so we generally except the runtime
package, with the exception of background goroutines that we know should
not be labeled.
For #50007
For #50032
Change-Id: I8dce7e7da04f278ce297422227901efe52782ca0
Reviewed-on: https://go-review.googlesource.com/c/go/+/369984
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Michael Pratt [Tue, 7 Dec 2021 20:59:14 +0000 (15:59 -0500)]
runtime: do not inherit labels on system goroutines
GC background mark worker goroutines are created when the first GC is
triggered (or next GC after GOMAXPROCS increases). Since the GC can be
triggered from a user goroutine, those workers will inherit any pprof
labels from the user goroutine.
That isn't meaningful, so avoid it by excluding system goroutines from
inheriting labels.
Fixes #50032
Change-Id: Ib425ae561a3466007ff5deec86b9c51829ab5507
Reviewed-on: https://go-review.googlesource.com/c/go/+/369983 Reviewed-by: Austin Clements <austin@google.com>
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Bryan C. Mills [Wed, 12 Jan 2022 15:26:04 +0000 (10:26 -0500)]
runtime: eliminate arbitrary timeout in TestStackGrowth
Instead, allow the test to run up until nearly the test's deadline,
whatever that may be, and then crash with a panic (instead of calling
t.Errorf) to get a useful goroutine dump.
With the arbitrary timeout removed, we can now also run this test in
short mode, reducing its impact on test latency.
Cherry Mui [Tue, 18 Jan 2022 23:36:00 +0000 (18:36 -0500)]
cmd/compile: don't elide extension for LoadReg to FP register on MIPS64
For an extension operation like MOWWreg, if the operand is already
extended, we optimize the second extension out. Usually a LoadReg
of a proper type would come already extended, as a MOVW/MOVWU etc.
instruction does. But for a LoadReg to a floating point register,
the instruction does not do the extension. So we cannot elide the
extension.
Fixes #50671.
Change-Id: Id8991df78d5acdecd3fd6138c558428cbd5f6ba3
Reviewed-on: https://go-review.googlesource.com/c/go/+/379236
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Austin Clements [Tue, 18 Jan 2022 21:27:40 +0000 (16:27 -0500)]
runtime: deflake TestPreemptionAfterSyscall
This test occasionally takes very slightly longer than the 3 second
timeout on slow builders (especially windows-386-2008), so increase
the timeout to 5 seconds. It fails with much longer timeouts on Plan
9, so skip it as flaky there.
x.go:3:33: no result values expected <<<
x.go:4:33: no result values expected <<<
x.go:5:26: not enough return values
have ()
want (int)
x.go:6:36: too many return values
have (number, number)
want (int)
x.go:7:26: not enough return values
have ()
want (int, int)
x.go:8:33: not enough return values
have (number)
want (int, int)
There are two problems with the current special case emitting the
errors on the marked line:
1. It calls them 'result values' instead of 'return values'.
2. It doesn't show the type being returned, which can be useful to programmers.
Using the general case solves both these problems,
so this CL removes the special case and calls the general case instead.
Now those two errors read:
x.go:3:33: too many return values
have (number, number)
want ()
x.go:4:33: too many return values
have (number)
want ()
Russ Cox [Tue, 18 Jan 2022 15:49:57 +0000 (10:49 -0500)]
cmd/compile, go/types: fix checking of bad type switch
Consider the following program:
package p
func f() {
x := 1
v := 2
switch v.(type) {
case int:
println(x)
println(x / 0)
case 1:
}
}
Before this CL, the compiler prints:
x.go:4:2: x declared but not used
x.go:6:9: v (variable of type int) is not an interface
x is in fact used, and other errors in the switch go undiagnosed.
This commit fixes that problem by processing the switch statement
even when the 'not an interface' error is reported.
Now the compiler drops the spurious 'declared but not used'
and adds two previously undiagnosed problems:
x.go:6:9: v (variable of type int) is not an interface
x.go:9:15: invalid operation: division by zero
x.go:10:7: 1 is not a type
go/types was printing roughly the same thing the compiler did before,
and now still prints roughly the same thing the compiler does after.
(The only differences are in the exact reported columns.)
Bryan C. Mills [Tue, 18 Jan 2022 17:21:18 +0000 (12:21 -0500)]
net/http: skip TestClientTimeout_Headers_h{1,2} on windows/arm and windows/arm64
This extends the skip added in CL 375635 to the "_Headers" variant of
the test, since we have observed similar failures in that variant on
the builders.
Dan Scales [Fri, 7 Jan 2022 00:51:10 +0000 (16:51 -0800)]
cmd/compile: support field access for typeparam with structural constraint
In the compiler, we need to distinguish field and method access on a
type param. For field access, we avoid the dictionary access (to create
an interface bound) and just do the normal transformDot() (which will
create the field access on the shape type).
This field access works fine for non-pointer types, since the shape type
preserves the underlying type of all types in the shape. But we
generally merge all pointer types into a single shape, which means the
field will not be accessible via the shape type. So, we need to change
Shapify() so that a type which is a pointer type is mapped to its
underlying type, rather than being merged with other pointers.
Because we don't want to change the export format at this point in the
release, we need to compute StructuralType() directly in types1, rather
than relying on types2. That implementation is in types/type.go, along
with the helper specificTypes().
I enabled the compiler-related tests in issue50417.go, added an extra
test for unnamed pointer types, and added a bunch more tests for
interesting cases involving StructuralType(). I added a test
issue50417b.go similar to the original example, but also tests access to
an embedded field.
I also added a unit test in
cmd/compile/internal/types/structuraltype_test.go that tests a bunch of
unusual cases directly (some of which have no structural type).
Updates #50417
Change-Id: I77c55cbad98a2b95efbd4a02a026c07dfbb46caa
Reviewed-on: https://go-review.googlesource.com/c/go/+/376194 Reviewed-by: Keith Randall <khr@golang.org> 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>
Russ Cox [Sun, 16 Jan 2022 17:58:45 +0000 (12:58 -0500)]
cmd/dist: avoid lapsing into x86 builds on ARM64 Macs
We use uname -m to decide the GOHOSTARCH default,
and on my ARM64 Mac laptop, uname -m prints x86_64.
uname -a prints:
Darwin p1.local 21.1.0 Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:01 PDT 2021; root:xnu-8019.41.5~1/RELEASE_ARM64_T6000 x86_64
(Note the x86_64 at the end, consistent with uname -m.)
The effect of this is that make.bash builds an x86 toolchain
even when I start with an ARM64 bootstrap toolchain!
Avoid being tricked by looking for RELEASE_ARM64 instead.
Fixes #50643.
Change-Id: I76eded84bde8009d29419d5982bf964a0bf1c8fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/378894
Trust: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
eric fang [Wed, 5 Jan 2022 09:20:06 +0000 (09:20 +0000)]
cmd/internal/obj/arm64: adjust rule for VMOVQ instruction
The VMOVQ instruction stores a 128-bit number into a V register, for
example:
VMOVQ $0x1122334455667788, $0x99aabbccddeeff00, V2
From a documentation (https://pkg.go.dev/cmd/internal/obj/arm64) point
of view, the value in V2 should be 0x112233445566778899aabbccddeeff00,
however the value is actually 0x99aabbccddeeff001122334455667788. The
reason is that we misplaced the high 64-bit and the low 64-bit in the
literal pool. To maintain backward compatibility, this CL adjusts the
rule of VMOVQ instruction to make the documentation consistent with the
code.
Fixes #50528
Change-Id: Ib51f59e97c55252ab2a50bbc6ba4d430732a7a04
Reviewed-on: https://go-review.googlesource.com/c/go/+/377055 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Eric Fang <eric.fang@arm.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Trust: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Austin Clements [Fri, 14 Jan 2022 18:11:36 +0000 (13:11 -0500)]
runtime/race: be less picky about test run time
Currently, there are two regexps in the race detector output tests
that assume subtests will complete in < 1 second. This isn't necessary
and very occasionally fails (on builders that are probably very
loaded). Make these tests less picky about timing.
Michael Pratt [Mon, 13 Dec 2021 22:32:07 +0000 (17:32 -0500)]
cmd/dist: log CPU model when testing
Knowing whether test failures are correlated with specific CPU models on
has proven useful on several issues. Log it for prior to testing so it
is always available.
internal/sysinfo provides the CPU model, but it is not available in the
bootstrap toolchain, so we can't access this unconditionally in
cmd/dist. Instead use a build-tagged file, as the final version of
cmd/dist will use the final toolchain.
The addition of new data to the beginning of cmd/dist output will break
x/build/cmd/coordinator's banner parsing, leaving extra lines in the log
output, though information will not be lost.
https://golang.org/cl/372538 fixes up the coordinator and should be
submitted and deployed before this CL is submitted.
Alessandro Arzilli [Tue, 4 Jan 2022 14:47:02 +0000 (15:47 +0100)]
debug/elf: do not read unrelated bytes for SHT_NOBITS sections
SHT_NOBITS sections do not occupy space in the file and their offset is
"conceptual", reading their data should return all zeroes instead of
reading bytes from the section that follows them.
Russ Cox [Wed, 12 Jan 2022 22:22:09 +0000 (17:22 -0500)]
runtime: fix net poll races
The netpoll code was written long ago, when the
only multiprocessors that Go ran on were x86.
It assumed that an atomic store would trigger a
full memory barrier and then used that barrier
to order otherwise racy access to a handful of fields,
including pollDesc.closing.
On ARM64, this code has finally failed, because
the atomic store is on a value completely unrelated
to any of the racily-accessed fields, and the ARMv8
hardware, unlike x86, is clever enough not to do a
full memory barrier for a simple atomic store.
We are seeing a constant background rate of trybot
failures where the net/http tests deadlock - a netpollblock
has clearly happened after the pollDesc has begun to close.
The code that does the racy reads is netpollcheckerr,
which needs to be able to run without acquiring a lock.
This CL fixes the race, without introducing unnecessary
inefficiency or deadlock, by arranging for every updater
of the relevant fields to publish a summary as a single
atomic uint32, and then having netpollcheckerr use a
single atomic load to fetch the relevant bits and then
proceed as before.
Dan Scales [Thu, 13 Jan 2022 21:20:19 +0000 (13:20 -0800)]
cmd/compile: add call to ImportedBody() when exporting shape inst body
When we export a shape instantiation, because a particular
fully-instantiated type is needed by an inlineable function, we possibly
export the body of the instantiation, if it is inlineable. In this case,
we should have been calling ImportedBody() to make sure that the
function body had already been read in (if it is actually imported from
another package).
Fixes #50598
Change-Id: I512d2bcc745faa6ff3a97e25bc8f46e2c2643d23
Reviewed-on: https://go-review.googlesource.com/c/go/+/378494
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
Keith Randall [Thu, 13 Jan 2022 00:11:35 +0000 (16:11 -0800)]
cmd/compile: stop interface conversions for generic method calls from allocating
Let T be a type parameter, and say we instantiate it with S, a type
that isn't pointer-like (e.g. a pair of ints, or as in 50182, a
slice). Then to call a method m on a variable of type T, the compiler
does essentially:
var v T = ...
i := (interface{m()})(v)
i.m()
The conversion at that second line allocates, as we need to make the
data word for an interface. And in the general case, that interface
may live an arbitrarily long time. But in this case, we know it
doesn't.
The data word of i has type *S. When we call i.m, we can't call S.m
directly. It is expecting an S, not a *S. We call through a wrapper
defined on *S, which looks like:
func (p *S) m() {
var s S = *p
s.m()
}
The value passed in for p is exactly the data word mentioned above. It
never escapes anywhere - the wrapper copies a type S variable out of
*p and p is dead after that. That means that in the situation where we
build an interface for the explicit purpose of calling a method on it,
and use that built interface nowhere else, the allocation of the data
word for that interface is known to die before the call returns and
thus can be stack allocated.
One tricky case is that although the allocation of the backing store
of the interface conversion doesn't escape, pointers we store *inside*
that allocation might escape (in fact they definitely will, unless we
can devirtualize the receiver).
Fixes #50182
Change-Id: I40e893955c2e6871c54ccecf1b9f0cae17871b0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/378178
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Dan Scales [Wed, 12 Jan 2022 19:30:57 +0000 (11:30 -0800)]
cmd/compile: descend through types to find fully-instantiated types
In order to make sure we export the dictionaries/shape methods for all
fully-instantiated types in inlineable functions, we need to descend
fully into types. For example, we may have a map type (e.g.
map[transactionID]Promise[*ByteBuffer]), where the key or value is a new
fully-instantiated type. So, I add a new checkFullyInst() traversal
function, which traverses all encountered types, but maintains a map, so
it only traverse it type once. We need to descend fully into interfaces,
structs, and methods, since a fully-instantiated type make occur in any
fields or arguments/results of methods, etc.
Fixes #50561
Change-Id: I88681a30384168539ed7229eed709f4e73ff0666
Reviewed-on: https://go-review.googlesource.com/c/go/+/378154 Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Thu, 16 Dec 2021 20:55:15 +0000 (12:55 -0800)]
cmd/compile: unique LinkString for renamed, embedded fields
Using type aliases, it's possible to create structs with embedded
fields that have no corresponding type literal notation. However, we
still need to generate a unique name for these types to use for linker
symbols. This CL introduces a new "struct{ Name = Type }" syntax for
use in LinkString formatting to represent these types.
Reattempt at CL 372914, which was rolled back due to race-y
LocalPkg.Lookup call that isn't safe for concurrency.
Fixes #50190.
Change-Id: I0b7fd81e1b0b3199a6afcffde96ade42495ad8d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/378434
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>