Commit Graph

4504 Commits

Author SHA1 Message Date
Matthew Dempsky
d14c02a20c test: remove obsolete test case that misuses -p
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>
2022-03-24 02:15:49 +00:00
Matthew Dempsky
999589e148 test: use dot-relative imports where appropriate
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>
2022-03-24 02:14:15 +00:00
Robert Findley
fd1b5904ae cmd/compile/internal/importer: key tparams by Package instead of pkgname
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>
2022-03-22 21:38:10 +00:00
Matthew Dempsky
adae6ec542 cmd/compile: replace Type.OrigSym with Type.OrigType
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>
2022-03-21 21:57:24 +00:00
Cuong Manh Le
129a2fcf6c cmd/compile: fix panic with nested dead hidden closures
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>
2022-03-21 17:27:41 +00:00
Keith Randall
fcf6afb82d cmd/compile: pointers to notinheap types need their own shape
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>
2022-03-18 19:22:30 +00:00
Cuong Manh Le
8419ec295c cmd/compile: fix wrong dict param when getting dict type
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>
2022-03-14 17:55:47 +00:00
Cuong Manh Le
9743e9b6d8 cmd/compile: fix re-export closure
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>
2022-03-11 00:42:08 +00:00
Keith Randall
2e46a0a997 cmd/compile: fix expression switches using type parameters
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>
2022-03-10 19:30:33 +00:00
Keith Randall
8cf11694ab cmd/compile: fix transform of OEQ/ONE when one arg is a type param
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>
2022-03-10 18:50:50 +00:00
Russ Cox
a987aaf5f7 cmd/compile: require -p flag
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>
2022-03-09 21:31:58 +00:00
Robert Griesemer
b8248fab89 go/types, types2: disable field accesses through type parameters
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>
2022-03-09 21:26:42 +00:00
thepudds
d3070a767b cmd/compile/internal/types2: more consistently print "check go.mod" if language version < 1.18
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>
2022-03-08 16:50:57 +00:00
Robert Findley
28fab5ef21 go/types, types2: disable inference for type instances
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>
2022-03-07 21:18:15 +00:00
Matthew Dempsky
dcb6547b76 cmd/compile: remove duplicate const logic from typecheck
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>
2022-03-07 18:17:40 +00:00
Matthew Dempsky
d1820f748f test: add test case for #51521
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>
2022-03-07 14:07:18 +00:00
Matthew Dempsky
d168c4f296 test: additional generic type switch test coverage
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>
2022-03-07 13:47:58 +00:00
Matthew Dempsky
0e2f1abf5b cmd/compile: represent derived types with ir.DynamicType in unified IR
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>
2022-03-07 13:47:51 +00:00
Matthew Dempsky
7c292ddf1f cmd/compile: fix reentrancy issue in unified IR function body reading
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>
2022-03-07 06:23:18 +00:00
Robert Griesemer
d3fe4e193e go/types, types2: fix scoping for iteration variables declared by range clause
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>
2022-03-03 16:02:44 +00:00
Keith Randall
d3672054fb cmd/compile: don't include instantiating types in type hash
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>
2022-03-03 00:23:02 +00:00
Robert Griesemer
0807986fe6 go/types, types2: correctly consider ~ (tilde) in constraint type inference
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>
2022-03-01 23:48:58 +00:00
Keith Randall
aaa3d39f27 cmd/compile: include all entries in map literal hint size
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>
2022-03-01 23:20:30 +00:00
Cherry Mui
b0db2f00a0 cmd/compile: use AutogeneratedPos for method value wrapper
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>
2022-03-01 21:27:42 +00:00
Matthew Dempsky
f4722d8449 test: workaround codegen bug in typeparam/mdempsky/13.go
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>
2022-03-01 20:52:30 +00:00
Matthew Dempsky
e24977d231 all: avoid use of cmd/compile -G flag in tests
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>
2022-03-01 19:45:34 +00:00
Dan Scales
eb8198d2f6 cmd/compile: deal with constructed types that have shapes in them
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>
2022-02-28 19:38:22 +00:00
Dan Scales
06a43e4ab6 cmd/compile: fix case for structural types where we should be looking at typeparams
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>
2022-02-28 15:58:07 +00:00
Keith Randall
0907d57abf cmd/compile: emit types of constants which are instantiated generic types
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>
2022-02-28 15:50:51 +00:00
Dan Scales
57dda9795d test: add new test case for 51219 that triggers the types2 issue
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>
2022-02-28 14:59:04 +00:00
Keith Randall
a064a4f29a cmd/compile: ensure dictionary assignment statements are defining statements
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>
2022-02-26 01:16:03 +00:00
Robert Griesemer
7c694fbad1 go/types, types2: delay receiver type validation
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>
2022-02-25 15:55:34 +00:00
Dan Scales
4edefe9568 cmd/compile: delay all call transforms if in a generic function
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>
2022-02-24 17:35:40 +00:00
Dan Scales
d35ed09486 cmd/compile: fix importers to deal with recursion through type constraints
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>
2022-02-18 16:56:42 +00:00
Cherry Mui
1ed30ca537 cmd/compile: correct type of pointer difference on RISCV64
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>
2022-02-14 23:08:44 +00:00
Ian Lance Taylor
16b1893600 test: add notinheap test that caused a gofrontend crash
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>
2022-02-13 18:51:07 +00:00
Robert Griesemer
badba359da go/types, types2: better error message for invalid array length
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>
2022-02-11 22:01:05 +00:00
Robert Griesemer
2bf5ae0c28 go/types, types2: rename structuralType/String to coreType/String
This is a pure rename of the respective Go functions/methods
with corresponding adjustments to error messages and tests.
A couple of comments were manually rephrased.

