Unified IR wasn't marking instantiated generic functions as DUPOK,
even though they can appear in multiple compilation units, which
evidently interfered with cmd/link's dead code elimination logic.
Manually confirmed to fix the issue, but non-trivial to test within
$GOROOT/test currently, because it's only reproducible when
cmd/compile is invoked with -p. @rsc is currently investigating
updating test/run.go appropriately, after which I'll revisit writing a
test case.
Fixes#51519.
Change-Id: I74a79ed0ca15b25b826e419714af5ceb6e567012
Reviewed-on: https://go-review.googlesource.com/c/go/+/390956
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
In function prologue and epilogue, we save and restore FP and LR
registers, and adjust RSP. The current instruction sequence is as
follow.
For frame size <= 240B,
prologue:
MOVD.W R30, -offset(RSP)
MOVD R29, -8(RSP)
epilogue:
MOVD -8(RSP), R29
MOVD.P offset(RSP), R30
For frame size > 240B,
prologue:
SUB $offset, RSP, R27
MOVD R30, (R27)
MOVD R27, RSP
MOVD R29, -8(RSP)
epilogue:
MOVD -8(RSP), R29
MOVD (RSP), R30
ADD $offset, RSP
Each sequence uses two load or store instructions, actually we can load
or store two registers with one LDP or STP instruction. This CL changes
the sequences as follow.
For frame size <= 496B,
prologue:
STP (R29, R30), -(offset+8)(RSP)
SUB $offset, RSP, RSP
epilogue:
LDP -8(RSP), (R29, R30)
ADD $offset, RSP, RSP
For frame size > 496B,
prologue:
SUB $offset, RSP, R20
STP (R29, R30), -8(R20)
MOVD R20, RSP
epilogue:
LDP -8(RSP), (R29, R30)
ADD $offset, RSP, RSP
Change-Id: Ia58af85fc81cce9b7c393dc38df43bffb203baad
Reviewed-on: https://go-review.googlesource.com/c/go/+/379075
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Eric Fang <eric.fang@arm.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
This CL adds the defines for ABI registers on riscv64.
Updates #40724
Change-Id: I53a89d88b6feb1a88cf7008b8484d444791e8a55
Reviewed-on: https://go-review.googlesource.com/c/go/+/356519
Trust: mzh <mzh@golangcn.org>
Run-TryBot: mzh <mzh@golangcn.org>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
One of the files in CompileGoFiles is actually _cgo_import.go, but
that file is only generated for gc, not for gccgo.
Change-Id: I87bb55552e1409cc57da8f35a32b37ce4a3df60c
Reviewed-on: https://go-review.googlesource.com/c/go/+/390895
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Type parameter lists are not ambiguous for function declarations in the
way that they are ambiguous for type declarations. Avoid printing an
extra comma to disambiguate.
Fixes#51548
Change-Id: I8ca2b21e271982013653b9e220f92ee74f577ba2
Reviewed-on: https://go-review.googlesource.com/c/go/+/390914
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>
Given that we have seen failures with the same failure mode on both
openbsd/arm and android/arm64, it seems likely that the underlying bug
affects at least all ARM-based architectures.
It appears that either these architectures are not able to sample at
the frequency expected by the test, or the samples are for some reason
being dropped.
For #50218
Change-Id: I42a6c8ecda57448f8068e8facb42a4a2cecbbb37
Reviewed-on: https://go-review.googlesource.com/c/go/+/383997
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
As a follow-up to https://golang.org/cl/371474, add the OS version to
the metadata printed for each test.
This is a redo of CL 371475. This version updates go.mod and conforms to
the changes made in the parent commit.
Fixes#50146.
Change-Id: Iba5541cc8dd2c85c1fa3a215e30c8c3f9b6aaaab
Reviewed-on: https://go-review.googlesource.com/c/go/+/378590
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>
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 in cmd/dist. Instead use a
separate binary which cmd/dist will only build once testing begins.
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.
This is a redo of CL 371474. It switches back to the original approach
of using a separate binary, as the bootstap toolchain won't allow
cmd/dist to import internal packages.
For #46272.
For #49209.
For #50146.
Change-Id: I906bbda987902a2120c5183290a4e89a2440de58
Reviewed-on: https://go-review.googlesource.com/c/go/+/378589
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>
When describing call stacks that include inlined function calls, the
runtime uses "fake" PCs to represent the frames that inlining removed.
Those PCs correspond to real NOP instructions that the compiler inserts
for this purpose.
Describing the call stack in a protobuf-formatted profile requires the
runtime/pprof package to collapse any sequences of fake call sites back
into single PCs, removing the NOPs but retaining their line info.
But because the NOP instructions are part of the function, they can
appear as leaf nodes in a CPU profile. That results in an address that
should sometimes be ignored (when it appears as a call site) and that
sometimes should be present in the profile (when it is observed
consuming CPU time).
When processing a PC address, consider it first as a fake PC to add to
the current inlining deck, and then as a previously-seen (real) PC.
Fixes#50996
Change-Id: I80802369978bd7ac9969839ecfc9995ea4f84ab4
Reviewed-on: https://go-review.googlesource.com/c/go/+/384239
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
All other platforms providing the pipe2 syscall already implement it
that way. Do so as well on solaris, which allows to drop
runtime.syscall_pipe and its dependencies as well.
Change-Id: Icf04777f21d1804da74325d173fefdc87caa42eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/390716
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
All platforms with the pipe2 syscall now provide syscall.Pipe2. Use it
to implement os.Pipe.
This also allows to drop the illumos-specific wrapper in
internal/sys/unix.
Change-Id: Ieb712a1498e86a389bad261e4e97c61c11d4bdd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/390715
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Other platforms already define Pipe2, so add it for solaris as well.
Change-Id: If0d2dfc9a3613479fb4611a673a20a4aa0af0b2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/390714
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This test was failing locally in my clone of the go repo due to a Git
branch ending in ".go", which the test found and was attempting to
parse as a file. It's fragile to try to parse .go files in
GOROOT/.git, and wasteful to scan GOROOT/pkg and other non-source
directories; instead, let's only parse the directories we actually
expect to contain source files.
(I was running the test for #51461.)
Change-Id: I5d4e31ec2bcd9b4b6840ec32ad9b12bf44f349a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/390023
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Also switch float64 NaN encoding to use hexadecimal, and accept
hexadecimal encoding for all other integer types too. (That gives us
the flexibility to change the encodings in either direction in the
future without breaking earlier Go versions.)
Out-of-range runes encoded using "%q" were previously replaced with
the Unicode replacement charecter, losing their values.
Out-of-range ints and uints on 32-bit platforms were previously
rejected. Now they are wrapped instead: an “interesting” case with a
large int or uint found on a 64-bit platform likely remains
interesting on a 32-bit platform, even if the specific values differ.
To verify the above changes, I have made TestMarshalUnmarshal accept
(and check for) arbitrary differences between input and output, and
added tests cases that include values in valid but non-canonical
encodings.
I have also added round-trip fuzz tests in the opposite direction for
most of the types affected by this change, verifying that a marshaled
value unmarshals to the same bitwise value.
Updates #51258
Updates #51526Fixes#51528
Change-Id: I7727a9d0582d81be0d954529545678a4374e88ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/390424
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
If you attempt to instantiate a generic type or func and run 'go build'
with a language version < 1.18 in the 'go' directive inside the go.mod
file, cmd/compile emits a friendly message that includes the suggestion
to 'check go.mod':
type instantiation requires go1.18 or later (-lang was set to go1.17; check go.mod)
However, if the code instead only declares a generic type or func
without instantiating, cmd/compile currently emits a less friendly
message:
type parameters require go1.18 or later
With this CL, the error in that situation becomes:
type parameter requires go1.18 or later (-lang was set to go1.17; check go.mod)
Within cmd/compile/internal/types2, it already calls check.versionErrorf
in a dozen or so places, including three existing calls to
check.versionErrorf within typeset.go (e.g., for embedding a constraint
interface).
This CL adds two more calls to check.versionErrorf, replacing calls to
check.softErrorf. Both check.versionErrorf and check.softErrorf call
check.err(at, <string>, true) after massaging the string message.
Fixes#51531
Change-Id: If54e179f5952b97701d1dfde4abb08101de07811
GitHub-Last-Rev: b0b7c1346f
GitHub-Pull-Request: golang/go#51536
Reviewed-on: https://go-review.googlesource.com/c/go/+/390578
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
The fieldtrack support is experimental and used mainly inside Google,
where we have included this change for years. No reason not to make
it in the public copy.
Change-Id: I5233e4e775ccce60a17098c007aed8c82a0425d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/387355
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The old code picks a random number n and then tests n, n+2, n+4, up to
n+(1<<20) for primality before giving up and picking a new n.
(The chance of finishing the loop and picking a new n is infinitesimally
small.) This approach, called “incremental search” in the Handbook of
Applied Cryptography, section 4.51, demands fewer bits from the random
source and amortizes some of the cost of the small-prime division
checks across the incremented values.
This commit deletes the n+2, n+4, ... checks, instead picking a series
of random n and stopping at the first one that is probably prime.
This approach is called “rejection sampling.”
Reasons to make this change, in decreasing order of importance:
1. Rejection sampling is simpler, and simpler is more clearly correct.
2. The main benefit of incremental search was performance, and that is
less important than before. Incremental search required fewer random
bits and was able to amortize the checks for small primes across the
entire sequence. However, both random bit generation and primality
checks have gotten faster much quicker than typical primes have
gotten longer, so the benefits are not as important today.
Also, random prime generation is not typically on the critical path.
Negating any lingering concerns about performance, rejection sampling
no slower in practice than the incremental search, perhaps because
the incremental search was using a somewhat inefficient test to
eliminate multiples of small primes; ProbablyPrime does it better.
name old time/op new time/op delta
Prime/MathRand 69.3ms ±23% 68.0ms ±37% ~ (p=0.531 n=20+19)
Prime/CryptoRand 69.2ms ±27% 63.8ms ±36% ~ (p=0.076 n=20+20)
(Here, Prime/MathRand is the current Prime benchmark,
and Prime/CryptoRand is an adaptation to use crypto/rand.Reader
instead of math/rand's non-cryptographic randomness source,
just in case the quality of the bits affects the outcome.
If anything, rejection sampling is even better with cryptographically
random bits, but really the two are statistically indistinguishable
over 20 runs.)
3. Incremental search has a clear bias when generating small primes:
a prime is more likely to be returned the larger the gap between
it and the next smaller prime. Although the bias is negligible in
practice for cryptographically large primes, people can measure the
bias for smaller prime sizes, and we have received such reports
extrapolating the bias to larger sizes and claiming a security bug
(which, to be clear, does not exist).
However, given that rejection sampling is simpler, more clearly
correct and at least no slower than incremental search, the bias
is indefensible.
4. Incremental search has a timing leak. If you can tell the incremental
search ran 10 times, then you know that p is such that there are no
primes in the range [p-20, p). To be clear, there are other timing
leaks in our current primality testing, so there's no definitive
benefit to eliminating this one, but there's also no reason to keep
it around.
(See https://bugs.chromium.org/p/boringssl/issues/detail?id=238 for
all the work that would be needed to make RSA key generation
constant-time, which is definitely not something we have planned for
Go crypto.)
5. Rejection sampling moves from matching OpenSSL to matching BoringSSL.
As a general rule BoringSSL is the better role model.
(Everyone started out using incremental search; BoringSSL switched
to rejection sampling in 2019, as part of the constant-time work
linked above.)
Change-Id: Ie67e572a967c12d8728c752045c7e38f21804f8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/387554
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Type inference for types was always a "nice to have" feature.
Given the under-appreciated complexity of making it work in all
cases, and the fact that we don't have a good understanding of
how it might affect readability of generic code, require explicit
type arguments for generic types.
This matches the current implementation.
Change-Id: Ie7ff6293d3fbea92ddc54c46285a4cabece7fe01
Reviewed-on: https://go-review.googlesource.com/c/go/+/390577
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When framesize <= objabi.StackSmall, 128B, the stacksplit prologue is:
MOVD 16(g), R16
MOVD SP, R17
CMP R16, R17
BLS morestack_label
The second instruction is not necessary, we can compare R16 with SP
directly, so the sequence becomes:
MOVD 16(g), R16
CMP R16, SP
BLS morestack_label
This CL removes this instruction.
Change-Id: I0567ac52e9be124880957271951e1186da203612
Reviewed-on: https://go-review.googlesource.com/c/go/+/379076
Trust: Eric Fang <eric.fang@arm.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Eric Fang <eric.fang@arm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Replace 24 spaces added in CL 389434 with 3 tabs,
so the new line is indented like other lines around it.
Updates #51419.
Change-Id: Ic3e50023a01f233c52dda53c36de2c461222d95c
Reviewed-on: https://go-review.googlesource.com/c/go/+/390674
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Adam Shannon <adamkshannon@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This should always be true, but use the HWCAP2 bit anyways.
Change-Id: Ib164cf05b4c9f0c509f41b7eb339ef32fb63e384
Reviewed-on: https://go-review.googlesource.com/c/go/+/389894
Trust: Paul Murphy <murp@ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This patch allows to zerocopy using MultiReader.
This is done by MultiReader implementing WriterTo.
Each sub reader is copied using usual io copy helper and thus use
WriterTo or ReadFrom with reflection.
There is a special case for when a subreader is a MultiReader.
Instead of using copyBuffer which would call multiReader.WriteTo,
multiReader.writeToWithBuffer is used instead, the difference
is that the temporary copy buffer is passed along, saving
allocations for nested MultiReaders.
The workflow looks like this:
- multiReader.WriteTo (allocates 32k buffer)
- multiReader.writeToWithBuffer
- for each subReader:
- is instance of multiReader ?
- yes, call multiReader.writeToWithBuffer
- no, call copyBuffer(writer, currentReader, buffer)
- does currentReader implements WriterTo ?
- yes, use use currentReader.WriteTo
- no, does writer implement ReadFrom ?
- yes, use writer.ReadFrom
- no, copy using Read / Write with buffer
This can be improved by lazy allocating the 32k buffer.
For example a MultiReader of such types:
MultiReader(
bytes.Reader, // WriterTo-able
bytes.Reader, // WriterTo-able
bytes.Reader, // WriterTo-able
)
Doesn't need any allocation, all copy can be done using bytes.Reader's
internal data slice. However currently we still allocate a 32k buffer
for nothing.
This optimisation has been omitted for a future patch because of high
complexity costs for a non obvious performance cost (it needs a benchmark).
This patch at least is on par with the previous MultiReader.Read
workflow allocation wise.
Fixes#50842
Change-Id: Ib070c8f36337d9dd86090df8a703c5df97a773ae
GitHub-Last-Rev: 8ebe60ceac
GitHub-Pull-Request: golang/go#51502
Reviewed-on: https://go-review.googlesource.com/c/go/+/390215
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: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
This change includes several smaller changes based on feedback
received so far.
These changes were reviewed at CL 385536. The only additional
change here is to the current date in the subtitle.
Change-Id: I653eb4a143e3b86c5357a2fd3b19168419c9f432
Reviewed-on: https://go-review.googlesource.com/c/go/+/390634
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
go/build is one of the packages that contributes the most towards
cmd/go's init cost, which adds up to any call to the tool.
One piece of low-hanging fruit is knownOS and knownArch,
maps which are filled via an init func from a space-separated list.
Using GODEBUG=inittrace=1, we can get three samples:
init go/build @0.36 ms, 0.024 ms clock, 6568 bytes, 74 allocs
init go/build @0.33 ms, 0.025 ms clock, 6888 bytes, 76 allocs
init go/build @0.36 ms, 0.025 ms clock, 6728 bytes, 75 allocs
After using a static map instead, we see an improvement:
init go/build @0.33 ms, 0.018 ms clock, 5096 bytes, 69 allocs
init go/build @0.36 ms, 0.021 ms clock, 5096 bytes, 69 allocs
init go/build @0.33 ms, 0.019 ms clock, 5096 bytes, 69 allocs
The speedup isn't huge, but it helps, and also reduces allocs.
One can also imagine that the compiler may get better with static,
read-only maps in the future, whereas the init func will likely always
have a linear cost and extra allocations.
Change-Id: I430212bad03d25358d2cc7b1eab4536ad88d05a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/390274
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Inference for type instances has dependencies on type-checking order
that can lead to subtle bugs. As explained in #51527, disable it for
1.18.
Fixes#51527
Change-Id: I42795bad30ce53abecfc5a4914599ae5a2041a9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/387934
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>
Due to instance de-duplication, we were failing to record some type
instances in types.Info.Instances. Fix this by moving the instance
recording out of the resolver.
Fixes#51494
Change-Id: Iddd8989307d95886eedb321efa4ab98cd2b3573a
Reviewed-on: https://go-review.googlesource.com/c/go/+/390041
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>
In Checker.typInternal, the SelectorExpr case was the only case that
didn't either set or pass along the incoming def *Named type.
Handle this by passing it along to Checker.selector and report a
cycle if one is detected.
Fixes#51509.
Change-Id: I6c2d46835f225aeb4cb25fe0ae55f6180cef038b
Reviewed-on: https://go-review.googlesource.com/c/go/+/390314
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The comparable bit was handled incorrectly. This CL establishes
a clear invariant for a type set's terms and its comparable bit
and correctly uses the bit when computing term intersections.
Relevant changes:
- Introduce a new function intersectTermLists that does the
correct intersection computation.
Minor:
- Moved the comparable bit after terms in _TypeSet to make it
clearer that they belong together.
- Simplify and clarify _TypeSet.IsAll predicate.
- Remove the IsTypeSet predicate which was only used for error
reporting in union.go, and use the existing predicates instead.
- Rename/introduce local variables in computeInterfaceTypeSet
for consistency and to avoid confusion.
- Update some tests whose output has changed because the comparable
bit is now only set if we have have the set of all types.
For instance, for interface{comparable; int} the type set doesn't
set the comparable bit because the intersection of comparable and
int is just int; etc.
- Add many more comments to make the code clearer.
Fixes#51472.
Change-Id: I8a5661eb1693a41a17ce5f70d7e10774301f38ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/390025
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>
Now that we always use types2 to validate user source code, we can
remove the constSet logic from typecheck for detecting duplicate
expression switch cases and duplicate map literal keys. This logic is
redundant with types2, and currently causes unified IR to report
inappropriate duplicate constant errors that only appear after type
substitution.
Updates #42758.
Change-Id: I51ee2c5106eec9abf40eba2480dc52603c68ba21
Reviewed-on: https://go-review.googlesource.com/c/go/+/390474
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>
This makes it easier to figure out where the crash is occurring.
Change-Id: Ie1f78a360367090dcd61c61b2a55c34f3e2ff2eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/390034
Trust: David Chase <drchase@google.com>
Reviewed-by: David Chase <drchase@google.com>
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>
Previous value used in the float32 roundtrip used float32(math.NaN())-1
which caused the quiet/signal bit to flip, which seemed to break the
test on MIPS platforms. Instead switch to using float32(math.NaN())+1,
which preserves the bit and makes the test happy.
Possibly related to #37455Fixes#51258
Change-Id: Ia85c649e89a5d02027c0ec197f0ff318aa819c19
Reviewed-on: https://go-review.googlesource.com/c/go/+/390214
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The test case is already working with unified IR, so add it to make
sure we don't regress while finishing unified IR's support for
dictionaries.
Updates #51521.
Change-Id: Ib7c8bf9612d30cd552e8e631fd0d487dcb177f14
Reviewed-on: https://go-review.googlesource.com/c/go/+/390356
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL changes unified IR to include itabs in its serialized
dictionary format.
Change-Id: I334c972dc1bc19293f955bb23cfb66844da7adec
Reviewed-on: https://go-review.googlesource.com/c/go/+/390355
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
None of the current generic type switch test cases exercise type
switches where the instantiated case is an interface type.
Change-Id: I9272fa61b8dde1fe1a3702d524d4f40253ef19b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/390354
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL switches unified IR to using ir.DynamicType for derived
types. This has an immediate effect of fixing compilation of generic
code that when fully stenciled results in statically invalid type
assertions. This does require updating typecheck to expect
ODYNAMICTYPE in type switches, but this is straightforward to
implement.
For now, we still statically resolve the runtime type (or itab)
pointer. However, a subsequent CL will allow reading these pointers
from the runtime dictionary.
Change-Id: I1666678fcc588bc9cb8b97871bd02b9059848e6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/390336
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
A few of the tests were printing garbage to stderr,
since FlagSet's default Output is os.Stderr:
$ go test
flag provided but not defined: -x
invalid value "1" for flag -v: test error
Usage of test:
flag needs an argument: -b
Usage of test:
-b usage
PASS
ok flag 0.008s
Add the remaining SetOutput(io.Discard) method calls.
Note that TestUserDefinedFunc was a tricky one.
Even with the added SetOutput calls,
the last part of the test would still print usage text to stderr.
It took me a while to figure out the problem was copying FlagSet.
I've filed go.dev/issue/51507 to record this particular sharp edge,
and the test code now avoids making FlagSet copies to avoid the bug.
Change-Id: I323f24091b98386312aa72df3eb890af6625628d
Reviewed-on: https://go-review.googlesource.com/c/go/+/390234
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Before #46477, the Go generics proposal allowed `type T = U` where `U`
was an uninstantiated generic type. However, we decided not to allow
that, and go/types and types2 have already been updated to disallow
it. This CL just removes the analogous code from unified IR.
Change-Id: I0fe6d1754c96790b498c1d5185b948333646d7de
Reviewed-on: https://go-review.googlesource.com/c/go/+/390315
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
We shouldn't need to read in function bodies for new functions found
during inlining, but something is expecting them to still be read
in. We should fix that code to not depend on them being read in, but
in the mean time reading them in anyway is at least correct, albeit
less efficient in time and space.
Fixes#49536.
Updates #50552.
Change-Id: I949ef45e7be09406e5a8149e251d78e015aca5fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/390335
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I7dfae0fc91c2d70873ec7ec920be7c0a4888153a
Reviewed-on: https://go-review.googlesource.com/c/go/+/390175
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Use dup3(oldfd, newfd, O_CLOEXEC) to atomically duplicate the file
descriptor and mark is as close-on-exec instead of dup2 & fcntl.
The dup3 system call first appeared in OpenBSD 5.7.
Change-Id: Ic06c2c7089dcdbd931ee24e5e8c316879d81474e
Reviewed-on: https://go-review.googlesource.com/c/go/+/389974
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Improve the test coverage of encoding/xml package by adding
the test cases for the execution paths that were not covered before.
Since it reveals a couple of issues, fix them as well while we're at it.
As I used an `strings.EqualFold` instead of adding one more `strings.ToLower`,
our fix to `autoClose()` tends to run faster as well as a result.
name old time/op new time/op delta
HTMLAutoClose-8 5.93µs ± 2% 5.75µs ± 3% -3.16% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
HTMLAutoClose-8 2.60kB ± 0% 2.58kB ± 0% -0.46% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
HTMLAutoClose-8 72.0 ± 0% 67.0 ± 0% -6.94% (p=0.000 n=10+10)
The overall `encoding/xml` test coverage increase is `88.1% -> 89.9%`;
although it may look insignificant, this CL covers some important corner cases,
like `autoClose()` functionality (that was not tested at all).
Fixes#49635Fixes#49636
Change-Id: I50b2769896c197eb285672313b7148f4fe8bdb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/364734
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
In a recent change CL 388654 a function was updated so it
no longer needed stack space, but the TEXT statement was
not updated to reflect that change. This corrects that problem.
Change-Id: I9e60cebddae620788b1097ab7b39c47b323d1f62
Reviewed-on: https://go-review.googlesource.com/c/go/+/389674
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Paul Murphy <murp@ibm.com>
For #51153
Change-Id: I4374c63498b62ba7a08f146eebd034cbd50623f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/389634
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
