A composite literal assignment
x = T{field: v}
may be compiled to
x = T{}
x.field = v
We already do not use this form is RHS uses LHS. If LHS is
address-taken, RHS may uses LHS implicitly, e.g.
v = &x.field
x = T{field: *v}
The lowering above would change the value of RHS (*v).
Fixes#52953.
Change-Id: I3f798e00598aaa550b8c17182c7472fef440d483
Reviewed-on: https://go-review.googlesource.com/c/go/+/407014
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Change-Id: I63eb42f3ce5ca452279120a5b33518f4ce16be45
GitHub-Last-Rev: a88f2f72be
GitHub-Pull-Request: golang/go#52951
Reviewed-on: https://go-review.googlesource.com/c/go/+/406843
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
For #51291
Change-Id: If47e4cbf899853ade5050852c3870b9500da4c63
Reviewed-on: https://go-review.googlesource.com/c/go/+/406916
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
CL 406358 added test that use -buildmode=plugin. But plugin mode only
supports on some os/arch pairs, so this CL moving the test to
misc/cgo/testplugin directory instead.
Updates #52937
Change-Id: Iad049443c1f6539f6af1988bebd4dff56c6e1bf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/406774
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. This is intended to be the reverse case of
CL 361260.
However, CL 361260 is using wider condition than necessary. Though it
only needs to check against function parameters, it checks whether the
function type has no shape. It does not cause any issue, because
!fn.Type().HasShape() implies !fn.Type().Params().HasShape().
But for the reverse case, it's not true. Function may have shape type,
but has no shape arguments. Thus, we must tighten the condition to
explicitly check against the function parameters only.
Fixes#52907
Change-Id: Ib87e87ff767c31d99d5b36aa4a6c1d8baf32746d
Reviewed-on: https://go-review.googlesource.com/c/go/+/406475
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Generic functions require instantiation, which package plugin doesn't
support, and likely never will. So instead, we can just skip writing
out any generic functions, which avoids an ICE in the plugin
generation code.
This issue doesn't affect GOEXPERIMENT=unified, because it avoids
leaking any non-instantiated types/functions to the rest of the
compiler backend.
Fixes#52937.
Change-Id: Ie35529c5c241e46b77fcb5b8cca48bb99ce7bfcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/406358
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
This test is currently overly sensitive to compiler optimizations,
because inlining can affect the order in which cmd/link emits field
references. The order doesn't actually matter though, so this CL just
tweaks the test to sort the tracked fields before printing them.
Updates #51734.
Change-Id: I3b65ca265856b2e1102f40406d5ce34610c70d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/406674
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Since CL 391014, cmd/compile now requires the -p flag to be set the
build system. This CL changes it to initialize LocalPkg.Path to the
provided path, rather than relying on writing out `"".` into object
files and expecting cmd/link to substitute them.
However, this actually involved a rather long tail of fixes. Many have
already been submitted, but a few notable ones that have to land
simultaneously with changing LocalPkg:
1. When compiling package runtime, there are really two "runtime"
packages: types.LocalPkg (the source package itself) and
ir.Pkgs.Runtime (the compiler's internal representation, for synthetic
references). Previously, these ended up creating separate link
symbols (`"".xxx` and `runtime.xxx`, respectively), but now they both
end up as `runtime.xxx`, which causes lsym collisions (notably
inittask and funcsyms).
2. test/codegen tests need to be updated to expect symbols to be named
`command-line-arguments.xxx` rather than `"".foo`.
3. The issue20014 test case is sensitive to the sort order of field
tracking symbols. In particular, the local package now sorts to its
natural place in the list, rather than to the front.
Thanks to David Chase for helping track down all of the fixes needed
for this CL.
Updates #51734.
Change-Id: Iba3041cf7ad967d18c6e17922fa06ba11798b565
Reviewed-on: https://go-review.googlesource.com/c/go/+/393715
Reviewed-by: David Chase <drchase@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>
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. But it does not consider the case where
function has shape params, but passing zero arguments. In this case, the
un-safe interface conversion that may be applied to a shape argument can
not happen, so it's safe to inline the function.
Fixes#52907
Change-Id: Ifa7b23709bb47b97e27dc1bf32343d92683ef783
Reviewed-on: https://go-review.googlesource.com/c/go/+/406176
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is the same fix as CL 36126, but for the reverse case, function
with shape params but passed no shape arg. The same conversion problem
may occur in this case, see details explanation there.
Fixes#51909Fixes#51925
Change-Id: Ib0c1973c7511d85b4918a252c80060f1864180cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/395854
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
For #52841
Change-Id: If4723a70fba0dbedb5d1e70dab58f0b4612bf8b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/405759
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 391014 requires the compiler to be invoked with the -p flag, to
specify the package path. Later, CL 394217 makes the compiler to
produce an unlinkable object file, so "go tool compile x.go" can
still be used on the command line. This CL does the same for the
assembler, requiring -p, otherwise generating an unlinkable object.
No special case for the main package, as the main package cannot
be only assembly code, and there is no way to tell if it is the
main package from an assembly file.
Now we guarantee that we always have an expanded package path in
the object file. A later CL will delete the name expansion code
in the linker.
Change-Id: I8c10661aaea2ff794614924ead958d80e7e2487d
Reviewed-on: https://go-review.googlesource.com/c/go/+/404298
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler use to compile f()(g()) as:
t1, t2 := g()
f()(t1, t2)
That violates the Go spec, since when "..., all function calls, ... are
evaluated in lexical left-to-right order"
This PR fixes the bug by compiling f()(g()) as:
t0 := f()
t1, t2 := g()
t0(t1, t2)
to make "f()" to be evaluated before "g()".
Fixes#50672
Change-Id: I6a766f3dfc7347d10f8fa3a151f6a5ea79bcf818
Reviewed-on: https://go-review.googlesource.com/c/go/+/392834
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
math/bits.Add64 and math/bits.Sub64 now lower and optimize
directly in SSA form.
The optimization of carry chains focuses around eliding
XER<->GPR transfers of the CA bit when used exclusively as an
input to a single carry operations, or when the CA value is
known.
This also adds support for handling XER spills in the assembler
which could happen if carry chains contain inter-dependencies
on each other (which seems very unlikely with practical usage),
or a clobber happens (SRAW/SRAD/SUBFC operations clobber CA).
With PPC64 Add64/Sub64 lowering into SSA and this patch, the net
performance difference in crypto/elliptic benchmarks on P9/ppc64le
are:
name old time/op new time/op delta
ScalarBaseMult/P256 46.3µs ± 0% 46.9µs ± 0% +1.34%
ScalarBaseMult/P224 356µs ± 0% 209µs ± 0% -41.14%
ScalarBaseMult/P384 1.20ms ± 0% 0.57ms ± 0% -52.14%
ScalarBaseMult/P521 3.38ms ± 0% 1.44ms ± 0% -57.27%
ScalarMult/P256 199µs ± 0% 199µs ± 0% -0.17%
ScalarMult/P224 357µs ± 0% 212µs ± 0% -40.56%
ScalarMult/P384 1.20ms ± 0% 0.58ms ± 0% -51.86%
ScalarMult/P521 3.37ms ± 0% 1.44ms ± 0% -57.32%
MarshalUnmarshal/P256/Uncompressed 2.59µs ± 0% 2.52µs ± 0% -2.63%
MarshalUnmarshal/P256/Compressed 2.58µs ± 0% 2.52µs ± 0% -2.06%
MarshalUnmarshal/P224/Uncompressed 1.54µs ± 0% 1.40µs ± 0% -9.42%
MarshalUnmarshal/P224/Compressed 1.54µs ± 0% 1.39µs ± 0% -9.87%
MarshalUnmarshal/P384/Uncompressed 2.40µs ± 0% 1.80µs ± 0% -24.93%
MarshalUnmarshal/P384/Compressed 2.35µs ± 0% 1.81µs ± 0% -23.03%
MarshalUnmarshal/P521/Uncompressed 3.79µs ± 0% 2.58µs ± 0% -31.81%
MarshalUnmarshal/P521/Compressed 3.80µs ± 0% 2.60µs ± 0% -31.67%
Note, P256 uses an asm implementation, thus, little variation is expected.
Change-Id: I88a24f6bf0f4f285c649e40243b1ab69cc452b71
Reviewed-on: https://go-review.googlesource.com/c/go/+/346870
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
This issue has been fixed in CL 403837.
Fixes#51840.
Change-Id: I282062bb06278696fe25e9ede333c64539dc964e
Reviewed-on: https://go-review.googlesource.com/c/go/+/404914
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
When a fully instantiated generic method is exported, be sure to also
export the types in its signature.
Fixes#52279.
Change-Id: Icc6bca05b01f914cf67faaf1bf184eaa5484f521
Reviewed-on: https://go-review.googlesource.com/c/go/+/405118
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Following CL 405114, the extension rule is also wrong. It is safe
to drop the extension if the value is from a boolean-generating
instruction, but not a boolean-typed Value in general (e.g. a Phi
or a in-register parameter). Fix it.
Updates #52788.
Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/405115
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
On ARM64, an If block is lowered to (NZ cond yes no). This is
incorrect because cond is a boolean value and therefore only the
last byte is meaningful (same as AMD64, see ARM64Ops.go). But here
we are comparing a full register width with 0. Correct it by
comparing only the last bit.
Fixes#52788.
Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/405114
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
For this code:
z &= 63
_ = x<<z | x>>(64-z)
Now can prove 'x<<z' in bound. In ppc64 lowering pass, it will not
produce an extra '(ANDconst <typ.Int64> [63] z)' causing
codegen/rotate.go failed. Just remove the type check in rewrite rules
as the workaround.
Removes 32 bounds checks during make.bat.
Fixes#52563.
Change-Id: I14ed2c093ff5638dfea7de9bc7649c0f756dd71a
Reviewed-on: https://go-review.googlesource.com/c/go/+/404315
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
An invalid program may produce invalid types. If the program
calls unsafe.Sizeof on such a type, which is a compile-time
computation, the size-computation must be able to handle it.
Add the invalid type to the list of permissible basic types
and give it a size of 1 (word).
Fixes#52748.
Change-Id: I6c409628f9b77044758caf71cdcb199f9e77adea
Reviewed-on: https://go-review.googlesource.com/c/go/+/404894
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Noder pass will build a closure to implement generic function
instantiation which may produce `.dict` and `.rcvr` ident.
Since we allow `.dict` during exporting, we should allow `.rcvr` too.
Fixes#52241.
Change-Id: Ifc3912ba5155b5ac1887f20830da64f4fb3fceb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/404314
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For #52535
Change-Id: I6798a8379163497ebebcdadf836b8569735c282b
Reviewed-on: https://go-review.googlesource.com/c/go/+/404496
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
In walkCompare, any ir.OCONVNOP was removed from both operands. So when
constructing assignments for them to preserve any side-effects, using
temporary variables can cause type mismatched with original type.
Instead, using blank assignments will prevent that issue and still make
sure that the operands will be evaluated.
Fixes#52701
Change-Id: I229046acb154890bb36fe441d258563687fdce37
Reviewed-on: https://go-review.googlesource.com/c/go/+/403997
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
It is hit ~70k times building go.
This make the go binary, 0.04% smaller.
I didn't included benchmarks because this is just constant foldings
and is hard to mesure objectively.
For example, this enable rewriting things like:
if x == 20 {
return x + 30 + z
}
Into:
if x == 20 {
return 50 + z
}
It's not just fixing programer's code,
the ssa generator generate code like this sometimes.
Change-Id: I0861f342b27f7227b5f1c34d8267fa0057b1bbbc
GitHub-Last-Rev: 4c2f9b5216
GitHub-Pull-Request: golang/go#52669
Reviewed-on: https://go-review.googlesource.com/c/go/+/403735
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This shows up in a few crypto functions, and other
assorted places.
Change-Id: I5a7f4c25ddd4a6499dc295ef693b9fe43d2448ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/404057
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
When we convert a type to a shaped interface type, we are not able
to recognize the itab. So passing the itab by dictionary as the
workaround.
Fixes#52026.
Change-Id: I75c23c7dd215daf9761dc24116a8af2c28c6d948
Reviewed-on: https://go-review.googlesource.com/c/go/+/401034
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
In #52529, we observed that checking types for duplicate fields and
methods during method collection can result in incorrect early expansion
of the base type. Fix this by delaying the check for duplicate fields.
Notably, we can't delay the check for duplicate methods as we must
preserve the invariant that added method names are unique.
After this change, it may be possible in the presence of errors to have
a type-checked type containing a method name that conflicts with a field
name. With the previous logic conflicting methods would have been
skipped. This is a change in behavior, but only for invalid code.
Preserving the existing behavior would likely require delaying method
collection, which could have more significant consequences.
As a result of this change, the compiler test fixedbugs/issue28268.go
started passing with types2, being previously marked as broken. The fix
was not actually related to the duplicate method error, but rather the
fact that we stopped reporting redundant errors on the calls to x.b()
and x.E(), because they are now (valid!) methods.
Fixes#52529
Change-Id: I850ce85c6ba76d79544f46bfd3deb8538d8c7d00
Reviewed-on: https://go-review.googlesource.com/c/go/+/403455
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We need to use the same marker everywhere. My CL to rename the
marker (CL 241661) and the CL to add more uses of the marker
under the old name (CL 241678) weren't coordinated with each other.
Fixes#52612
Change-Id: I97023c0769e518491924ef457fe03bf64a2cefa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/403094
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Accept ~x as ordinary unary expression in the parser but recognize
such expressions as invalid in the type checker.
This change opens the door to recognizing complex type constraint
literals such as `*E|~int` in `[P *E|~int]` and parse them correctly
instead of reporting a parse error because `P*E|~int` syntactically
looks like an incorrect array length expression (binary expression
where the RHS of | is an invalid unary expression ~int).
As a result, the parser is more forgiving with expressions but the
type checker will reject invalid uses as before.
We could pass extra information into the binary/unary expression
parse functions to prevent the use of ~ in invalid situations but
it doesn't seem worth the trouble. In fact it may be advantageous
to allow a more liberal expression syntax especially in the presence
of errors (better parser synchronization after an error).
Preparation for fixing #49482.
Change-Id: I119e8bd9445dfa6460fcd7e0658e3554a34b2769
Reviewed-on: https://go-review.googlesource.com/c/go/+/402255
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Change-Id: Ia0a4be56d4e1fbfc73e6ce24f01a658c89a74adb
GitHub-Last-Rev: dd95e50c4b
GitHub-Pull-Request: golang/go#52393
Reviewed-on: https://go-review.googlesource.com/c/go/+/400694
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This CL exports the existing ir.UintptrKeepAlive via the new directive
//go:uintptrkeepalive. This makes the compiler insert KeepAlives for
pointers converted to uintptr in calls, keeping them alive for the
duration of the call.
//go:uintptrkeepalive requires //go:nosplit, as stack growth can't
handle these arguments (it cannot know which are pointers). We currently
check this on the immediate function, but the actual restriction applies
to all transitive calls.
The existing //go:uintptrescapes is an extension of
//go:uintptrkeepalive which forces pointers to escape to the heap, thus
eliminating the stack growth issue.
This pragma is limited to the standard library.
For #51087
Change-Id: If9a19d484d3561b4219e5539b70c11a3cc09391e
Reviewed-on: https://go-review.googlesource.com/c/go/+/388095
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 388095 will change this file significantly. Move it preemptively to
ensure git tracks the move properly.
For #51087
Change-Id: I1408aecf8675723041b64e54cf44cdec38cc655c
Reviewed-on: https://go-review.googlesource.com/c/go/+/388094
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fixes#52438.
Change-Id: I5cbf8c448dba037e9e0c5fe8f209401d6bf7d43f
Reviewed-on: https://go-review.googlesource.com/c/go/+/401134
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
The linker performs a global analysis of all nosplit call chains to
check they fit in the stack space ensured by splittable functions.
That analysis has two problems right now:
1. It's inefficient. It performs a top-down analysis, starting with
every nosplit function and the nosplit stack limit and walking *down*
the call graph to compute how much stack remains at every call. As a
result, it visits the same functions over and over, often with
different remaining stack depths. This approach is historical: this
check was originally written in C and this approach avoided the need
for any interesting data structures.
2. If some call chain is over the limit, it only reports a single call
chain. As a result, if the check does fail, you often wind up playing
whack-a-mole by guessing where the problem is in the one chain, trying
to reduce the stack size, and then seeing if the link works or reports
a different path.
This CL completely rewrites the nosplit stack check. It now uses a
bottom-up analysis, computing the maximum stack height required by
every function's call tree. This visits every function exactly once,
making it much more efficient. It uses slightly more heap space for
intermediate storage, but still very little in the scheme of the
overall link. For example, when linking cmd/go, the new algorithm
virtually eliminates the time spent in this pass, and reduces overall
link time:
│ before │ after │
│ sec/op │ sec/op vs base │
Dostkcheck 7.926m ± 4% 1.831m ± 6% -76.90% (p=0.000 n=20)
TotalTime 301.3m ± 1% 296.4m ± 3% -1.62% (p=0.040 n=20)
│ before │ after │
│ B/op │ B/op vs base │
Dostkcheck 40.00Ki ± 0% 212.15Ki ± 0% +430.37% (p=0.000 n=20)
Most of this time is spent analyzing the runtime, so for larger
binaries, the total time saved is roughly the same, and proportionally
less of the overall link.
If the new implementation finds an error, it redoes the analysis,
switching to preferring quality of error reporting over performance.
For error reporting, it computes stack depths top-down (like the old
algorithm), and reports *all* paths that are over the stack limit,
presented as a tree for compactness. For example, this is the output
from a simple test case from test/nosplit with two over-limit paths
from f1:
main.f1: nosplit stack overflow
main.f1
grows 768 bytes, calls main.f2
grows 56 bytes, calls main.f4
grows 48 bytes
80 bytes over limit
grows 768 bytes, calls main.f3
grows 104 bytes
80 bytes over limit
While we're here, we do a few nice cleanups:
- We add a debug output flag, which will be useful for understanding
what our nosplit chains look like and which ones are close to
running over.
- We move the implementation out of the fog of lib.go to its own file.
- The implementation is generally more Go-like and less C-like.
Change-Id: If1ab31197f5215475559b93695c44a01bd16e276
Reviewed-on: https://go-review.googlesource.com/c/go/+/398176
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The nosplit test was originally written when the stack limit was a
mere 128 bytes. Now it's much larger, but rather than rewriting all of
the tests, we apply a hack to just add the extra space into the stack
frames of the existing tests.
Unfortunately, we add it in the wrong place. The extra space should be
added just once per chain of nosplit functions, but instead we add it
to every frame that appears first on a line in the test's little
script language. This means that for tests like
start 0 call f1
f1 16 nosplit call f2
f2 16 nosplit call f3
f3 16 nosplit call f4
f4 16 nosplit call f5
f5 16 nosplit call f6
f6 16 nosplit call f7
f7 16 nosplit call f8
f8 16 nosplit call end
end 1000
REJECT
we add 672 bytes to *every* frame, meaning that we wind up way over
the stack limit by the end of the stanza, rather than just a little as
originally intended.
Fix this by instead adding the extra space to the first nosplit
function in a stanza. This isn't perfect either, since we could have a
nosplit -> split -> nosplit chain, but it's the best we can do without
a graph analysis.
Change-Id: Ibf156c68fe3eb1b64a438115f4a17f1a6c7e2bd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/398174
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
So that the inliner knows all the other cases are dead and doesn't
accumulate any cost for them.
The canonical case for this is switching on runtime.GOOS, which occurs
several places in the stdlib.
Fixes#50253
Change-Id: I44823aaebb6c1b03c9b0c12d10086db81954350f
Reviewed-on: https://go-review.googlesource.com/c/go/+/399694
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
runtime.getitab need filled fun[0] to identify whether
implemented the interface.
Fixes#51700Fixes#52228
Change-Id: I0173b98f4e1b45e3a0183a5b60229d289140d1e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/399058
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
gofmt is rewriting +build comments into //go:build anyway, so update
the test script to support both.
Change-Id: Ia6d950cfaa2fca9f184b8b2d3625a551bff88dde
Reviewed-on: https://go-review.googlesource.com/c/go/+/399794
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
After CL 398014 fixed a compiler deadlock on syntax errors,
this CL adds a test case and more details for that.
How it was fixed:
CL 57751 introduced a channel "sem" to limit the number of
simultaneously open files.
Unfortunately, when the number of syntax processing goroutines
exceeds this limit, will easily trigger deadlock.
In the original implementation, "sem" only limited the number
of open files, not the number of concurrent goroutines, which
will cause extra goroutines to block on "sem". When the p.err
of the following iteration happens to be held by the blocking
goroutine, it will fall into a circular wait, which is a deadlock.
CL 398014 fixed the above deadlock, also see issue #52127.
First, move "sem <- struct{}{}" to the outside of the syntax
processing goroutine, so that the number of concurrent goroutines
does not exceed the number of open files, to ensure that all
goroutines in execution can eventually write to p.err.
Second, move the entire syntax processing logic into a separate
goroutine to avoid blocking on the producer side.
Change-Id: I1bb89bfee3d2703784f0c0d4ded82baab2ae867a
Reviewed-on: https://go-review.googlesource.com/c/go/+/399054
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Fixes#52278
Change-Id: Ibf67c7b019feec277d316e04d93b458efea133fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/399574
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
In the load tests, we only want to test the assembly produced by
the load operations. If we use the global variable sink, it will produce
one load operation and one store operation(assign to sink).
For example:
func load_be64(b []byte) uint64 {
sink64 = binary.BigEndian.Uint64(b)
}
If we compile this function with GOAMD64=v3, it may produce MOVBEQload
and MOVQstore or MOVQload and MOVBEQstore, but we only want MOVBEQload.
Discovered when developing CL 395474.
Same for the store tests.
Change-Id: I65c3c742f1eff657c3a0d2dd103f51140ae8079e
Reviewed-on: https://go-review.googlesource.com/c/go/+/397875
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
With this change, the shift checking code matches the corresponding
go/types code, but for the differences in the internal error reporting,
and call of check.overflow.
The change leads to the recording of an untyped int value if the RHS
of a non-constant shift is an untyped integer value. Adjust the type
in the compiler's irgen accordingly. Add test/shift3.go to verify
behavior.
Change-Id: I20386fcb1d5c48becffdc2203081fb70c08b282d
Reviewed-on: https://go-review.googlesource.com/c/go/+/398236
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The SHRX/SHLX instruction can take any general register as the shift count operand, and can read source from memory. This CL introduces some operators to combine load and shift to one instruction.
For #47120
Change-Id: I13b48f53c7d30067a72eb2c8382242045dead36a
Reviewed-on: https://go-review.googlesource.com/c/go/+/385174
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
LZCNT is similar to BSR, but BSR(x) is undefined when x == 0, so using
LZCNT can avoid a special case for zero input. Except that case,
LZCNTQ(x) == 63-BSRQ(x) and LZCNTL(x) == 31-BSRL(x).
And according to https://www.agner.org/optimize/instruction_tables.pdf,
LZCNT instructions are much faster than BSR on AMD CPU.
name old time/op new time/op delta
LeadingZeros-8 0.91ns ± 1% 0.80ns ± 7% -11.68% (p=0.000 n=9+9)
LeadingZeros8-8 0.98ns ±15% 0.91ns ± 1% -7.34% (p=0.000 n=9+9)
LeadingZeros16-8 0.94ns ± 3% 0.92ns ± 2% -2.36% (p=0.001 n=10+10)
LeadingZeros32-8 0.89ns ± 1% 0.78ns ± 2% -12.49% (p=0.000 n=10+10)
LeadingZeros64-8 0.92ns ± 1% 0.78ns ± 1% -14.48% (p=0.000 n=10+10)
Change-Id: I125147fe3d6994a4cfe558432780408e9a27557a
Reviewed-on: https://go-review.googlesource.com/c/go/+/396794
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL add MOVBE support for 16-bit version, but MOVBEWload is
excluded because it does not satisfy zero extented.
For #51724
Change-Id: I3fadf20bcbb9b423f6355e6a1e340107e8e621ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/396617
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
The -G compiler option doesn't exist anymore. Update some variable
names and comments to reflect the new reality.
Change-Id: I227e9c59a01615c3a40c3869102e8045cb012980
Reviewed-on: https://go-review.googlesource.com/c/go/+/397254
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For syntax errors in various (syntactic) lists, instead of reporting
a set of "expected" tokens (which may be incomplete), provide context
and mention "possibly missing" tokens. The result is a friendlier and
more accurate error message.
Fixes#49205.
Change-Id: I38ae7bf62febfe790075e62deb33ec8c17d64476
Reviewed-on: https://go-review.googlesource.com/c/go/+/396914
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When parsing method declarations in an interface, the parser has
for historic reasons gracefully handled a list of method names with
a single (common) signature, and then reported an error. For example
interface {
m1, m2, m3 (x int)
}
This code originally came from the very first parser for Go which
initially permitted such declarations (or at least assumed that
people would write such declarations). Nobody is doing this at this
point, so there's no need for being extra careful here. Remove the
respective code and adjust the corresponding test.
Change-Id: If6f9b398bbc9e425dcd4328a80d8bf77c37fe8b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/396654
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 367755 added soleComponent for handling 1-byte type interface conversion.
This implementation must be kept in sync with Type.SoleComponent, but it
does not. When seeing a blank field in struct, we must continue looking
at the field type to find sole component, if any. The current code just
terminate immediately, which causes wrong sole component type returned.
Fixes#52020
Change-Id: I4f506fe094fa7c5532de23467a4f9139476bb0a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/396614
Trust: 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>
Use the 1.17 compiler error message, sans "array" prefix.
Change-Id: I0e70781c5ff02dca30a2004ab4d0ea82b0849eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/396296
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 394074 broke the noopt builder. Something about time.After's inlining
depends on the build flags to make.bash, not the build flags that run.go
passes.
Change-Id: Ib284c66ea2008a4d32829c055d57c54a34ec3fb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/396037
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Historically, we sometimes recorded imports based on either package
path ("net/http") or object file path ("net/http.a"). But modern Go
build systems always use package path, and the extra ".a" suffix
doesn't mean anything anyway.
Change-Id: I6060ef8bafa324168710d152a353f4d8db062133
Reviewed-on: https://go-review.googlesource.com/c/go/+/395254
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>
Add a new rewrite rule to merge ANDconst and UBFX into
UBFX.
Add test cases.
Change-Id: I24d6442d0c956d7ce092c3a3858d4a3a41771670
Reviewed-on: https://go-review.googlesource.com/c/go/+/377054
Trust: Fannie Zhang <Fannie.Zhang@arm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL updates test/run.go to compile xxx.dir/x.go with a package
path of "test/x" instead of just "x". This prevents collisions with
standard library packages.
It also requires updating a handful of tests to account for the
updated package paths.
Fixes#25693.
Change-Id: I49208c56ab3cb229ed667d547cd6e004d2175fcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/395258
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 187617 removed oldescape_linkname.go, but forgot to remove this
directory too.
Change-Id: I6d208c4d96d636b3df93adec1ee22fe1d4f5f61d
Reviewed-on: https://go-review.googlesource.com/c/go/+/395259
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>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
bug302 compiles p.go with -p=p, and then manually creates a pp.a
archive, and imports it as both "p" and "pp". This is a misuse of
cmd/compile's -p flag, and it isn't representative of how any actual
Go build systems work anyway.
This test made sense back when cmd/compile still wrote out bare object
files, which was then split into separate __.PKGDEF and _go_.o archive
entries when added to a pack archive. But since CL 102236, cmd/compile
always writes out pack files.
Updates #51734.
Change-Id: I4b5de22d348ecc0a72c98b512351c2d267c77736
Reviewed-on: https://go-review.googlesource.com/c/go/+/393896
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, run.go's *dir tests allow "x.go" to be imported
interchangeably as either "x" or "./x". This is generally fine, but
can cause problems when "x" is the name of a standard library
package (e.g., "fixedbugs/bug345.dir/io.go").
This CL is an automated rewrite to change all `import "x"` directives
to use `import "./x"` instead. It has no effect today, but will allow
subsequent CLs to update test/run.go to resolve "./x" to "test/x" to
avoid stdlib collisions.
Change-Id: Ic76cd7140e83b47e764f8a499e59936be2b3c876
Reviewed-on: https://go-review.googlesource.com/c/go/+/395116
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The importer type param index used package name type parameter key,
causing type parameters to be reused/overwritten if two packages in the
import graph had the same combination of (name, declaration name, type
parameter name).
Fix this by instead using the *Package in the key.
Fixes#51836
Change-Id: I881ceaf3cf7c1ab4e0835962350feb552e79b233
Reviewed-on: https://go-review.googlesource.com/c/go/+/394219
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
First law of cmd/compile frontend development: thou shalt not rely on
types.Sym.
This CL replaces Type.OrigSym with Type.OrigType, which semantically
matches what all of the uses within the frontend actually care about,
and avoids using types.Sym, which invariably leads to mistakes because
symbol scoping in the frontend doesn't work how anyone intuitively
expects it to.
Fixes#51765.
Change-Id: I4affe6ee0718103ce5006ab68aa7e1bb0cac6881
Reviewed-on: https://go-review.googlesource.com/c/go/+/394274
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
CL 342350 fixed deadcode panic with dead hidden closures. However, a
closure may contains nested dead hidden closures, so we need to mark
them dead as well.
Fixes#51839
Change-Id: Ib54581adfc1bdea60e74d733cd30fd8e783da983
Reviewed-on: https://go-review.googlesource.com/c/go/+/394079
Trust: 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: Cherry Mui <cherryyz@google.com>
They should not share a shape with regular pointers. We could coalesce
multiple pointer-to-not-in-heap types, but doesn't seem worth it - just
make them fully stenciled.
Fixes#51733
Change-Id: Ie8158177226fbc46a798e71c51897a82f15153df
Reviewed-on: https://go-review.googlesource.com/c/go/+/393895
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
CL 338129 added getDictionaryType to get the dictionary type from the
specified dict param, but still using the one in info.dictParam, which
is wrong.
Fixes#51413
Change-Id: Ie13460c1e5751c4c5fc44479a44f6eed8b3b06e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/391994
Trust: 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>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
For hidden closure built during stenciling to implement a function
instantiation, the function may come from other package, not local
package, which causes the ICE for code that re-export the hidden closure
after inlining.
To fix it, use the closure package for export writer when writing out
the closure itself.
Fixes#51423
Change-Id: I23b067ba14e2d602a0fc3b2e99bd9317afbe53ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/391574
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Both the thing we're switching on, as well as the cases we're switching for.
Convert anything containing a type parameter to interface{} before the
comparison happens.
Fixes#51522
Change-Id: I97ba9429ed332cb7d4240cb60f46d42226dcfa5f
Reviewed-on: https://go-review.googlesource.com/c/go/+/391594
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
At this point in stenciling, we have shape types, not raw type parameters.
The code was correct in the other part of this function.
Update #51522
Change-Id: Ife495160a2be5f6af5400363c3efb68dda518b5f
Reviewed-on: https://go-review.googlesource.com/c/go/+/391475
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The -p flag specifies the import path of the package being compiled.
This CL makes it required when invoking the compiler and
adjusts tests that invoke the compiler directly to conform to this
new requirement. The go command already passes the flag, so it
is unmodified in this CL. It is expected that any other Go build systems
also already pass -p, or else they will need to arrange to do so before
updating to Go 1.19. Of particular note, Bazel already does for rules
with an importpath= attribute, which includes all Gazelle-generated rules.
There is more cleanup possible now in cmd/compile, cmd/link,
and other consumers of Go object files, but that is left to future CLs.
Additional historical background follows but can be ignored.
Long ago, before the go command, or modules, or any kind of
versioning, symbols in Go archive files were named using just the
package name, so that for example func F in math/rand and func F in
crypto/rand would both be the object file symbol 'rand.F'. This led to
collisions even in small source trees, which made certain packages
unusable in the presence of other packages and generally was a problem
for Go's goal of scaling to very large source trees.
Fixing this problem required changing from package names to import
paths in symbol names, which was mostly straightforward. One wrinkle,
though, is that the compiler did not know the import path of the
package being compiled; it only knew the package name. At the time,
there was no go command, just Makefiles that people had invoking 6g
(now “go tool compile”) and then copying the resulting object file to
an importable location. That is, everyone had a custom build setup for
Go, because there was no standard one. So it was not particularly
attractive to change how the compiler was invoked, since that would
break approximately every Go user at the time. Instead, we arranged
for the compiler to emit, and other tools reading object files to
recognize, a special import path (the empty string, it turned out)
denoting “the import path of this object file”. This worked well
enough at the time and maintained complete command-line compatibility
with existing Go usage.
The changes implementing this transition can be found by searching
the Git history for “package global name space”, which is what they
eliminated. In particular, CL 190076 (a6736fa4), CL 186263 (758f2bc5),
CL 193080 (1cecac81), CL 194053 (19126320), and CL 194071 (531e6b77)
did the bulk of this transformation in January 2010.
Later, in September 2011, we added the -p flag to the compiler for
diagnostic purposes. The problem was that it was easy to create import
cycles, especially in tests, and these could not be diagnosed until
link time. You'd really want the compiler to diagnose these, for
example if the compilation of package sort noticed it was importing a
package that itself imported "sort". But the compilation of package
sort didn't know its own import path, and so it could not tell whether
it had found itself as a transitive dependency. Adding the -p flag
solved this problem, and its use was optional, since the linker would
still diagnose the import cycle in builds that had not updated to
start passing -p. This was CL 4972057 (1e480cd1).
There was still no go command at this point, but when we introduced
the go command we made it pass -p, which it has for many years at this
point.
Over time, parts of the compiler began to depend on the presence of
the -p flag for various reasonable purposes. For example:
In CL 6497074 (041fc8bf; Oct 2012), the race detector used -p to
detect packages that should not have race annotations, such as
runtime/race and sync/atomic.
In CL 13367052 (7276c02b; Sep 2013), a bug fix used -p to detect the
compilation of package reflect.
In CL 30539 (8aadcc55; Oct 2016), the compiler started using -p to
identify package math, to be able to intrinsify calls to Sqrt inside
that package.
In CL 61019 (9daee931; Sep 2017), CL 71430 (2c1d2e06; Oct 2017), and
later related CLs, the compiler started using the -p value when
creating various DWARF debugging information.
In CL 174657 (cc5eaf93; May 2019), the compiler started writing
symbols without the magic empty string whenever -p was used, to reduce
the amount of work required in the linker.
In CL 179861 (dde7c770; Jun 2019), the compiler made the second
argument to //go:linkname optional when -p is used, because in that
case the compiler can derive an appropriate default.
There are more examples. Today it is impossible to compile the Go
standard library without using -p, and DWARF debug information is
incomplete without using -p.
All known Go build systems pass -p. In particular, the go command
does, which is what nearly all Go developers invoke to build Go code.
And Bazel does, for go_library rules that set the importpath
attribute, which is all rules generated by Gazelle.
Gccgo has an equivalent of -p and has required its use in order to
disambiguate packages with the same name but different import paths
since 2010.
On top of all this, various parts of code generation for generics
are made more complicated by needing to cope with the case where -p
is not specified, even though it's essentially always specified.
In summary, the current state is:
- Use of the -p flag with cmd/compile is required for building
the standard library, and for complete DWARF information,
and to enable certain linker speedups.
- The go command and Bazel, which we expect account for just
about 100% of Go builds, both invoke cmd/compile with -p.
- The code in cmd/compile to support builds without -p is
complex and has become more complex with generics, but it is
almost always dead code and therefore not worth maintaining.
- Gccgo already requires its equivalent of -p in any build
where two packages have the same name.
All this supports the change in this CL, which makes -p required
and adjusts tests that invoke cmd/compile to add -p appropriately.
Future CLs will be able to remove all the code dealing with the
possibility of -p not having been specified.
Change-Id: I6b95b9d4cffe59c7bac82eb273ef6c4a67bb0e43
Reviewed-on: https://go-review.googlesource.com/c/go/+/391014
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a feature that is not understood well enough and may have
subtle repercussions impacting future changes. Disable for Go 1.18.
The actual change is trivial: disable a branch through a flag.
The remaining changes are adjustments to tests.
Fixes#51576.
Change-Id: Ib77b038b846711a808315a8889b3904e72367bce
Reviewed-on: https://go-review.googlesource.com/c/go/+/391135
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>
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>
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>
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>
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>
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>
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>
Also correct scope position for such variables.
Adjusted some comments.
Fixes#51437.
Change-Id: Ic49a1459469c8b2c7bc24fe546795f7d56c67cb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/389594
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This CL is a bit overkill, but it is pretty safe for 1.18. We'll
want to revisit for 1.19 so we can avoid the hash collisions between
types, e.g. G[int] and G[float64], that will cause some slowdowns
(but not incorrect behavior). Thanks Cherry for the simple idea.
Fixes#51250
Change-Id: I68130e09ba68e7cc35687bc623f63547bc552867
Reviewed-on: https://go-review.googlesource.com/c/go/+/389474
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When doing constraint type inference, we must consider whether the
constraint's core type is precise (no tilde) or imprecise (tilde,
or not a single specific type). In the latter case, we cannot infer
an unknown type argument from the (imprecise) core type because there
are infinitely many possible types. For instance, given
[E ~byte]
if we don't know E, we cannot infer that E must be byte (it could be
myByte, etc.). On the other hand, if we do know the type argument,
say for S in this example:
[S ~[]E, E any]
we must consider the underlying type of S when matching against ~[]E
because we have a tilde.
Because constraint type inference may infer type arguments that were
not eligible initially (because they were unknown and the core type
is imprecise), we must iterate the process until nothing changes any-
more. For instance, given
[S ~[]E, M ~map[string]S, E any]
where we initially only know the type argument for M, we must ignore
S (and E) at first. After one iteration of constraint type inference,
S is known at which point we can infer E as well.
The change is large-ish but the actual functional changes are small:
- There's a new method "unknowns" to determine the number of as of yet
unknown type arguments.
- The adjCoreType function has been adjusted to also return tilde
and single-type information. This is now conveniently returned
as (*term, bool), and the function has been renamed to coreTerm.
- The original constraint type inference loop has been adjusted to
consider tilde information.
- This adjusted original constraint type inference loop has been
nested in another loop for iteration, together with some minimal
logic to control termination.
The remaining changes are modifications to tests:
- There's a substantial new test for this issue.
- Several existing test cases were adjusted to accomodate the
fact that they inferred incorrect types: tildes have been
removed throughout. Most of these tests are for pathological
cases.
- A couple of tests were adjusted where there was a difference
between the go/types and types2 version.
Fixes#51229.
Change-Id: If0bf5fb70ec22913b5a2da89adbf8a27fbc921d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/387977
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>
Currently we only include static entries in the hint for sizing
the map when allocating a map for a map literal. Change that to
include all entries.
This will be an overallocation if the dynamic entries in the map have
equal keys, but equal keys in map literals are rare, and at worst we
waste a bit of space.
Fixes#43020
Change-Id: I232f82f15316bdf4ea6d657d25a0b094b77884ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/383634
Run-TryBot: Keith Randall <khr@golang.org>
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We use AutogeneratedPos for most compiler-generated functions. But
for method value wrappers we currently don't. Instead, we use the
Pos for their (direct) declaration if there is one, otherwise
not set it in methodValueWrapper, which will probably cause it to
inherit from the caller, i.e. the Pos of that method value
expression. If that Pos has inline information, it will cause the
method wrapper to have bogus inline information, which could lead
to infinite loop when printing a stack trace.
Change it to use AutogeneratedPos instead.
Fixes#51401.
Change-Id: I398dfe85f9f875e1fd82dc2f489dab63ada6570d
Reviewed-on: https://go-review.googlesource.com/c/go/+/388794
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This test case is failing on the noopt builder, because it disables
inlining. Evidently the explicit -gcflags flag in all of our generics
tests was overriding the noopt builder's default mode.
This CL restores a noop -gcflags to get the builder green again until
the issue can be properly fixed.
Updates #51413.
Change-Id: I61d22a007105f756104ba690b73f1d68ce4be281
Reviewed-on: https://go-review.googlesource.com/c/go/+/388894
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The next CL will remove the -G flag, effectively hard-coding it to its
current default (-G=3).
Change-Id: Ib4743b529206928f9f1cca9fdb19989728327831
Reviewed-on: https://go-review.googlesource.com/c/go/+/388534
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We convert type args to shape types inside instantiations. If an
instantiation constructs a compound type based on that shape type and
uses that as a type arg to another generic function being called, then
we have a type arg with a shape type embedded inside of it. In that
case, we need to substitute out those embedded shape types with their
underlying type.
If we don't do this, we may create extra unneeded shape types that
have these other shape types embedded in them. This may lead to
generating extra shape instantiations, and a mismatch between the
instantiations that we used in generating dictionaries and the
instantations that are actually called.
Updates #51303
Change-Id: Ieef894a5fac176cfd1415f95926086277ad09759
Reviewed-on: https://go-review.googlesource.com/c/go/+/387674
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In getInstantiation, we were not computing tparams correctly for the
case where the receiver of a method was a fully-instantiated type. This
wasn't affecting later parts of the function, since method
instantiations of fully-instantiated types were already being calculated
in an earlier path. But it did give us a non-typeparam when trying to
see if a shape was associated with a type param with a structural type.
The fix is just to get the typeparams associated with the base generic
type. Then we can eliminate a conditional check later in the code.
The tparam parameter of Shapify should always be non-nil
Fixes#51367
Change-Id: I6f95fe603886148b2dad0c581416c51373c85009
Reviewed-on: https://go-review.googlesource.com/c/go/+/388116
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Normally types of constants are emitted when the type is defined (an
ODCLTYPE). However, the types of constants where the type is an
instantiated generic type made inside the constant declaration, do not
normally get emitted. But the DWARF processor in the linker wants
to see those types. So we emit them during stenciling.
Fixes#51245
Change-Id: I59f20f1d7b91501c9ac760cf839a354356331fc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/388117
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The existing test for 51219 didn't actually trigger the types2 issue - I
hadn't been able to minimize the test case yet properly. This new test
case issue51219b.go now does trigger the types2 issue (it's only
slightly different).
Updates #51219
Change-Id: Iaba8144b4702ff4fefec86c899b8acef127b10dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/387814
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The problem in 51355 is that escape analysis decided that the
dictionary variable was captured by reference instead of by value. We
want dictionaries to always be captured by value.
Escape analysis was confused because it saw what it thought was a
reassignment of the dictionary variable. In fact, it was the only
assignment, it just wasn't marked as the defining assignment. Fix
that.
Add an assert to make sure this stays true.
Fixes#51355
Change-Id: Ifd9342455fa107b113f5ff521a94cdbf1b8a7733
Reviewed-on: https://go-review.googlesource.com/c/go/+/388115
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Delay validation of receiver type as it may cause premature expansion
of types the receiver type is dependent on. This was actually a TODO.
While the diff looks large-ish, the actual change is small: all the
receiver validation code has been moved inside the delayed function
body, and a couple of comments have been adjusted.
Fixes#51232.
Fixes#51233.
Change-Id: I44edf0ba615996266791724b832d81b9ccb8b435
Reviewed-on: https://go-review.googlesource.com/c/go/+/387918
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We changed to delaying all transforms of generic functions, since there
are so many complicated situations where type params can be used. We
missed changing so that all Call expressions(not just some) are delayed
if in a generic function. This changes to delaying all transforms on
calls in generic functions. Had to convert Call() to g.callExpr() (so we
can access g.delayTransform()). By always delaying transforms on calls
in generic functions, we actually simplify the code a bit both in
g.CallExpr() and stencil.go.
Fixes#51236
Change-Id: I0342c7995254082c4baf709b0b92a06ec14425e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/386220
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The code for issue #51219 reveals bugs in the types1 and types2
importers that can occur for recursive types that are recursive through
the type constraint.
The crash in the issue is caused by the types1 bug, which leads to the
production of a type1 type which is incomplete and improperly has the
HasTParam flag set. The bug in the types1 importer is that we were not
deferring type instantiations when reading the type parameters, but we
need to do that exactly to correctly handle recursion through the type
constraint. So, the fix is to move the start of the deferrals (in the
'U' section of doDecl in typecheck/iimport.go) above the code that reads
the type params.
Once that bug is fixed, the test still crashes due to a related types2
importer issues. The problem is that t.SetConstraint(c) requires c to be
fully constructed (have its underlying type set). Since that may not be
done yet in the 'U' case in (*importReader).obj() in
importer/iimport.go, we need to defer the call to SetConstraint() in
that case, until we are done importing all the types.
I added a test case with recursion through a type constraint that causes
a problem that is fixed by the types1 importer change, though the error
is not the same as in the issue. I added more types in the test case
(which try to imitate the issue types more closely) the types2 bug, but
wasn't able to cause it yet with the smaller test case.
Fixes#51219
Change-Id: I85d860c98c09dddc37f76ce87a78a6015ec6fd20
Reviewed-on: https://go-review.googlesource.com/c/go/+/386335
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Pointer comparison is lowered to the following on RISCV64
(EqPtr x y) => (SEQZ (SUB <x.Type> x y))
The difference of two pointers (the SUB) should not be pointer
type. Otherwise it can cause the GC to find a bad pointer.
Should fix#51101.
Change-Id: I7e73c2155c36ff403c032981a9aa9cccbfdf0f64
Reviewed-on: https://go-review.googlesource.com/c/go/+/385655
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: Ie949f2131845f9f9292caff798f6933648779122
Reviewed-on: https://go-review.googlesource.com/c/go/+/385434
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
If an invalid array length is just an identifier, mention
"array length" so that it's clear this is an invalid array
declaration and not a (invalid) generic type declaration.
Fixes#51145.
Change-Id: I8878cbb6c7b1277fc0a9a014712ec8d55499c5c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/385255
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>
This is a pure rename of the respective Go functions/methods
with corresponding adjustments to error messages and tests.
A couple of comments were manually rephrased.
With this change, the implementation and error messages match
the latest spec.
No functionality change.
Change-Id: Iaa92a08b64756356fb2c5abdaca5c943c9105c96
Reviewed-on: https://go-review.googlesource.com/c/go/+/384618
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Change run.go to apply the GO_TEST_TIMEOUT_SCALE scaling factor to
test timeouts (mentioned in "-t" clause in test header).
Also with this patch, bump up the timeout for fixedbugs/issue46234.go
from 30 to 45 seconds, to avoid flakes on very slow builders.
Updates #50973.
Change-Id: Icbafa482860e24cc1e72fee53511bcc764d06bf1
Reviewed-on: https://go-review.googlesource.com/c/go/+/382774
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Refactor Checker.comparison such that its logic is easier to reason
about and so that special cases can be handled more directly.
Use the appropriate operand (of 1st or 2nd operand) for error
reporting (position and type), rather than always using the
first operand.
Use an extra parameter to indicate a switch case
comparison; in this case the error is always reported at
the position of the first operand. (The error messages are
not yet adjusted for switches; see next CL.)
Introduce a new kindString function which is used to print simplified
types in error messages (related to comparisons only): instead of
printing the details of a struct type, we just print "struct" where
the details are not relevant. This matches the 1.17 compiler behavior.
Added a "reportf" parameter to the internal comparable function so we
can report an error cause in addition to the boolean result. Rather
than passing a *string for cause, we pass a function to record the
cause so that we can use the *Checker context for printing (needed
for proper type qualification). This mechanism reports the same
details now as the 1.17 compiler.
Adjusted various tests as needed added new test files.
Fixes#50918.
Change-Id: I1f0e7af22f09db4d31679c667c71a9038a8dc9d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/381964
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
It has moved to golang.org/x/exp/constraints. Perhaps it will move
back to the standard library in a future release.
For golang/go#45458Fixesgolang/go#50792
Change-Id: I93aa251a7afe7b329a3d3faadc0c5d6388b1f0e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/382460
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This makes the error case pointed out in the issue like the current
message in Go 1.17 or -G=0 mode. The priority is to point out the
similar but wrong method name, rather than a difference in type.
Made changes to both cmd/compile/internal/types2 and go/types.
Added in a missing tab in an error message in go/types.
At the same time, removed the extra "at info" on the have lines (and
pointer receiver lines) of error messages, as requested in #50907.
Fixes#50816Fixes#50907
Change-Id: I04f8151955bdb6192246cbcb59adc1c4b8a2c4e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381774
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We can type-check these fine but the API implications are unclear.
Fixes#50912.
For #50937.
Change-Id: If29bbb4a257ff6a85e3bfcd4755fd8f90c80fb87
Reviewed-on: https://go-review.googlesource.com/c/go/+/382116
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>
- Use the correct predicate in Checker.implements: for interfaces
we cannot use the API Comparable because it always returns true
for all non-type parameter interface types: Comparable simply
answers if == and != is permitted, and it's always been permitted
for interfaces. Instead we must use Interface.IsComparable which
looks at the type set of an interface.
- When comparing interfaces for identity, we must also consider the
whether the type sets have the comparable bit set.
With this change, `any` doesn't implement `comparable` anymore. This
only matters for generic functions and types, and the API functions.
It does mean that for now (until we allow type-constrained interfaces
for general non-constraint use, at some point in the future) a type
parameter that needs to be comparable cannot be instantiated with an
interface anymore.
For #50646.
Change-Id: I7e7f711bdcf94461f330c90509211ec0c2cf3633
Reviewed-on: https://go-review.googlesource.com/c/go/+/381254
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For a composite literal expression like []T{{f: 1}}, we allow T to be
a pointer to struct type, so it's consistent to allow T to also be a
type parameter whose structural type is a pointer to struct type.
Fixes#50833.
Change-Id: Ib0781ec4a4f327c875ea25b97740ff2c0c86b916
Reviewed-on: https://go-review.googlesource.com/c/go/+/381075
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
These can go wrong when one of the operands is the minimum integer value.
Fixes#50854.
Change-Id: I238fe284f60c7ee5aeb9dc9a18e8b1578cdb77d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/381318
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add a missing shape check in (*Tsubster).tinter when substituting on a
generic type which is an empty interface, analogous to same check in
(*Tsubster).tstruct. Empty structs/interfaces that have rparams (i.e.
are a generic type or a shape type) need to get a new type of their
rparams - they will be different even though they don't have any
fields/methods. Without this shape check, we were not correctly
completing the Token[int] type during substitution in the example in the
issue. This issue only happens for a generic type which is an empty
interface (i.e. doesn't actually use the type param, hence quite unusual).
Added the test case already created by Keith.
Fixes#50841
Change-Id: Ia985b9f52c0e87ed0647b46373e44c51cb748ba4
Reviewed-on: https://go-review.googlesource.com/c/go/+/381175
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This CL updates unified IR to look at the structural type of a
composite literal type, rather than merely the underlying type, to
determine if it's a structure. This fixes a number of currently
failing regress test cases.
Updates #50833.
Change-Id: I11c040c77ec86c23e8ffefcf1ce1aed548687dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381074
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
For #50646.
Change-Id: I7420545556e0df2659836364a62ce2c32ad7a8b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/380654
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We have disallowed having a typeparam on the right-hand-side of a type
declaration. So, we disabled much of the test absdiff.go. I recently
wrote a new test absdiff2.go to use a structure containing the type
param type, so I could attach a method properly and run the full test.
As a contrast, I thought I would create absdiff3.go, where the Abs
functionality is passed in as a function callback (but derived from a
generic function). This is simpler, and more inline with some of the
guidelines that Ian has been proposing (use passed-in functions rather
than requiring methods, when possible, for greater ease-of-use).
Only adds a new test absdiff3.go. (And fixes a comment in absdiff2.go.)
Change-Id: I6dd185b50a3baeec31f689a892319963468a7201
Reviewed-on: https://go-review.googlesource.com/c/go/+/380774
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Added a new absdiff2.go test case, which works fully without using a
typeparam on the right-hand-side of a type declaration (which is
disallowed). Fixed an issue that the test revealed, which is that we
need to set g.curDecl properly for the "later" functions which are
deferred until after all declarations are initially processed. Also,
g.curDecl may be non-nil in typeDecl for local type declaration. So, we
adjust the associate assertion, and save/restore g.curDecl
appropriately.
Fixes#50790
Change-Id: Ieed76a7ad0a83bccb99cbad4bf98a7bfafbcbbd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/380594
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
By processing non-alias type declarations before alias type declaration,
and those before everything else we can avoid some of the remaining
errors which are due to alias types not being available.
For #25838.
For #50259.
For #50276.
For #50729.
Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/380056
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The type checker doesn't have a general mechanism to "use" the type
of a type alias whose type depends on a recursive type declaration
which is not yet completely type-checked. In some cases, the type of
a type alias is needed before it is determined; the type is incorrect
(invalid) in that case but no error is reported. The type-checker is
happy with this (incorrect type), but the compiler may crash under
some circumstances.
A correct fix will likely require some form of forwarding type which
is a fairly pervasive change and may also affect the type checker API.
This CL introduces a simple side table, a map of broken type aliases,
which is consulted before the type associated with a type alias is
used. If the type alias is broken, an error is reported.
This is a stop-gap solution that prevents the compiler from crashing.
The reported error refers to the corresponding issue which suggests
a work-around that may be applicable in some cases.
Also fix a minor error related to type cycles: If we have a cycle
that doesn't start with a type, don't use a compiler error message
that explicitly mentions "type".
Fixes#50259.
Fixes#50276.
Fixes#50779.
For #50729.
Change-Id: Ie8e38f49ef724e742e8e78625e6d4f3d4014a52c
Reviewed-on: https://go-review.googlesource.com/c/go/+/379916
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In validType, when we see an instantiated type, proceed as with
non-generic types but provide an environment in which to look up
the values (the corresponding type arguments) of type parameters
of the instantiated type. For each type parameter for which there
is a type argument, proceed with validating that type argument.
This corresponds to applying validType to the instantiated type
without actually instantiating the type (and running into infinite
instantiations in case of invalid recursive types).
Also, when creating a type instance, use the correct source position
for the instance (the start of the qualified identifier if we have an
imported type).
Fixes#48962.
Change-Id: I196c78bf066e4a56284d53368b2eb71bd8d8a780
Reviewed-on: https://go-review.googlesource.com/c/go/+/379414
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Given we have support for field access to type params with a single
structural type, we need to distinguish between methods calls and field
access when we have an OXDOT node on an expression which is a typeparam
(or correspondingly a shape). We were missing checks in getInstInfo,
which figures out the dictionary format, which then caused problems when
we generate the dictionaries. We don't need/want dictionary entries for
field access, only for bound method calls. Added a new function
isBoundMethod() to distinguish OXDOT nodes which are bound calls vs.
field accesses on a shape.
Removed isShapeDeref() - we can't have field access or method call on a
pointer to variable of type param type.
Fixes#50690
Change-Id: Id692f65e6f427f28cd2cfe474dd30e53c71877a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/379674
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
In a method declaration "func (f *Foo[_, _]) String() string { ... }",
the two blank typeparams have the same name, but our current design with
types1 needs unique names for type params. Similarly, for export/import,
we need unique names to keep the type params straight in generic types
and connect the proper type param with the proper constraint. We make
blank type params unique by changing them to $1, $2, etc in noder.typ0()
via typecheck.TparamExportName(). We then revert $<num> back to _ during
type2 import via typecheck.TparamName(). We similarly revert
during gcimporter import. We don't need/want to revert in the types1
importer, since we want unique names for type params.
Rob Findley has made a similar change to x/tools (and we tried to make
the source code changes similar for the gcimporter and types2 importer
changes).
Fixes#50419
Change-Id: I855cc3d90d06bcf59541ed0c879e9a0e4ede45bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/379194
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Most CONVIFACEs are created in the transform phase (or old typechecker,
in -G=0 mode). But if the main result of a multi-value assignment (map,
channel, or dot-type) must be converted to an interface during the
assignment, that CONVIFACE is not created until (*orderState).as2ok in
the order phase (because the AS2* ops and their sub-ops are so tightly
intertwined). But we need to create the CONVIFACE during the
stenciling/transform phase to enable dictionary lookups. So, in
transformAssign(), if we are doing a special multi-value assignment
involving a type-param-derived type, assign the results first to temps,
so that we can manifest the CONVIFACE during the transform in assigning
the first temp to lhs[0].
Added a test for both AS2RECV (channel receives) and AS2MAPR (maps). I
don't think we can have a type assertion on a type-param-derived type.
Fixes#50642
Change-Id: I4d079fc46c93d8494d7db4ea8234d91522edb02a
Reviewed-on: https://go-review.googlesource.com/c/go/+/379054
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Slightly better for cases such as string(1 << s).
Leaves type-checker tests alone for now because
there are multiple dozens.
For #45117.
Change-Id: I47b314c713fabe424c2158674bf965416a8a6f5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/379274
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For an extension operation like MOWWreg, if the operand is already
extended, we optimize the second extension out. Usually a LoadReg
of a proper type would come already extended, as a MOVW/MOVWU etc.
instruction does. But for a LoadReg to a floating point register,
the instruction does not do the extension. So we cannot elide the
extension.
Fixes#50671.
Change-Id: Id8991df78d5acdecd3fd6138c558428cbd5f6ba3
Reviewed-on: https://go-review.googlesource.com/c/go/+/379236
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Currently the code handles the case of returning values from
a function with no result parameters as a special case.
Consider this input:
package p
func f0_2() { return 1, 2 }
func f0_1() { return 1 }
func f1_0() int { return }
func f1_2() int { return 1, 2 }
func f2_0() (int, int) { return }
func f2_1() (int, int) { return 1 }
The errors are:
x.go:3:33: no result values expected <<<
x.go:4:33: no result values expected <<<
x.go:5:26: not enough return values
have ()
want (int)
x.go:6:36: too many return values
have (number, number)
want (int)
x.go:7:26: not enough return values
have ()
want (int, int)
x.go:8:33: not enough return values
have (number)
want (int, int)
There are two problems with the current special case emitting the
errors on the marked line:
1. It calls them 'result values' instead of 'return values'.
2. It doesn't show the type being returned, which can be useful to programmers.
Using the general case solves both these problems,
so this CL removes the special case and calls the general case instead.
Now those two errors read:
x.go:3:33: too many return values
have (number, number)
want ()
x.go:4:33: too many return values
have (number)
want ()
Fixes#50653.
Change-Id: If6b47dcece14ed4febb3a2d3d78270d5be1cb24d
Reviewed-on: https://go-review.googlesource.com/c/go/+/379116
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
In the compiler, we need to distinguish field and method access on a
type param. For field access, we avoid the dictionary access (to create
an interface bound) and just do the normal transformDot() (which will
create the field access on the shape type).
This field access works fine for non-pointer types, since the shape type
preserves the underlying type of all types in the shape. But we
generally merge all pointer types into a single shape, which means the
field will not be accessible via the shape type. So, we need to change
Shapify() so that a type which is a pointer type is mapped to its
underlying type, rather than being merged with other pointers.
Because we don't want to change the export format at this point in the
release, we need to compute StructuralType() directly in types1, rather
than relying on types2. That implementation is in types/type.go, along
with the helper specificTypes().
I enabled the compiler-related tests in issue50417.go, added an extra
test for unnamed pointer types, and added a bunch more tests for
interesting cases involving StructuralType(). I added a test
issue50417b.go similar to the original example, but also tests access to
an embedded field.
I also added a unit test in
cmd/compile/internal/types/structuraltype_test.go that tests a bunch of
unusual cases directly (some of which have no structural type).
Updates #50417
Change-Id: I77c55cbad98a2b95efbd4a02a026c07dfbb46caa
Reviewed-on: https://go-review.googlesource.com/c/go/+/376194
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When we export a shape instantiation, because a particular
fully-instantiated type is needed by an inlineable function, we possibly
export the body of the instantiation, if it is inlineable. In this case,
we should have been calling ImportedBody() to make sure that the
function body had already been read in (if it is actually imported from
another package).
Fixes#50598
Change-Id: I512d2bcc745faa6ff3a97e25bc8f46e2c2643d23
Reviewed-on: https://go-review.googlesource.com/c/go/+/378494
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
In order to make sure we export the dictionaries/shape methods for all
fully-instantiated types in inlineable functions, we need to descend
fully into types. For example, we may have a map type (e.g.
map[transactionID]Promise[*ByteBuffer]), where the key or value is a new
fully-instantiated type. So, I add a new checkFullyInst() traversal
function, which traverses all encountered types, but maintains a map, so
it only traverse it type once. We need to descend fully into interfaces,
structs, and methods, since a fully-instantiated type make occur in any
fields or arguments/results of methods, etc.
Fixes#50561
Change-Id: I88681a30384168539ed7229eed709f4e73ff0666
Reviewed-on: https://go-review.googlesource.com/c/go/+/378154
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Using type aliases, it's possible to create structs with embedded
fields that have no corresponding type literal notation. However, we
still need to generate a unique name for these types to use for linker
symbols. This CL introduces a new "struct{ Name = Type }" syntax for
use in LinkString formatting to represent these types.
Reattempt at CL 372914, which was rolled back due to race-y
LocalPkg.Lookup call that isn't safe for concurrency.
Fixes#50190.
Change-Id: I0b7fd81e1b0b3199a6afcffde96ade42495ad8d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/378434
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
With this change, we shall now see:
*myS does not implement S (wrong type for DoSomething method)
have DoSomething() (string, error) at ./main.go:9:14
want DoSomething() (int, error)
instead of previously:
*myS does not implement S (wrong type for DoSomething method)
have DoSomething() (string, error)
want DoSomething() (int, error)
Fixes#42841Fixes#45813
Change-Id: I66990929e39b0d36f2e91da0d92f60586a9b84e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/373634
Trust: Robert Findley <rfindley@google.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The names given to methods of types created during type substitution
were possible incorrect when the type parameters themselves were nested
types.
Fixes#50485
Change-Id: I7e0043ed22c26406a5f9d8d51d9e928770a678f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/377494
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The loading of the base type in typ0() may cause s.Def to be defined for
the instantiated type, so load the base type before checking s.Def.
Fixes#50486
Change-Id: Ic039bc8f774dda534f4ccd1f920220b7a10dede6
Reviewed-on: https://go-review.googlesource.com/c/go/+/377094
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Issue #50552 is due to a problem with my recent improvement in the
interaction between generics and inlining. In markInlBody(), we now mark
dictionaries and shape methods for export, so they will be available for
any package that inlines the current inlineable function. But we need to
make sure that the dictionary and method symbols have actually been
resolved into Nodes (looked up in the import data), if they are not
already defined, so we can then mark them for export.
Improved header comment on Resolve().
Fixes#50552
Change-Id: I89e52d39d3b9894591d2ad6eb3a8ed3bb5f1e0a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/377714
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
For some reason, aix sometimes executes the bogus function body. This
should never happen as it lives in a no-execute section. It might be
a transient permission blip as the heap grows.
Add a small function to cleanup and synchronize the icache before
jumping to the bogus function to ensure it causes a panic, not SIGILL.
Fixes#44583
Change-Id: Iadca62d82bfb70fc62088705dac42a880a1208fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/377314
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
I made the default be that, where there are differences between types2
and -G=0 error messages, we want errorcheck tests to pass types2.
Typically, we can get errorcheck to pass on types2 and -G=0 if they give
the same number of error messages on the same lines, just different
wording. If they give a different number of error messages, then I made
types2 pass. I added an exception list for -G=0 to cover those cases
where -G=0 and types give different numbers of error messages.
Because types2 does not run if there are syntax errors, for several
tests, I had to split the tests into two parts in order to get all the
indicated errors to be reported in types2 (bug228.go, bug388.go,
issue11610.go, issue14520.go)
I tried to preserve the GCCGO labeling correctly (but may have gotten
some wrong). When types2 now matches where a GCCGO error previously
occurred, I transformed GCCGO_ERROR -> ERROR. When types2 no longer
reports an error in a certain place, I transformed ERROR -> GCCGO_ERROR.
When types2 reports an error in a new place, I used GC_ERROR.
The remaining entries in types2Failures are things that I think we
probably still need to fix - either actually missing errors in types2,
or cases where types2 gives worse errors than -G=0.
Change-Id: I7f01e82b322b16094096b67d7ed2bb39b410c34f
Reviewed-on: https://go-review.googlesource.com/c/go/+/372854
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- detect *interface case and report specific error
- replaced switch with sequence of if's for more clarity
- fixed isInterfacePtr: it applies to all interfaces, incl.
type parameters
- reviewed/fixed all uses of isInterfacePtr
- adjusted error messages to be consistently of the format
"type %s is pointer to interface, not interface"
Fixes#48312.
Change-Id: Ic3c8cfcf93ad57ecdb60f6a727cce9e1aa4afb5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/376914
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Added a test to make sure that the private methods of a local generic
type are properly exported, if there is a global variable with that
type.
Added comments in crawler.go, to give more detail and to give more about
the overall purpose.
Fixed one place where t.isFullyInstantiated() should be replaced by
isPtrFullyInstantiated(t), so that we catch pointers to generic types
that may be used as a method receiver.
Change-Id: I9c42d14eb6ebe14d249df7c8fa39e889f7cd3f22
Reviewed-on: https://go-review.googlesource.com/c/go/+/374754
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Using type aliases, it's possible to create structs with embedded
fields that have no corresponding type literal notation. However, we
still need to generate a unique name for these types to use for linker
symbols. This CL introduces a new "struct{ Name = Type }" syntax for
use in LinkString formatting to represent these types.
Fixes#50190.
Change-Id: I025ceb09a86e00b7583d3b9885d612f5d6cb44fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/372914
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Finally figured out how to deal with the interaction between generics
and inlining. The problem has been: what to do if you inline a function
that uses a new instantiated type that hasn't been seen in the current
package? This might mean that you need to do another round of
function/method instantiatiations after inlining, which might lead to
more inlining, etc. (which is what we currently do, but it's not clear
when you can stop the inlining/instantiation loop).
We had thought that one solution was to export instantiated types (even
if not marked as exportable) if they are referenced in exported
inlineable functions. But that was quite complex and required changing
the export format. But I realized that we really only need to make sure
the relevant dictionaries and shape instantiations for the instantiated
types are exported, not the instantiated type itself and its wrappers.
The instantiated type is naturally created as needed, and the wrappers
are generated automatically while writing out run-time type (making use
of the exported dictionaries and shape instantiations).
So, we just have to make sure that those dictionaries and shape
instantiations are exported, and then they will be available without any
extra round of instantiations after inlining. We now do this in
crawler.go. This is especially needed when the instantiated type is only
put in an interface, so relevant dictionaries/shape instantiations are
not directly referenced and therefore exported, but are still needed for
the itab.
This fix avoids the phase ordering problem where we might have to keep
creating new type instantiations and instantiated methods after each
round of inlining we do.
Removed the extra round of instantiation/inlining that were added in the
previous fix. The existing tests
test/typeparam{geninline.go,structinit.go} already test this situation
of inlining a function referencing a new instantiated type.
Added the original example from issue 50121 as test (has 5 packages),
since it found a problem with this code that the current simpler test
for 50121 did not find.
Change-Id: Iac5d0dddf4be19376f6de36ee20a83f0d8f213b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/375494
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Similarly to what we do for the built-in function `copy`,
where we allow a string as 2nd argument to append, also
permit a type parameter constrained by string|[]byte.
While at it, change date in the manual.go2 test files so
that we don't need to constantly correct it when copying
a test case from that file into a proper test file.
Fixes#50281.
Change-Id: I23fed66736aa07bb3c481fe97313e828425ac448
Reviewed-on: https://go-review.googlesource.com/c/go/+/376214
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
types2 allows the conversion of a slice of a user-defined byte type B
(not builtin uint8 or byte) to string. But runtime.slicebytetostring
requires a []byte argument, so add in a CONVNOP from []B to []byte if
needed. Same for the conversion of a slice of user-defined rune types to
string.
I made the same change in the transformations of the old typechecker, so
as to keep tcConv() and transformConv() in sync. That fixes the bug for
-G=0 mode as well.
Fixes#23536
Change-Id: Ic79364427f27489187f3f8015bdfbf0769a70d69
Reviewed-on: https://go-review.googlesource.com/c/go/+/376056
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 352870 added extra phase for instantiation after inlining, to take
care of the new fully-instantiated types. However, when fetching inlined
body of these types's methods, we need to allow OADDR operations on
untyped expressions, the same as what main inlining phase does.
The problem does not show up, until CL 371554, which made the compiler
do not re-typecheck while importing, thus leaving a OXDOT node to be
marked as address taken when it's not safe to do that.
Fixes#50437
Change-Id: I20076b872182c520075a4f8b84230f5bcb05b341
Reviewed-on: https://go-review.googlesource.com/c/go/+/375574
Trust: 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: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
For #50317.
Change-Id: I24ccf333c380283a36b573ef8fc3e7fcd71bd17f
Reviewed-on: https://go-review.googlesource.com/c/go/+/376215
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Work-around for #50481: report an error for multiple
blank type parameters. It's always possible to use
non-blank names in those cases.
We expect to lift this restriction for 1.19.
For #50481.
Change-Id: Ifdd2d91340aac1da3387f7d80d46e44f5997c2a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/376058
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Move switch to enable method type parameters entirely
to the parser, by adding the mode AllowMethodTypeParams.
Ensure that the error messages are consistent.
Remove unnecessary code in the type checker.
Fixes#50317.
Change-Id: I4f3958722400bdb919efa4c494b85cf62f4002bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/376054
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>
This change implements field the access p.f where the type of p
is a type parameter with a structural constraint that is a struct
with a field f. This is only the fix for the type checker. The
compiler will need a separate CL.
This makes the behavior consistent with the fact that we can
write struct composite literals for type parameters with a
struct structural type.
For #50417.
For #50233.
Change-Id: I87d07e016f97cbf19c45cde19165eae3ec0bad2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/375795
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For #50439
Change-Id: Ifad6e6f8de42121c695b5a4dc56e0f6606e2917e
Reviewed-on: https://go-review.googlesource.com/c/go/+/375796
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
In the dict info, we need to save the SelectorExpr of a generic method
call when making its sub-dictionary entry. The generic method call will
eventually be transformed into a function call on the method shape
instantiation, so we may not always have the selector info available
when we need it to create a dictionary. We use this SelectorExpr as
needed if the relevant call node has already been transformed.
Similarly, we save the InstExpr of generic function calls, since the
InstExpr will be dropped when the function call is transformed to a call
to a shape instantiation. We use this InstExpr if the relevant function
call has already been transformed.
Added an extra generic function Some2 and a call to it from Some that
exercises the generic function case. The existing test already tests the
method call case.
Fixes#50264
Change-Id: I2c7c7d79a8e33ca36a5e88e64e913c57500c97f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/373754
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Since we use existing instantiations from the symbol table when possible
(to make sure each instantiation is unique), we need to pop
instantiations of local types when leaving the containing scope.
g.stmts() now pushes and pops scope, and we do a Pushdcl() in g.typ0()
when creating an instantiation of a local type.
Non-instantiated local types (generic or not) are translated directly
from types2, so they don't need to be pushed/popped. We don't export
function bodies with local types, so there is no issue during import.
We still don't support local types in generic functions/methods.
Fixes#50177
Change-Id: If2d2fe71aec003d13f0338565c7a0da2c9580a14
Reviewed-on: https://go-review.googlesource.com/c/go/+/372654
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The imported code is already typechecked. NodAddrAt typechecks its
argument, which is unnecessary here and leads to errors when
typechecking unexported field references in other packages' code.
Mark the node is question as already typechecked, so we don't
retypecheck it.
Fixes#50148
Change-Id: I9789e3e7dd4d58ec095675e27b1c98389f7a0c44
Reviewed-on: https://go-review.googlesource.com/c/go/+/371554
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Generic instantiations can produce conversions from constant
literal ints or floats to complex values. We could constant literals
during instantiation, but it is just as easy to upgrade the code
generator to do the conversions.
Fixes#50193
Change-Id: I24bdc09226c8e868f6282e0e4057ba6c3ad5c41a
Reviewed-on: https://go-review.googlesource.com/c/go/+/372514
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Thanks to the simpler test case for issue 50109. I'm keeping the old
test case in place, since it's not too complex, and may be useful for
testing other things as well.
Updates #50109
Change-Id: I80cdbd1da473d0cc4dcbd68e45bab7ddb6f9263e
Reviewed-on: https://go-review.googlesource.com/c/go/+/371515
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
The builtin "any" type should only be identical to an unnamed empty
interface type, not a defined empty interface type.
Fixes#50169.
Change-Id: Ie5bb88868497cb795de1fd0276133ba9812edfe4
Reviewed-on: https://go-review.googlesource.com/c/go/+/372217
Trust: Cherry Mui <cherryyz@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We delay all transformations on generic functions, and only do them on
instantiated functions, for several reasons, of which one is that
otherwise the compiler won't understand the relationship between
constrained type parameters. In an instantiation with shape arguments,
the underlying relationship between the type arguments are clear and
don't lead to compiler errors.
This issue is because I missed delaying assignment transformations for
variable declarations. So, we were trying to transform an assignment,
and the compiler doesn't understand the relationship between the T and U
type parameters.
The fix is to delay assignment transformations for variable declarations
of generic functions, just as we do already for normal assignment
statements.
A work-around for this issue would be to just separate the assignment
from the variable declaration in the generic function (for this case of
an assignment involving both of the constrained type parameters).
Fixes#50147
Change-Id: Icdbcda147e5c4b386e4715811761cbe73d0d837e
Reviewed-on: https://go-review.googlesource.com/c/go/+/371534
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
We can import an shape-instantiated function/method for inlining
purposes. If we are instantiating the methods of a instantiated type
that we have seen, and it happens to need a shape instantiation that we
have imported, then don't re-create the instantiation, since we will end
up with conflicting/duplicate definitions for the instantiation symbol.
Instead, we can just use the existing imported instantation, and enter
it in the instInfoMap[].
Fixes#50121
Change-Id: I6eeb8786faad71106e261e113048b579afad04fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/371414
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
A "RET f(SB)" wasn't assembled correctly in a leaf function with
non-zero frame size. Follows CL 371034, for MIPS(32/64)(be/le)
and S390X. Other architectures seem to do it right. Add a test.
Change-Id: I41349a7ae9862b924f3a3de2bcb55b782061ce21
Reviewed-on: https://go-review.googlesource.com/c/go/+/371214
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
In identical(), we don't want any to match a shape empty-interface type
for the identStrict option, since IdenticalStrict() is specifically not
supposed to match a shape type with a non-shape type.
There is similar code in (*Type).cmp() (TINTER case), but I don't
believe that we want to disqualify shape types from matching any in this
case, since cmp() is used for back-end code, where we don't care about
shape types vs non-shape types.
The issue mainly comes about when 'any' is used as a type argument
(rather than 'interface{}'), but only with some complicated
circumstances, as shown by the test case. (Couldn't reproduce with
simpler test cases.)
Fixes#50109
Change-Id: I3f2f88be158f9ad09273237e1d346bc56aac099f
Reviewed-on: https://go-review.googlesource.com/c/go/+/371154
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Augmented some of the typeswitch*.go tests so that some instantiations
have duplicate cases, in order to ensure we're testing that.
Spacing changes in the tests are due to gofmt.
Change-Id: I5d3678813505c520c544281d4ac8a62ce7e236ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/370155
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Deal with case where a certain instantiation of a generic
function/method leads to an unsatisfiable type assertion or type case.
In that case, the compiler was causing a fatal error while trying to
create an impossible itab for the dictionary. To deal with that case,
allow ITabLsym() to create a dummy itab even when the concrete type
doesn't implement the interface. This dummy itab is analogous to the
"negative" itabs created on-the-fly by the runtime.
We will use the dummy itab in type asserts and type switches in
instantiations that use that dictionary entry. Since the dummy itab can
never be used for any real value at runtime (since the concrete type
doesn't implement the interface), there will always be a failure for the
corresponding type assertion or a non-match for the corresponding
type-switch case.
Fixes#50002
Change-Id: I1df05b1019533e1fc93dd7ab29f331a74fab9202
Reviewed-on: https://go-review.googlesource.com/c/go/+/369894
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The type was changed in https://golang.org/cl/3991043 but the comment
wasn't updated.
Change-Id: I7ba3f625c732e5e801675ffc5d4a28e1d310faa3
Reviewed-on: https://go-review.googlesource.com/c/go/+/369374
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Make sure that we can import/export selects for generics.
Change-Id: Ibf36e98fc574ce9275820aa426b3e6703b0aae6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/369101
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Enable a bunch of types2-related error tests to run successfully, so
they no longer have to be disabled in run.go.
- directive.go: split it into directive.go and directive2.go, since the
possible errors are now split across the parser and noder2, so they
can't all be reported in one file.
- linkname2.go: similarly, split it into linkname2.go and linkname3.go
for the same reason.
- issue16428.go, issue17645.go, issue47201.dir/bo.go: handle slightly
different wording by types2
- issue5609.go: handle slight different error (array length must be
integer vs. array bound too large).
- float_lit3.go: handle slightly different wording (overflows
float vs cannot convert to float)
I purposely didn't try to fix tests yet where there are extra or missing
errors on different lines, since that is not easy to make work for both
-G=3 and -G=0. In a later change, will flip to make the types2 version
match correctly, vs. the -G=0 version.
Change-Id: I6079ff258e3b90146335b9995764e3b1b56cda59
Reviewed-on: https://go-review.googlesource.com/c/go/+/368455
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
First, we need to set base.Pos in varDecl() and typeDecl(), so it will
be correct if we need to report type size errors while converting types.
Changed error calls in types/sizes.go to use Errorf, not ErrorfAt, since
we want to use base.Pos (which will set from t.Pos(), if that is
available).
Second, we need to add an extra call CalcSize(t1.Elem()) in the
TCHANARGS case of CalcSize(). We can use CalcSize() rather than
CheckSize(), since we know the top-level recursive type will have been
calculated by the time we process the fake TCHANARGS type. In -G=0 mode,
the size of the channel element has often been calculated because of
some other processing (but not in the case of #49767). But in -G=3 mode,
we just calculate sizes during the single noder2 pass, so we are more
likely to have not gotten to calculating the size of the element yet,
depending on the order of processing of the deferredTypeStack.
Fixes the tests fixedbugs/issue{42058a,42058b}.go that were
disabled for -G=3 mode.
Had to add exceptions in stdlib_test.go for go/types and types2, because
the types2 typechecker does not know about type size limits.
Fixes#49814Fixes#49771
Updates #49767
Change-Id: I77d058e8ceff68a58c4c386a8cf46799c54b04c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/367955
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
When we set g.curDecl for the type params created during fillinMethods
for an instantiated type, we need to save/restore its value, because
fillinMethods() may be called while processing a typeDecl. We want the
value of g.curDecl to continue to be correct for type params created in
the typeDecl. Because of ordering issues, not restoring g.curDecl
happens to cause problems (which don't always show up visibly) exactly
when a type param is not actually used in a type declaration.
Cleared g.curDecl to "" at the later points in typeDecl() and
funcDecl(). This allows adding asserts that g.curDecl is always empty
("") when we set it in typeDecl() and funcDecl(), and always non-empty
when we use it in typ0().
Fixes#49893
Change-Id: Ic2fb1df791585bd257f2b86ffaae0453c31705c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/368454
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Currently, identical handles any and interface{} by checking against
Types[TINTER]. This is not always true, since when two generated
interface{} types may not use the same *Type instance.
Instead, we must check whether Type is empty interface or not.
Fixes#49875
Change-Id: I28fe4fc0100041a01bb03da795cfe8232b515fc4
Reviewed-on: https://go-review.googlesource.com/c/go/+/367754
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Thanks to emmanuel@orijtech.com who wrote the initial version of
this change (CL 354490).
This change is following CL 354490 in idea but also contains various
simplifications, slightly improved printing of signature/type patterns,
adjustments for types2, and some fine-tuning of error positions.
Also adjusted several ERROR regexp patterns.
Fixes#48834.
Fixes#48835.
Change-Id: I31cf20c81753b1dc84836dbe83a39030ceb9db23
Reviewed-on: https://go-review.googlesource.com/c/go/+/364874
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
This patch revises the fix for issue 46234, fixing a bug that was
accidentally introduced by CL 320913. When inlining a chunk of code
with a closure expression, we want to avoid updating the source
positions in the function being closed over, but we do want to update
the position for the ClosureExpr itself (since it is part of the
function we are inlining). CL 320913 unintentionally did away with the
closure expr source position update; here we restore it again.
Updates #46234.
Fixes#49171.
Change-Id: Iaa51bc498e374b9e5a46fa0acd7db520edbbbfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/366494
Trust: Than McIntosh <thanm@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
When we have a typed nil, we already say so; thus it is sufficient
to use "nil" in all the other cases.
This is closer to (1.17) compiler behavior. In cases where the
1.17 compiler prints "untyped nil" (e.g., wrong uses of "copy"),
we already print a different message. We can do better in those
cases as well; will be addressed in a separate CL (see #49735).
Fixes#48852.
Change-Id: I9a7a72e0f99185b00f80040c5510a693b1ea80f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/366276
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For -G=3 for test using 'any'.
Change-Id: Ia37ee944a38be4f4330e62ad187f10f2d42e41bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/365839
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
In the case where we need to re-export a generic function/method from
another package in the export data of the current package, make sure it
is loaded before trying to write it out.
Fixed#49667
Change-Id: I177754bb762689f34cf5c8ad246d43f1cdbbf195
Reviewed-on: https://go-review.googlesource.com/c/go/+/365837
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
`types.Types[types.TINTER]` is already used for `interface{}`, so we
can conveniently just extend the existing logic that substitutes
`byte` and `rune` with `uint8` and `int32` to also substitute `any`.
Fixes#49665.
Change-Id: I1ab1954699934150aab899b35037d5611c8ca47e
Reviewed-on: https://go-review.googlesource.com/c/go/+/365354
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
It causes a crash because of the unexpected XDOT operation. It's not
needed, since we will run ComputeAddrTaken() on function instantiations
after stenciling. And it's not always correct, since we may not be able
to distinguish between a array and a slice, if a type is dependent on a
type param.
However, we do need to call ComputeAddrTaken on instantiations created
during inlining, since that is after the main ComputeAddrTaken pass.
Fixes#49659
Change-Id: I0bb610cf11f14e4aa9068f6ca2a012337b069c79
Reviewed-on: https://go-review.googlesource.com/c/go/+/365214
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Use actual unmapped memory instead of small integers to make
pointers that will fault when accessed.
Fixes#49562
Change-Id: I2c60c97cf80494dd962a07d10cfeaff6a00f4f8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/364914
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 364377 emitted definition of 'any' when compiling runtime. But 'any'
is only available when generic enabled. Thus emitting its definition
unconditionally causes the compiler crashes.
Updates #49619
Change-Id: I0888ca1cbc7a7df300310a99a344f170636333f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/364614
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Include the predefined type 'any' in the list of other important
predefined types that are emitted when compiling the runtime package
(uintptr, string, etc).
Fixes#49619.
Change-Id: I4a851ba2f302fbc3a425e65daa325c6bf83659da
Reviewed-on: https://go-review.googlesource.com/c/go/+/364377
Trust: Than McIntosh <thanm@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Type constraint satisfaction is interface implementation.
Adjusted a few error messages.
Change-Id: I4266af78e83131a76b1e3e44c847a21de760ac6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/363839
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We hope to support this feature one day, but it doesn't work currently.
Issue a nice error message instead of having the compiler crash.
Update #47631
Change-Id: I0359411410acbaf9a5b9dbb988cd933de1bb8438
Reviewed-on: https://go-review.googlesource.com/c/go/+/364054
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
A recent change to error message formatting was missing a nil check.
Fixes#49592
Change-Id: Ic1843e0277ba75eec0e8e41fe34b59c323d7ea31
Reviewed-on: https://go-review.googlesource.com/c/go/+/364034
Trust: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@google.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
We want package names exposed by reflect to be things like
main.F[main.foo], not main.F["".foo].
Fixes#49547
Change-Id: I182411a75d56ce1f64fde847e5b9ee74ce44e00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/363656
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
This test is failing with -G=0, so specify -G=3.
Change-Id: I4c74707d0a43f8191cb0b156204604458ba85136
Reviewed-on: https://go-review.googlesource.com/c/go/+/363699
Trust: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
When being used by the compiler, augment the types2 missing method
message with extra info, if a method is missing, but a method with the
correct name except for case (i.e. equal via string.EqualFold()) is
present. In that case, print out the wanted method and the method that
is present (that has the wrong case).
In the 1.17 compiler, we don't do this case-folding check when assigning
an interface to an interface, so I didn't add that check, but we could
add that.
Fixes#48471
Change-Id: Ic54549c1f66297c9221d979d49c1daa719aa66cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/363437
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When being used by the compiler, fix up types2 error messages to be more
like Go 1.17 compiler errors. In particular:
- add information about which method is missing when a type is not
assignable/convertible/etc. to an interface.
- add information about any existing method which has the same name,
but wrong type.
- add extra hint in the case that the source or destination type is a
pointer to an interface, rather than an interface.
- add extra hint "need type assertion" in the case that the source is
an interface that is implemented by the destination.
- the following change in the CL stack also adds information about any
existing method with a different name that only differs in case.
Include much of the new logic in a new common function
(*Checker).missingMethodReason().
types2 still adds a little more information in some cases then the Go
1.17 compiler. For example, it typically says "(value of type T)",
rather than "(type T)", where "value" could also be "constant",
"variable", etc.
I kept the types2 error messages almost all the same when types2 is not
used by the compiler. The only change (to reduce amount of compatibility
code) was to change "M method" phrasing in one case to "method M"
phrasing in one error message (which is the phrasing it uses in all
other cases). That is the reason that there are a few small changes in
types2/testdata/check/*.src.
Added new test test/fixedbugs/issue48471.go to test that the added
information is appearing correctly.
Also adjusted the pattern matching in a bunch of other
test/fixedbugs/*.go, now that types2 is producing error messages closer
to Go 1.17. Was able to remove a couple test files from the types2
exception list in run.go.
Updated #48471
Change-Id: I8af1eae6eb8a5541d8ea20b66f494e2e795e1956
Reviewed-on: https://go-review.googlesource.com/c/go/+/363436
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Like OFUNCINST, in case of OXDOT call expression, the arguments need
to be transformed earlier, so any needed CONVIFACE nodes are exposed.
Fixes#49538
Change-Id: I275ddf6f53a9cadc8708e805941cdf7bdffabba9
Reviewed-on: https://go-review.googlesource.com/c/go/+/363554
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Currently the recover4 test, which recovers from a panic created from a
fault, generates a fault by creating a hole in a mapping. It does this
via munmap. However, it's possible the runtime can create a new mapping
that ends up in that hole, for example if the GC executes, causing the
test to fail.
In fact, this is the case now with a smaller minimum heap size.
Modify the test to use mprotect, and clean up the code a little while
we're here: define everything in terms of the length of original
mapping, deduplicate some constants and expressions, and have the test
recover properly even if recover() returns nil (right now it panics
because it fails to type assert nil as error).
Fixes#49381.
Change-Id: If399eca564466e5e8aeb2dc6f86a246d0fce7b5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/363534
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