With this change, the implementation and error messages match
the latest spec.

No functionality change.

Change-Id: Iaa92a08b64756356fb2c5abdaca5c943c9105c96
Reviewed-on: https://go-review.googlesource.com/c/go/+/384618
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-02-09 22:58:35 +00:00
Than McIntosh
867a3d5502 test: apply GO_TEST_TIMEOUT_SCALE scaling to test timeouts
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>
2022-02-07 12:32:51 +00:00
Robert Griesemer
e052044d6b go/types, types2: better error messages for comparisons
Refactor Checker.comparison such that its logic is easier to reason
about and so that special cases can be handled more directly.

Use the appropriate operand (of 1st or 2nd operand) for error
reporting (position and type), rather than always using the
first operand.

Use an extra parameter to indicate a switch case
comparison; in this case the error is always reported at
the position of the first operand. (The error messages are
not yet adjusted for switches; see next CL.)

Introduce a new kindString function which is used to print simplified
types in error messages (related to comparisons only): instead of
printing the details of a struct type, we just print "struct" where
the details are not relevant. This matches the 1.17 compiler behavior.

Added a "reportf" parameter to the internal comparable function so we
can report an error cause in addition to the boolean result. Rather
than passing a *string for cause, we pass a function to record the
cause so that we can use the *Checker context for printing (needed
for proper type qualification). This mechanism reports the same
details now as the 1.17 compiler.

Adjusted various tests as needed added new test files.

Fixes #50918.

Change-Id: I1f0e7af22f09db4d31679c667c71a9038a8dc9d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/381964
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-02-04 23:42:25 +00:00
Keith Randall
4e2410617d cmd/compile: restrict generics test to -G=3
Change-Id: Ifdb4f4f4fab8d45847ca525198b3960f87799f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/383034
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
2022-02-03 23:55:05 +00:00
Keith Randall
1ab8273718 cmd/compile: ensure size is computed for shape types
Fixes #50993

