This CL improves the error messages reported when a field or method
name is used that doesn't exist. It brings the error messges on par
(or better) with the respective errors reported before Go 1.18 (i.e.
before switching to the new type checker):
Make case distinctions based on whether a field/method is exported
and how it is spelled. Factor out that logic into a new function
(lookupError) in a new file (errsupport.go), which is generated for
go/types. Use lookupError when reporting selector lookup errors
and missing struct field keys.
Add a comprehensive set of tests (lookup2.go) and spot tests for
the two cases brought up by the issue at hand.
Adjusted existing tests as needed.
Fixes#49736.
Change-Id: I2f439948dcd12f9bd1a258367862d8ff96e32305
Reviewed-on: https://go-review.googlesource.com/c/go/+/560055
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Rather than implementing a new, less complete mechanism to check
if a selector exists with different capitalization, use the
existing mechanism in lookupFieldOrMethodImpl by making it
available for internal use.
Pass foldCase parameter all the way trough to Object.sameId and
thus make it consistently available where Object.sameId is used.
From sameId, factor out samePkg functionality into stand-alone
predicate.
Do better case distinction when reporting an error for an undefined
selector expression.
Cleanup.
Change-Id: I7be3cecb4976a4dce3264c7e0c49a320c87101e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/558315
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The input index to a jump table can be out of range for unreachable code.
Dynamically the compiler ensures that an out-of-range index can never
reach a jump table, but that guarantee doesn't extend to the static
realm.
Fixes#64826
Change-Id: I5829f3933ae5124ffad8337dfd7dd75e67a8ec33
Reviewed-on: https://go-review.googlesource.com/c/go/+/552055
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Per the discussion on the issue, since no problems related to this
appeared since Go 1.20, remove the ability to disable the check for
anonymous interface cycles permanently.
Adjust various tests accordingly.
For #56103.
Change-Id: Ica2b28752dca08934bbbc163a9b062ae1eb2a834
Reviewed-on: https://go-review.googlesource.com/c/go/+/550896
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Right shifts, for some odd reasons, can encode shifts of constant
1-32 instead of 0-31. Left shifts, however, can encode shifts 0-31.
When the shift amount is 0, arm recommends encoding right shifts
using left shifts.
Fixes#64715
Change-Id: Id3825349aa7195028037893dfe01fa0e405eaa51
Reviewed-on: https://go-review.googlesource.com/c/go/+/549955
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
This commit is aimed at improving the readability and consistency
of the code base. Extraneous newline characters were present after
some return statements, creating unnecessary separation in the code.
Fixes#64610
Change-Id: Ic1b05bf11761c4dff22691c2f1c3755f66d341f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/548316
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
We can't delete all the outgoing edges and then add one back in, because
then we've lost the argument of any phi at the target. Instead, move
the important target to the front of the list and delete the rest.
This normally isn't a problem, because there is never normally a phi
at the target of a jump table. But this isn't quite true when in race
build mode, because there is a phi of the result of a bunch of raceread
calls.
The reason this happens is that each case is written like this (where e
is the runtime.eface we're switching on):
if e.type == $type.int32 {
m = raceread(e.data, m1)
}
m2 = phi(m1, m)
if e.type == $type.int32 {
.. do case ..
goto blah
}
so that if e.type is not $type.int32, it falls through to the default
case. This default case will have a memory phi for all the (jumped around
and not actually called) raceread calls.
If we instead did it like
if e.type == $type.int32 {
raceread(e.data)
.. do case ..
goto blah
}
That would paper over this bug, as it is the only way to construct
a jump table whose target is a block with a phi in it. (Yet.)
But we'll fix the underlying bug in this CL. Maybe we can do the
rewrite mentioned above later. (It is an optimization for -race mode,
which isn't particularly important.)
Fixes#64606
Change-Id: I6f6e3c90eb1e2638112920ee2e5b6581cef04ea4
Reviewed-on: https://go-review.googlesource.com/c/go/+/548356
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When I was plumbing min/max support through the compiler, I was
thinking mostly about numeric argument types. As a result, I forgot
that escape analysis would need to be aware that min/max can operate
on string values, which contain pointers.
Fixes#64565.
Change-Id: I36127ce5a2da942401910fa0f9de922726c9f94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/547715
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL interleaves devirtualization and inlining, so that
devirtualized calls can be inlined.
Fixes#52193.
Change-Id: I681e7c55bdb90ebf6df315d334e7a58f05110d9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/528321
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
The early return here is meant to suppress inlining of the function
call itself. However, it also suppresses recursing to visit the call
arguments, which are safe to inline.
Change-Id: I75887574c00931cb622277d04a822bc84c29bfa2
Reviewed-on: https://go-review.googlesource.com/c/go/+/543658
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fix#63955
parseIndVar, prove and maybe more are on the assumption that the loop header
is a single block. This can be wrong, ensure we don't match theses cases we
don't know how to handle.
In the future we could update them so that they know how to handle such cases
but theses cases seems rare so I don't think the value would be really high.
We could also run a loop canonicalization pass first which could handle this.
The repro case looks weird because I massaged it so it would crash with the
previous compiler.
Change-Id: I4aa8afae9e90a17fa1085832250fc1139c97faa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/539977
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Have nil checks return a pointer that is known non-nil. Users of
that pointer can use the result, ensuring that they are ordered
after the nil check itself.
The order dependence goes away after scheduling, when we've fixed
an order. At that point we move uses back to the original pointer
so it doesn't change regalloc any.
This prevents pointer arithmetic on nil from being spilled to the
stack and then observed by a stack scan.
Fixes#63657
Change-Id: I1a5fa4f2e6d9000d672792b4f90dfc1b7b67f6ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/537775
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
I will shortly be sending CLs to let the gofrontend code pass
these tests.
Change-Id: I53ccbdac3ac224a4fdc9577270f48136ca73a62c
Reviewed-on: https://go-review.googlesource.com/c/go/+/536537
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Most of the test cases in the test directory use the new go:build syntax
already. Convert the rest. In general, try to place the build constraint
line below the test directive comment in more places.
For #41184.
For #60268.
Change-Id: I11c41a0642a8a26dc2eda1406da908645bbc005b
Cq-Include-Trybots: luci.golang.try:gotip-linux-386-longtest,gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/536236
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The funcdata is encoded as varint, with the upper limit set to 1e9.
However, the stack offsets could be up to 1<<30. Thus emitOpenDeferInfo
will trigger an ICE for function with large frame size.
By using binary.PutUvarint, the frame offset could be encoded correctly
for value larger than 1<<35, allow the compiler to report the error.
Further, the runtime also do validation when reading in the funcdata
value, so a bad offset won't likely cause mis-behavior.
Fixes#52697
Change-Id: I084c243c5d24c5d31cc22d5b439f0889e42b107c
Reviewed-on: https://go-review.googlesource.com/c/go/+/535077
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The immediate-data interface shortcuts also apply to pointer-shaped
things like maps, channels, and functions.
Fixes#63505.
Change-Id: I43982441bf523f53e9e5d183e95aea7c6655dd6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/534657
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Do this by removing all stores of zero-sized anything.
Fixes#63433.
Change-Id: I5d8271edab992d15d02005fa3fe31835f2eff8fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/534296
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 419456 starts using lookupObj to find types2.Object associated with
builtin functions. However, the new code does not un-parenthesized the
callee expression, causing an ICE because of nil obj returned.
Un-parenthesizing the callee expression fixes the problem.
Fixes#63436
Change-Id: Iebb4fbc08575e7d0b1dbd026c98e8f949ca16460
Reviewed-on: https://go-review.googlesource.com/c/go/+/533476
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The types2 typechecker already reported all invalid conversions required
by the Go language spec. However, the conversion involves go pragma is
not specified in the spec, so is not checked by types2.
Fixing this by handling the error gracefully during typecheck, just like
how old typechecker did before CL 394575.
Fixes#63333
Change-Id: I04c4121971c62d96f75ded1794ab4bdf3a6cd0ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/532515
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This patch changes the inliner to use callsite scores when deciding to
inline as opposed to looking only at callee cost/hairyness.
For this to work, we have to relax the inline budget cutoff as part of
CanInline to allow for the possibility that a given function might
start off with a cost of N where N > 80, but then be called from a
callsites whose score is less than 80. Once a given function F in
package P has been approved by CanInline (based on the relaxed budget)
it will then be emitted as part of the export data, meaning that other
packages importing P will need to also need to compute callsite scores
appropriately.
For a function F that calls function G, if G is marked as potentially
inlinable then the hairyness computation for F will use G's cost for
the call to G as opposed to the default call cost; for this to work
with the new scheme (given relaxed cost change described above) we
use G's cost only if it falls below inlineExtraCallCost, otherwise
just use inlineExtraCallCost.
Included in this patch are a bunch of skips and workarounds to
selected 'errorcheck' tests in the <GOROOT>/test directory to deal
with the additional "can inline" messages emitted when the new inliner
is turned on.
Change-Id: I9be5f8cd0cd8676beb4296faf80d2f6be7246335
Reviewed-on: https://go-review.googlesource.com/c/go/+/519197
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, the unified frontend implemented unsafe.Sizeof, etc that
involved derived types by constructing a normal OSIZEOF, etc
expression, including fully instantiating their argument. (When
unsafe.Sizeof is applied to a non-generic type, types2 handles
constant folding it.)
This worked, but involves unnecessary work, since all we really need
to track is the argument type (and the field selections, for
unsafe.Offsetof).
Further, the argument expression could generate temporary variables,
which would then go unused after typecheck replaced the OSIZEOF
expression with an OLITERAL. This results in compiler failures after
CL 523315, which made later passes stricter about expecting the
frontend to not construct unused temporaries.
Fixes#62515.
Change-Id: I37baed048fd2e35648c59243f66c97c24413aa94
Reviewed-on: https://go-review.googlesource.com/c/go/+/527097
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Currently, cmd/compile optimizes `var a = true; var b = a` into `var a
= true; var b = true`. But this may not be safe if we need to
initialize any other global variables between `a` and `b`, and the
initialization involves calling a user-defined function that reassigns
`a`.
This CL changes staticinit to keep track of the initialization
expressions that we've seen so far, and to stop applying the
staticcopy optimization once we've seen an initialization expression
that might have modified another global variable within this package.
To help identify affected initializers, this CL adds a -d=staticcopy
flag to warn when a staticcopy is suppressed and turned into a dynamic
copy.
Currently, `go build -gcflags=all=-d=staticcopy std` reports only four
instances:
```
encoding/xml/xml.go:1600:5: skipping static copy of HTMLEntity+0 with map[string]string{...}
encoding/xml/xml.go:1869:5: skipping static copy of HTMLAutoClose+0 with []string{...}
net/net.go:661:5: skipping static copy of .stmp_31+0 with poll.ErrNetClosing
net/http/transport.go:2566:5: skipping static copy of errRequestCanceled+0 with ~R0
```
Fixes#51913.
Change-Id: Iab41cf6f84c44f7f960e4e62c28a8aeaade4fbcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/395541
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When creating the struct type to hold variables captured by a function
literal, we currently reuse the captured variable names as fields.
However, there's no particular reason to do this: these struct types
aren't visible to users, and it adds extra complexity in making sure
fields belong to the correct packages.
Further, it turns out we were getting that subtly wrong. If two
function literals from different packages capture variables with
identical names starting with an uppercase letter (and in the same
order and with corresponding identical types) end up in the same
function (e.g., due to inlining), then we could end up creating
closure struct types that are "different" (i.e., not types.Identical)
yet end up with equal LinkString representations (which violates
LinkString's contract).
The easy fix is to just always use simple, exported, generated field
names in the struct. This should allow further struct reuse across
packages too, and shrink binary sizes slightly.
Fixes#62498.
Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
One of the more tedious quirks of the original frontend (i.e.,
typecheck) to preserve was that it preserved the original
representation of constants into the backend. To fit into the unified
IR model, I ended up implementing a fairly heavyweight workaround:
simply record the original constant's string expression in the export
data, so that diagnostics could still report it back, and match the
old test expectations.
But now that there's just a single frontend to support, it's easy
enough to just update the test expectations and drop this support for
"raw" constant expressions.
Change-Id: I1d859c5109d679879d937a2b213e777fbddf4f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/526376
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Unfortunately, there isn't a single op that provides the resulting
computation.
At least, I couldn't find one.
Fixes#62469
Change-Id: I236f3965b827aaeb3d70ef9fe89be66b116494f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/526276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
This reverts commit c1dfbf72e1.
Reason for revert: TESTL rule is wrong when the result is used for an ordered comparison.
Fixes#62360
Change-Id: I4d5b6aca24389b0a2bf767bfbc0a9d085359eb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/524255
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Jakub Ciolek <jakub@ciolek.dev>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
In CL 522879, I moved the logic for setting Addrtaken from typecheck's
markAddrOf and ComputeAddrtaken directly into ir.NewAddrExpr. However,
I took the logic from markAddrOf, and failed to notice that
ComputeAddrtaken also set Addrtaken on the canonical ONAME.
The result is that if the only address-of expressions were within a
function literal, the canonical variable never got marked Addrtaken.
In turn, this could cause the consistency check in ir.Reassigned to
fail. (Yay for consistency checks turning mistakes into ICEs, rather
than miscompilation.)
Fixes#62313.
Change-Id: Ieab2854cd7fcc1b6c5d1e61de66453add9890a4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/523375
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Also gofmt a test file to make sure the parser works.
Fixes#62267.
Change-Id: I9b9f12b06bae7df626231000879b5ed7df3cd9ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/522635
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
This test passes "-linkmode=external" to 'go run' to link the binary
using the system C linker.
CGO_ENABLED=0 explicitly tells cmd/go not to use the C toolchain,
so the test should not be run in that configuration.
Updates #46330.
Change-Id: I16ac66aac91178045f9decaeb28134061e9711f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/522495
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Currently, we rewrite:
go f(new(T))
into:
tmp := new(T)
go func() { f(tmp) }()
However, we can both shrink the closure and improve escape analysis by
instead rewriting it into:
go func() { f(new(T)) }()
This CL does that.
Change-Id: Iae16a476368da35123052ca9ff41c49159980458
Reviewed-on: https://go-review.googlesource.com/c/go/+/520340
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Normalizing go/defer statements to always use functions with zero
parameters and zero results was added to escape analysis, because that
was the earliest point at which all three frontends converged. Now
that we only have the unified frontend, we can do it during typecheck,
which is where we perform all other desugaring and normalization
rewrites.
Change-Id: Iebf7679b117fd78b1dffee2974bbf85ebc923b23
Reviewed-on: https://go-review.googlesource.com/c/go/+/520260
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
For aggregate-typed arguments passed to a call, expandCalls
decomposed them into parts in the same block where the value
was created. This is not necessarily the call block, and in
the case where stores are involved, can change the memory
leaving that block, and getting that right is problematic.
Instead, do all the expanding in the same block as the call,
which avoids the problems of (1) not being able to reorder
loads/stores across a block boundary to conform to memory
order and (2) (incorrectly, not) exposing the new memory to
consumers in other blocks. Putting it all in the same block
as the call allows reordering, and the call creates its own
new memory (which is already dealt with correctly).
Fixes#61992.
Change-Id: Icc7918f0d2dd3c480cc7f496cdcd78edeca7f297
Reviewed-on: https://go-review.googlesource.com/c/go/+/519276
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Open-coded defer slots are assigned indices upfront, so they're
logically like elements in an array. Without reassigning the indices,
we need to keep all of the elements alive so their relative offsets
are correct.
Fixes#61895.
Change-Id: Ie0191fdb33276f4e8ed0becb69086524fff022b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/517856
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 497276 added optimization for len(string([]byte)) by avoiding call to
slicebytetostring. However, the bytes to string expression may contain
init nodes, which need to be preserved. Otherwise, it would make the
liveness analysis confusing about the lifetime of temporary variables
created by init nodes.
Fixes#61778
Change-Id: I6d1280a7d61bcc75f11132af41bda086f084ab54
Reviewed-on: https://go-review.googlesource.com/c/go/+/516375
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler/assembler's -S output prints relocation type
numerically, which is hard to understand. Every time I need to
count the relocation type constants to figure out which relocation
it actually is. Print the symbolic name instead.
Change-Id: I4866873bbae8b3dc0ee212609cb00280f9164243
Reviewed-on: https://go-review.googlesource.com/c/go/+/501856
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
User code is unlikely to be correct, but don't crash the compiler
when the offset of a pointer in an object is not a multiple of the
pointer size.
Fixes#61187
Change-Id: Ie56bfcb38556c5dd6f702ae4ec1d4534c6acd420
Reviewed-on: https://go-review.googlesource.com/c/go/+/508555
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
The large-function phi placement algorithm evidently doesn't like the
same pseudo-variable being used to represent expressions of varying
types.
Instead, use the same tactic as used for "valVar" (ssa.go:6585--6587),
which is to just generate a fresh marker node each time.
Maybe we could just use the OMIN/OMAX nodes themselves as the key
(like we do for OANDAND/OOROR), but that just seems needlessly risky
for negligible memory savings. Using fresh marker values each time
seems obviously safe by comparison.
Fixes#61041.
Change-Id: Ie2600c9c37b599c2e26ae01f5f8a433025d7fd08
Reviewed-on: https://go-review.googlesource.com/c/go/+/506679
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
For float or string, min/max builtin performs a runtime call, so we need
to save its result to temporary variable. Otherwise, the runtime call
will clobber closure's arguments currently on the stack when passing
min/max as argument to closures.
Fixes#60990
Change-Id: I1397800f815ec7853182868678d0f760b22afff2
Reviewed-on: https://go-review.googlesource.com/c/go/+/506115
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Our large-function phi placement algorithm is incompatible with phi
opcodes already existing in the SSA representation. Instead, use simple
variable assignments and have the phi placement algorithm place the phis
we need for min/max.
Turns out the small-function phi placement algorithm doesn't have this
sensitivity, so this bug only occurs in large functions (>500 basic blocks).
Maybe we should document/check that no phis are present when we start
phi placement (regardless of size). Leaving for a potential separate CL.
We should probably also fix the placement algorithm to handle existing
phis correctly. But this CL is probably a lot smaller/safer than
messing with phi placement.
Fixes#60982
Change-Id: I59ba7f506c72b22bc1485099a335d96315ebef67
Reviewed-on: https://go-review.googlesource.com/c/go/+/505756
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
CL 410344 fixed missing method value wrapper, by visiting body of
wrapper function after applying inlining pass.
CL 492017 allow more inlining of functions that construct closures,
which ends up making the wrapper function now inlineable, but can
contain closure nodes that couldn't be inlined. These closures body may
contain OMETHVALUE nodes that we never seen, thus we need to scan
closures body for finding them.
Fixes#60945
Change-Id: Ia1e31420bb172ff87d7321d2da2989ef23e6ebb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/505255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Change-Id: Idd872c5b90dbca564ed8a37bb3683e642142ae63
Reviewed-on: https://go-review.googlesource.com/c/go/+/505015
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
types2 have already errored about any spec-required overflows, and
division by zero. CL 469595 unintentionally fixed typecheck not to error
about overflows, but zero division is still be checked during tcArith.
This causes unsafe operations with variable size failed to compile,
instead of raising runtime error.
Fixes#60601
Change-Id: I7bea2821099556835c920713397f7c5d8a4025ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/501735
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 496257 adds min/max builtins, which may appear as argument to a
function call, so it will be tested by mayCall. But those ops are not
handled by mayCall, causes the compiler crashes.
Fixes#60582
Change-Id: I729f10bf62b4aad39ffcb1433f576e74d09fdd9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/500575
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
HAUTO should be handled the same way as other stack offsets for
adding to constant pool. Add the missing cases.
Fixes#57955.
Change-Id: If7fc82cafb2bbf0a6121e73e353b8825cb36b5bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/463138
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Some testdir tests fail if GOEXPERIMENT=cgocheck2 is set. Fix this by
skipping these tests.
Change-Id: I58d4ef0cceb86bcf93220b4a44de9b9dc4879b16
Reviewed-on: https://go-review.googlesource.com/c/go/+/499675
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Two interface types that are assignable don't have to be identical;
specifically, if they are defined types, they can be different
defined types. If those defined types specify type parameters which
are never used, do not infer a type argument based on the instantiation
of a matching defined type.
Adjusted three existing tests where we inferred type arguments incorrectly.
Fixes#60377.
Change-Id: I91fb207235424b3cbc42b5fd93eee619e7541cb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/498315
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This enables the implementation for proposal #58671, which is
a likely accept. By enabling it early we get a bit extra soak
time for this feature. The change can be reverted trivially, if
need be.
For #58671.
Change-Id: Id6c27515e45ff79f4f1d2fc1706f3f672ccdd1ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/495955
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If the function referenced by a closure expression is incorporated
into a static init, be sure to mark it as non-hidden, since otherwise
it will be live but no longer reachable from the init func, hence it
will be skipped during escape analysis, which can lead to
miscompilations.
Fixes#59680.
Change-Id: Ib858aee296efcc0b7655d25c23ab8a6a8dbdc5f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/492135
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
[This is a roll-forward of CL 484859, this time including a fix for
issue #59709. The call to do dead function marking was taking place in
the wrong spot, causing it to run more than once if generics were
instantiated.]
This patch generalizes the code in the inliner that marks unreferenced
hidden closure functions as dead. Rather than doing the marking on the
fly (previous approach), this new approach does a single pass at the
end of inlining, which catches more dead functions.
Change-Id: I0e079ad755c21295477201acbd7e1a732a98fffd
Reviewed-on: https://go-review.googlesource.com/c/go/+/492016
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Previously, type arguments could only be inferred for generic
functions in call expressions, whereas with the reverse type inference
proposal they can now be inferred in assignment contexts too. As a
consequence, we now need to check Info.Instances to find the inferred
type for more cases now.
Updates #59338.
Fixes#59955.
Change-Id: I9b6465395869459c2387d0424febe7337b28b90e
Reviewed-on: https://go-review.googlesource.com/c/go/+/492455
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Marking variables in erroneous variable declarations as used is
convenient for tests but doesn't necessarily hide follow-on errors
in real code: either the variable is not supposed to be declared in
the first place and then we should get an error if it is not used,
or it is there because it is intended to be used, and the we expect
an error it if is not used.
This brings types2 closer to go/types.
Change-Id: If7ee1298fc770f7ad0cefe7e968533fd50ec2343
Reviewed-on: https://go-review.googlesource.com/c/go/+/486175
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This reverts commit http://go.dev/cl//484859
Reason for revert: causes linker errors in a number of google-internal tests.
Change-Id: I322252f784a46d2b1d447ebcdca86ce14bc0cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/485755
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
This patch generalizes the code in the inliner that marks unreferenced
hidden closure functions as dead. Rather than doing the marking on the
fly (previous approach), this new approach does a single pass at the
end of inlining, which catches more dead functions.
Fixes#59638.
Updates #59404.
Updates #59547.
Change-Id: I54fd63e9e37c9123b08a3e7def7d1989919bba91
Reviewed-on: https://go-review.googlesource.com/c/go/+/484859
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Static init inliner is using typecheck.EvalConst to handle string
concatenation expressions. But static init inliner may reveal constant
expressions after substitution, and the compiler needs to evaluate those
expressions in non-constant semantic. Using typecheck.EvalConst, which
always evaluates expressions in constant semantic, is not the right
choice.
For safety, this CL fold the logic to handle string concatenation to
static init inliner, so there won't be regression in handling constant
expressions in non-constant semantic. And also, future CL can simplify
typecheck.EvalConst logic.
Updates #58293
Updates #58339Fixes#58439
Change-Id: I74068d99c245938e576afe9460cbd2b39677bbff
Reviewed-on: https://go-review.googlesource.com/c/go/+/466277
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
(This is a retry of CL 462035 which was reverted at 474976.
The only change from that CL is the aix fix SRODATA->SNOPTRDATA
at inittask.go:141)
As described here:
https://github.com/golang/go/issues/31636#issuecomment-493271830
"Find the lexically earliest package that is not initialized yet,
but has had all its dependencies initialized, initialize that package,
and repeat."
Simplify the runtime a bit, by just computing the ordering required
in the linker and giving a list to the runtime.
Update #31636Fixes#57411
RELNOTE=yes
Change-Id: I28c09451d6aa677d7394c179d23c2c02c503fc56
Reviewed-on: https://go-review.googlesource.com/c/go/+/478916
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Range statement will mutate the key and value, so we should treat them as reassigned.
Fixes#59572
Change-Id: I9c6b67d938760a0c6a1d9739f2737c67af4a3a10
Reviewed-on: https://go-review.googlesource.com/c/go/+/483855
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#58141
Co-authored-by: Richard Musiol <neelance@gmail.com>
Co-authored-by: Achille Roussel <achille.roussel@gmail.com>
Co-authored-by: Julien Fabre <ju.pryz@gmail.com>
Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I49b66946acc90fdf09ed9223096bfec9a1e5b923
Reviewed-on: https://go-review.googlesource.com/c/go/+/479627
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
So iterators that are in progress can know entries have been deleted and
terminate the iterator properly.
Update #55002
Update #56351Fixes#59411
Change-Id: I924f16a00fe4ed6564f730a677348a6011d3fb67
Reviewed-on: https://go-review.googlesource.com/c/go/+/481935
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Use the type of the store for the byteswap, not the type of the
store's value argument.
Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.
If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.
Fixes#59367
Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
When a closure is inlined, it may contain other hidden closures, which
the inliner will duplicate, rendering the original nested closures as
unreachable. Because they are unreachable, they don't get processed in
escape analysis, meaning that go/defer statements don't get rewritten,
which can then in turn trigger errors in walk. This patch looks for
nested hidden closures and marks them as dead, so that they can be
skipped later on in the compilation flow. NB: if during escape
analysis we rediscover a hidden closure (due to an explicit reference)
that was previously marked dead, revive it at that point.
Fixes#59404.
Change-Id: I76db1e9cf1ee38bd1147aeae823f916dbbbf081b
Reviewed-on: https://go-review.googlesource.com/c/go/+/482355
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Unified IR already records the correct type for them.
Fixes#59378
Change-Id: I275c45b48f67bde55c8e2079d60b5868d0acde7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/481555
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Casting to a *uintptr is not ok if there isn't at least 8 bytes of
data backing that pointer (on 64-bit archs).
So although we end up making a slice of 0 length with that pointer,
the cast itself doesn't know that.
Instead, bail early if the result is going to be 0 length.
Fixes#59334
Change-Id: Id3c0e09d341d838835c0382cccfb0f71dc3dc7e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/480575
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Change the Checker.use/useLHS functions to report if all "used"
expressions evaluated without error. Use that information to
control whether to report an assignment mismatch error or not.
This will reduce the number of errors reported per assignment,
where the assignment mismatch is only one of the errors.
Change-Id: Ia0fc3203253b002e4e1d5759d8d5644999af6884
Reviewed-on: https://go-review.googlesource.com/c/go/+/478756
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
For #55242
Change-Id: I092b1881623ea997b178d038c0afd10cd5bca937
Reviewed-on: https://go-review.googlesource.com/c/go/+/479898
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
unsafe.SliceData can return pointers which are nil. That function gets
lowered to the SSA OpSlicePtr, which the compiler assumes is non-nil.
This used to be the case as OpSlicePtr was only used in situations
where the bounds check already passed. But with unsafe.SliceData that
is no longer the case.
There are situations where we know it is nil. Use Bounded() to
indicate that.
I looked through all the uses of OSPTR and added SetBounded where it
made sense. Most OSPTR results are passed directly to runtime calls
(e.g. memmove), so even if we know they are non-nil that info isn't
helpful.
Fixes#59293
Change-Id: I437a15330db48e0082acfb1f89caf8c56723fc51
Reviewed-on: https://go-review.googlesource.com/c/go/+/479896
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
As with changes in prior CLs, we don't suppress legitimate
"declared but not used" errors anymore simply because the
respective variables are used in incorrect assignments,
unrelated to the variables in question.
Adjust several (ancient) tests accordingly.
Change-Id: I5826393264d9d8085c64777a330d4efeb735dd2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/478716
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
This CL re-introduces useLHS because we don't want to suppress
correct "declared but not used" errors for variables that only
appear on the LHS of an assignment (using Checker.use would mark
them as used).
This CL also adjusts a couple of places where types2 differed
from go/types (and suppressed valid "declared and not used"
errors). Now those errors are surfaced. Adjusted a handful of
tests accordingly.
Change-Id: Ia555139a05049887aeeec9e5221b1f41432c1a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/478635
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Don't say "array length must be integer" if it is in fact an integer.
Fixes#59209
Change-Id: If60b93a0418f5837ac334412d3838eec25eeb855
Reviewed-on: https://go-review.googlesource.com/c/go/+/479115
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In the Sizes API, recognize an overflow (to a negative value) as a
consequence of an oversize value, and specify as such in the API.
Adjust the various size computations to take overflow into account.
Recognize a negative size or offset as an error and report it rather
than panicking.
Use the same protocol for results provided by the default (StdSizes)
and external Sizes implementations.
Add a new error code TypeTooLarge for the new errors.
Fixes#59190.
Fixes#59207.
Change-Id: I8c33a9e69932760275100112dde627289ac7695b
Reviewed-on: https://go-review.googlesource.com/c/go/+/478919
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
For #59169
Change-Id: Id72ad9fe8b6e1d7cf64f972520ae8858f70c025a
Reviewed-on: https://go-review.googlesource.com/c/go/+/478217
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
This reverts commit ce2a609909.
aka CL 462035
Reason for revert: this CL is causing some problems in some internal Google programs.
Change-Id: I4476b8d8d2c3d7b5703d1d85c93baebb4b4e5d26
Reviewed-on: https://go-review.googlesource.com/c/go/+/474976
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
On ARM64, in -dynlink mode (building a shared library or a plugin),
accessing global variable is made using the GOT. Currently, the
GOT accessing instruction sequence our assembler generates doesn't
handle large offset well, so we don't fold the offset into loads
and stores in the compiler. Currently, the rewrite rules are
guarded with the -shared flag. However, the GOT access
instructions are only generated in the -dynlink mode (which
implies -shared, but not the other direction).
CL 445535 attempted to remove the guard althgether. But that
causes build failure for -dynlink mode for the reason above. This
CL changes it to guard specifically on -dynlink mode, allowing
the optimization in more cases (-shared but not -dynlink build
modes).
Updates #58826.
Change-Id: I1391db6a33e8d0455a304e7cae7fcfdeb49bfdab
Reviewed-on: https://go-review.googlesource.com/c/go/+/473999
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Adds:
GOEXPERIMENT=loopvar (expected way of invoking)
-d=loopvar={-1,0,1,2,11,12} (for per-package control and/or logging)
-d=loopvarhash=... (for hash debugging)
loopvar=11,12 are for testing, benchmarking, and debugging.
If enabled,for loops of the form `for x,y := range thing`, if x and/or
y are addressed or captured by a closure, are transformed by renaming
x/y to a temporary and prepending an assignment to the body of the
loop x := tmp_x. This changes the loop semantics by making each
iteration's instance of x be distinct from the others (currently they
are all aliased, and when this matters, it is almost always a bug).
3-range with captured iteration variables are also transformed,
though it is a more complex transformation.
"Optimized" to do a simpler transformation for
3-clause for where the increment is empty.
(Prior optimization of address-taking under Return disabled, because
it was incorrect; returns can have loops for children. Restored in
a later CL.)
Includes support for -d=loopvarhash=<binary string> intended for use
with hash search and GOCOMPILEDEBUG=loopvarhash=<binary string>
(use `gossahash -e loopvarhash command-that-fails`).
Minor feature upgrades to hash-triggered features; clients can specify
that file-position hashes use only the most-inline position, and/or that
they use only the basenames of source files (not the full directory path).
Most-inlined is the right choice for debugging loop-iteration change
once the semantics are linked to the package across inlining; basename-only
makes it tractable to write tests (which, otherwise, depend on the full
pathname of the source file and thus vary).
Updates #57969.
Change-Id: I180a51a3f8d4173f6210c861f10de23de8a1b1db
Reviewed-on: https://go-review.googlesource.com/c/go/+/411904
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
As described here:
https://github.com/golang/go/issues/31636#issuecomment-493271830
"Find the lexically earliest package that is not initialized yet,
but has had all its dependencies initialized, initialize that package,
and repeat."
Simplify the runtime a bit, by just computing the ordering required
in the linker and giving a list to the runtime.
Update #31636Fixes#57411
RELNOTE=yes
Change-Id: I1e4d3878ebe6e8953527aedb730824971d722cac
Reviewed-on: https://go-review.googlesource.com/c/go/+/462035
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
A 0-sized no-op shouldn't prevent us from detecting that the first
instruction is from an inlined callee.
Update #58300
Change-Id: Ic5f6ed108c54a32c05e9b2264b516f2cc17e4619
Reviewed-on: https://go-review.googlesource.com/c/go/+/467977
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The assertion here was to make sure the newly constructed and
typechecked expression selected the same receiver-qualified method,
but in the case of anonymous receiver types we can actually end up
with separate types.Field instances corresponding to each types.Type
instance. In that case, the assertion spuriously failed.
The fix here is to relax and assertion and just compare the method's
name and type (including receiver type).
Fixes#58563.
Change-Id: I67d51ddb020e6ed52671473c93fc08f283a40886
Reviewed-on: https://go-review.googlesource.com/c/go/+/471676
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
For go/defer calls like "defer f(x, y)", the compiler rewrites it to:
x1, y1 := x, y
defer func() { f(x1, y1) }()
However, if "f" needs runtime type information, the "RType" field will
refer to the outer ".dict" param, causing wrong liveness analysis.
To fix this, if "f" refers to outer ".dict", the dict param will be
copied to an autotmp, and "f" will refer to this autotmp instead.
Fixes#58341
Change-Id: I238b6e75441442b5540d39bc818205398e80c94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/466035
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There are a plenty of regression in 1.20 with this optimization. This CL
disable inline static init, so it's safer to backport to 1.20 branch.
The optimization will be enabled again during 1.21 cycle.
Updates #58293
Updates #58339
For #58293
Change-Id: If5916008597b46146b4dc7108c6b389d53f35e95
Reviewed-on: https://go-review.googlesource.com/c/go/+/467015
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This patch fixes a panic from incorrect interface conversion from
*ir.BasicLit to *ir.ConstExpr. This only occurs when nounified
GOEXPERIMENT is set, so ideally it should be backported to Go
1.20 and removed from master.
Fixes#58339
Change-Id: I357069d7ee1707d5cc6811bd2fbdd7b0456323ae
GitHub-Last-Rev: 641dedb5f9
GitHub-Pull-Request: golang/go#58389
Reviewed-on: https://go-review.googlesource.com/c/go/+/466175
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Since go1.19, these errors are already reported by types2 for any user's
Go code. Compiler generated code, which looks like constant expression
should be evaluated as non-constant semantic, which allows overflows.
Fixes#58293
Change-Id: I6f0049a69bdb0a8d0d7a0db49c7badaa92598ea2
Reviewed-on: https://go-review.googlesource.com/c/go/+/465096
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
This gets eliminated by thoses rules above:
// for rewriting results of some late-expanded rewrites (below)
(SelectN [0] (MakeResult x ___)) => x
(SelectN [1] (MakeResult x y ___)) => y
(SelectN [2] (MakeResult x y z ___)) => z
Fixes#58161
Change-Id: I4fbfd52c72c06b6b3db906bd9910b6dbb7fe8975
Reviewed-on: https://go-review.googlesource.com/c/go/+/463846
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Blank node must be ignored when building arguments substitued tree.
Otherwise, it could be used to replace other blank node in left hand
side of an assignment, causing an invalid IR node.
Consider the following code:
type S1 struct {
s2 S2
}
type S2 struct{}
func (S2) Make() S2 {
return S2{}
}
func (S1) Make() S1 {
return S1{s2: S2{}.Make()}
}
var _ = S1{}.Make()
After staticAssignInlinedCall, the assignment becomes:
var _ = S1{s2: S2{}.Make()}
and the arg substitued tree is "map[*ir.Name]ir.Node{_: S1{}}". Now,
when doing static assignment, if there is any assignment to blank node,
for example:
_ := S2{}
That blank node will be replaced with "S1{}":
S1{} := S2{}
So constructing an invalid IR which causes the ICE.
Fixes#58325
Change-Id: I21b48357f669a7e02a7eb4325246aadc31f78fb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/465098
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
CL 458619 fixed the problem un-intentionally, so adding test to prevent
regression happening.
Updates #58345
Change-Id: I80cf60716ef85e142d769e8621fce19c826be03d
Reviewed-on: https://go-review.googlesource.com/c/go/+/465455
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
People are using this to get the name of the function from a function type:
runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name()
Unfortunately, this technique falls down when the first instruction
of the function is from an inlined callee. Then the expression above
gets you the name of the inlined function instead of the function itself.
To fix this, ensure that the first instruction is never from an inlinee.
Normally functions have prologs so those are already fine. In just the
cases where a function is a leaf with no local variables, and an instruction
from an inlinee appears first in the prog list, add a nop at the start
of the function to hold a non-inlined position.
Consider the nop a "mini-prolog" for leaf functions.
Fixes#58300
Change-Id: Ie37092f4ac3167fe8e5ef4a2207b14abc1786897
Reviewed-on: https://go-review.googlesource.com/c/go/+/465076
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: David Chase <drchase@google.com>
ir.VisitFuncsBottomUp returns recursive==true for functions which
call themselves. It also returns any closures inside that function.
We don't want to report the closures as recursive, as they really
aren't. Only the containing function is recursive.
Fixes#54159
Change-Id: I3b4d6710a389ec1d6b250ba8a7065f2e985bdbe1
Reviewed-on: https://go-review.googlesource.com/c/go/+/463233
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
This CL removes the GOEXPERIMENT=nounified knob, and any conditional
statements that depend on that knob. Further CLs to remove unreachable
code follow this one.
Updates #57410.
Change-Id: I39c147e1a83601c73f8316a001705778fee64a91
Reviewed-on: https://go-review.googlesource.com/c/go/+/458615
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This has been investigated and explained on the issue tracker.
Fixes#54402.
Change-Id: I4d8b971faa810591983ad028b7db16411f3b3b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/461456
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for #56986.
Fixes#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