Change-Id: I5f1bf5a8375c3da3203083b11de26962523ccb36
Reviewed-on: https://go-review.googlesource.com/c/go/+/382874
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
2022-02-03 18:23:55 +00:00
Ian Lance Taylor
070951c5dc constraints: remove package
It has moved to golang.org/x/exp/constraints. Perhaps it will move
back to the standard library in a future release.

For golang/go#45458
Fixes golang/go#50792

Change-Id: I93aa251a7afe7b329a3d3faadc0c5d6388b1f0e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/382460
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-03 03:13:33 +00:00
Dan Scales
902dc38212 go/types, types2: tweak missingMethodReason logic to improve message
This makes the error case pointed out in the issue like the current
message in Go 1.17 or -G=0 mode. The priority is to point out the
similar but wrong method name, rather than a difference in type.

Made changes to both cmd/compile/internal/types2 and go/types.
Added in a missing tab in an error message in go/types.

At the same time, removed the extra "at info" on the have lines (and
pointer receiver lines) of error messages, as requested in #50907.

Fixes #50816
Fixes #50907

Change-Id: I04f8151955bdb6192246cbcb59adc1c4b8a2c4e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381774
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-02-01 16:52:46 +00:00
Robert Griesemer
4fea5935f5 go/types, types2: disallow real, imag, complex on type parameters
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>
2022-02-01 01:07:25 +00:00
Robert Griesemer
360e1b8197 go/types, types2: fix implements and identical predicates
- Use the correct predicate in Checker.implements: for interfaces
  we cannot use the API Comparable because it always returns true
  for all non-type parameter interface types: Comparable simply
  answers if == and != is permitted, and it's always been permitted
  for interfaces. Instead we must use Interface.IsComparable which
  looks at the type set of an interface.

- When comparing interfaces for identity, we must also consider the
  whether the type sets have the comparable bit set.

With this change, `any` doesn't implement `comparable` anymore. This
only matters for generic functions and types, and the API functions.
It does mean that for now (until we allow type-constrained interfaces
for general non-constraint use, at some point in the future) a type
parameter that needs to be comparable cannot be instantiated with an
interface anymore.

For #50646.

Change-Id: I7e7f711bdcf94461f330c90509211ec0c2cf3633
Reviewed-on: https://go-review.googlesource.com/c/go/+/381254
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2022-01-31 20:35:07 +00:00
Matthew Dempsky
1a2435c95f go/types, cmd/compile: fix composite literal structural typing
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>
2022-01-27 19:55:38 +00:00
David Chase
b7b44b3173 cmd/compile: remove incorrect arm,arm64 CMP->CMN transformations
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>
2022-01-27 19:52:52 +00:00
Dan Scales
a991d9dc27 cmd/compile: add missing shape check in (*Tsubster).tinter
Add a missing shape check in (*Tsubster).tinter when substituting on a
generic type which is an empty interface, analogous to same check in
(*Tsubster).tstruct. Empty structs/interfaces that have rparams (i.e.
are a generic type or a shape type) need to get a new type of their
rparams - they will be different even though they don't have any
fields/methods. Without this shape check, we were not correctly
completing the Token[int] type during substitution in the example in the
issue. This issue only happens for a generic type which is an empty
interface (i.e. doesn't actually use the type param, hence quite unusual).

Added the test case already created by Keith.

Fixes #50841

Change-Id: Ia985b9f52c0e87ed0647b46373e44c51cb748ba4
Reviewed-on: https://go-review.googlesource.com/c/go/+/381175
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2022-01-27 05:30:27 +00:00
Matthew Dempsky
f4aa021985 cmd/compile: support structural typing in unified IR
This CL updates unified IR to look at the structural type of a
composite literal type, rather than merely the underlying type, to
determine if it's a structure. This fixes a number of currently
failing regress test cases.

Updates #50833.

Change-Id: I11c040c77ec86c23e8ffefcf1ce1aed548687dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381074
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
2022-01-27 00:03:31 +00:00