mirror of https://github.com/golang/go.git
1831 Commits
| Author | SHA1 | Message | Date |
|---|---|---|---|
Cuong Manh Le
|
1901e2647f |
test: add test for findTypeLoop with symbols from other packages
CL 274294 improved findTypeLoop but also fixed a new found bug on master branch. This Cl adds test cases for this. Updates #44266 Change-Id: Ie4a07a3487758a1e4ad2f2847dcde975b10d2a77 Reviewed-on: https://go-review.googlesource.com/c/go/+/292889 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
Dan Scales
|
5f3dabbb79 |
cmd/compile: fix import of functions of multiple nested closure
For import of functions with closures, the connections among closure variables are constructed on-the-fly via CaptureName(). For multiple nested closures, we need to temporarily set r.curfn to each closure we construct, so that the processing of closure variables will be correct for any nested closure inside that closure. Fixes #44335 Change-Id: I34f99e2822250542528ff6b2232bf36756140868 Reviewed-on: https://go-review.googlesource.com/c/go/+/294212 Run-TryBot: Dan Scales <danscales@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Dan Scales <danscales@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
zhengjianxun
|
1391d4142c |
fix typo in issue16760.go
fix typo in issue16760.go, unconditinally -> unconditionally
Change-Id: I3a04fbcb23395c562821b35bc2d81cfaec0bc1ed
GitHub-Last-Rev:
|
|
Josh Bleecher Snyder
|
04903476fe |
cmd/compile: reject some rare looping CFGs in shortcircuit
One CFGs that shortcircuit looks for is: p q \ / b / \ t u The test case creates a CFG like that in which p == t. That caused the compiler to generate a (short-lived) invalid phi value. Fix this with a relatively big hammer: Disallow single-length loops entirely. This is probably overkill, but it such loops are very rare. This doesn't change the generated code for anything in std. It generates worse code for the test case: It no longer compiles the entire function away. Fixes #44465 Change-Id: Ib8cdcd6cc9d7f48b4dab253652038ace24eae152 Reviewed-on: https://go-review.googlesource.com/c/go/+/295130 Trust: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> |
|
|
Josh Bleecher Snyder
|
87e984ab29 |
test: add test for issue 38698
It was fixed by CL 294289, for #44378. This is a different style of test that uses line directives instead of extremely long lines. Fixes #38698. Change-Id: I50a1585030978b35fffa9981d6ed96b99216dc3e Reviewed-on: https://go-review.googlesource.com/c/go/+/295129 Trust: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Go Bot <gobot@golang.org> |
|
Than McIntosh
|
e78e04ce39 |
cmd/compile: fix panic in DWARF-gen handling obfuscated code
DWARF generation uses variable source positions (file/line/col) as a way to uniquely identify locals and parameters, as part of the process of matching up post-optimization variables with the corresponding pre-optimization versions (since the DWARF needs to be in terms of the original source constructs). This strategy can run into problems when compiling obfuscated or machine-generated code, where you can in some circumstances wind up with two local variables that appear to have the same name, file, line, and column. This patch changes DWARF generation to skip over such duplicates as opposed to issuing a fatal error (if an obfuscation tool is in use, it is unlikely that a human being will be able to make much sense of DWARF info in any case). Fixes #44378. Change-Id: I198022d184701aa9ec3dce42c005d29b72d2e321 Reviewed-on: https://go-review.googlesource.com/c/go/+/294289 TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Jeremy Faller <jeremy@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Trust: Than McIntosh <thanm@google.com> Run-TryBot: Than McIntosh <thanm@google.com> |
|
Robert Griesemer
|
7cdfa4969a |
[dev.typeparams] all: merge master (06b86e9) into dev.typeparams
Merge List: + 2021-02-19 |
|
|
Dan Scales
|
06b86e9803 |
cmd/compile: fix check to avoid creating new closure function when typechecking inline body
By default, when typechecking a closure, tcClosure() creates a new closure function. This should really be done separate from typechecking. For now, we explicitly avoid creating a new closure function when typechecking an inline body (in ImportedBody). However, the heuristic for determining when we are typechecking an inline body was not correct for double nested closures in an inline body, since CurFunc will then be the inner closure, which has a body. So, use a simple global variable to indicate when we typechecking an inline body. The global variable is fine (just like ir.CurFunc), since the front-end runs serially. Fixes #44325 Change-Id: If2829fe1ebb195a7b1a240192b57fe6f04d1a36b Reviewed-on: https://go-review.googlesource.com/c/go/+/294211 TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Trust: Dan Scales <danscales@google.com> Run-TryBot: Dan Scales <danscales@google.com> |
|
|
Than McIntosh
|
fce2a94d84 |
cmd/compile: fix buglet in inlined info abstract function dwarf-gen
When generating DWARF inlined info records, it's possible to have a local function whose only callsites are inlined away, meaning that we emit an abstract function DIE but no regular subprogram DIE. When emitting DWARF scope info we need to handle this case (specifically when scoping PCs, check for the case that the func in question has been entirely deleted). Fixes #44344. Change-Id: I9f5bc692f225aa4c5c23f7bd2e50bcf7fe4fc5f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/293309 TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Russ Cox <rsc@golang.org> Trust: Than McIntosh <thanm@google.com> Run-TryBot: Than McIntosh <thanm@google.com> |
|
Rob Findley
|
493363ccff |
[dev.regabi] go/types: must not import a package called "init"
This is a port of CL 287494 to go/types. The additional checks in test/fixedbugs are included, though they won't be executed by go/types. Support for errorcheckdir checks will be added to go/types in a later CL. Change-Id: I37e202ea5daf7d7b8fc6ae93a4c4dbd11762480f Reviewed-on: https://go-review.googlesource.com/c/go/+/290570 Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Go Bot <gobot@golang.org> |
|
|
Robert Griesemer
|
721488498a |
[dev.typeparams] cmd/compile: pass -lang flag value to new type checker
This enables another test. Change-Id: I80763b97d939e225158a083299b2e0d189268bc7 Reviewed-on: https://go-review.googlesource.com/c/go/+/289569 Trust: Robert Griesemer <gri@golang.org> Trust: Dan Scales <danscales@google.com> Reviewed-by: Dan Scales <danscales@google.com> |
|
|
Robert Griesemer
|
0d2d6c7464 |
[dev.typeparams] all: merge dev.regabi (23b0c1f) into dev.typeparams
Merge List: + 2021-02-02 |
|
|
Robert Griesemer
|
23b0c1f76e |
[dev.regabi] all: merge master (fca94ab) into dev.regabi
Conflicts: - src/syscall/mksyscall.pl Merge List: + 2021-02-02 |
|
Tom Thorogood
|
32e789f4fb |
test: fix incorrectly laid out instructions in issue11656.go
CL 279423 introduced a regression in this test as it incorrectly laid out various instructions. In the case of arm, the second instruction was overwriting the first. In the case of 386, amd64 and s390x, the instructions were being appended to the end of the slice after 64 zero bytes. This was causing test failures on "linux/s390x on z13". Fixes #44028 Change-Id: Id136212dabdae27db7e91904b0df6a3a9d2f4af4 Reviewed-on: https://go-review.googlesource.com/c/go/+/288278 Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> |
|
|
Robert Griesemer
|
f7d1c5990b |
[dev.typeparams] cmd/compile/internal/types2: must not import a package called "init"
Updates #43962. Change-Id: I070153c55baec62d13ca9284f02781b8c1276844 Reviewed-on: https://go-review.googlesource.com/c/go/+/287494 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> |
|
Matthew Dempsky
|
34704e374f |
[dev.typeparams] all: merge dev.regabi (5e4a0cd) into dev.typeparams
Merge List: + 2021-01-25 |
|
|
Matthew Dempsky
|
c97af0036b |
[dev.typeparams] cmd/compile: force untyped constants from types2 to expected kind
Currently, types2 sometimes produces constant.Values with a Kind
different than the untyped constant type's Is{Integer,Float,Complex}
info, which irgen expects to always match.
While we mull how best to proceed in #43891, this CL adapts irgen to
types2's current behavior. In particular, fixedbugs/issue11945.go now
passes with -G=3.
Updates #43891.
Change-Id: I24823a32ff49af6045a032d3903dbb55cbec6bef
Reviewed-on: https://go-review.googlesource.com/c/go/+/286652
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
|
|
|
Matthew Dempsky
|
5e4a0cdde3 |
[dev.regabi] all: merge master (bf0f7c9) into dev.regabi
This merge involved two merge conflicts: 1. walk's ascompatee code has been substantially refactored on dev.regabi, so CL 285633 is ported to the new style. 2. The os.TestDirFS workaround added in CL 286213 can be removed now that #42637 has been fixed by CL 285720. Conflicts: - src/cmd/compile/internal/gc/walk.go - src/os/os_test.go Merge List: + 2021-01-25 |
|
|
Matthew Dempsky
|
deaf29a8a8 |
cmd/compile: fix order-of-assignment issue w/ defers
CL 261677 fixed a logic issue in walk's alias detection, where it was checking the RHS expression instead of the LHS expression when trying to determine the kind of assignment. However, correcting this exposed a latent issue with assigning to result parameters in functions with defers, where an assignment could become visible earlier than intended if a later expression could panic. Fixes #43835. Change-Id: I061ced125e3896e26d65f45b28c99db2c8a74a8c Reviewed-on: https://go-review.googlesource.com/c/go/+/285633 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Robert Griesemer
|
3663a437a7 |
[dev.typeparams] go/constant: in ToFloat, convert to rational numbers, not floats
Floating-point constants are represented as rational numbers when possible (i.e., when numerators and denominators are not too large). If we convert to floats when not necessary, we risk losing precision. This is the minimal fix for the specific issue, but it's too aggressive: If the numbers are too large, we still want to convert to floats. Will address in a separate CL that also does a few related cleanups. Fixes #43908. Change-Id: Id575e34fa18361a347c43701cfb4dd7221997f66 Reviewed-on: https://go-review.googlesource.com/c/go/+/286552 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> |
|
|
Matthew Dempsky
|
6a4739ccc5 |
[dev.regabi] cmd/compile: enable rational constant arithmetic
This allows more precision and matches types2's behavior. For backwards compatibility with gcimporter, for now we still need to write out declared constants as limited-precision floating-point values. To ensure consistent behavior of constant arithmetic whether it spans package boundaries or not, we include the full-precision rational representation in the compiler's extension section of the export data. Also, this CL simply uses the math/big.Rat.String text representation as the encoding. This is inefficient, but because it's only in the compiler's extension section, we can easily revisit this in the future. Declaring exported untyped float and complex constants isn't very common anyway. Within the standard library, only package math declares any at all, containing just 15. And those 15 are only imported a total of 12 times elsewhere in the standard library. Change-Id: I85ea23ab712e93fd3b68e52d60cbedce9be696a0 Reviewed-on: https://go-review.googlesource.com/c/go/+/286215 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> |
|
|
Matthew Dempsky
|
063c72f06d |
[dev.regabi] cmd/compile: backport changes from dev.typeparams (9456804)
This CL backports a bunch of changes that landed on dev.typeparams, but are not dependent on types2 or generics. By backporting, we reduce the divergence between development branches, hopefully improving test coverage and reducing risk of merge conflicts. Updates #43866. Change-Id: I382510855c9b5fac52b17066e44a00bd07fe86f5 Reviewed-on: https://go-review.googlesource.com/c/go/+/286172 Trust: Matthew Dempsky <mdempsky@google.com> Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> |
|
|
Matthew Dempsky
|
6e46c8fbb5 |
[dev.typeparams] all: merge dev.regabi (7e0a81d) into dev.typeparams
As with CL 285875, this required resolving some conflicts around handling of //go:embed directives. Still further work is needed to reject uses of //go:embed in files that don't import "embed", so this is left as a TODO. (When this code was written for dev.typeparams, we were still leaning towards not requiring the "embed" import.) Also, the recent support for inlining closures (CL 283112) interacts poorly with -G=3 mode. There are some known issues with this code already (#43818), so for now this CL disables inlining of closures when in -G=3 mode with a TODO to revisit this once closure inlining is working fully. Conflicts: - src/cmd/compile/internal/noder/noder.go - src/cmd/compile/internal/typecheck/dcl.go - src/cmd/compile/internal/typecheck/func.go - test/run.go Merge List: + 2021-01-22 |
|
|
Matthew Dempsky
|
7e0a81d280 |
[dev.regabi] all: merge master (dab3e5a) into dev.regabi
This merge had two conflicts to resolve:
1. The embed code on master had somewhat substantially diverged, so
this CL tediously backported the changes to dev.regabi. In particular,
I went through all of the embed changes to gc/{embed,noder,syntax}.go
and made sure the analogous code on dev.regabi in noder/noder.go and
staticdata/embed.go mirrors it.
2. The init-cycle reporting code on master was extended slightly to
track already visited declarations to avoid exponential behavior. The
same fix is applied on dev.regabi, just using ir.NameSet instead of
map[ir.Node]bool.
Conflicts:
- src/cmd/compile/internal/gc/embed.go
- src/cmd/compile/internal/gc/noder.go
- src/cmd/compile/internal/gc/syntax.go
- src/cmd/compile/internal/pkginit/initorder.go
- src/embed/internal/embedtest/embed_test.go
- src/go/types/stdlib_test.go
Merge List:
+ 2021-01-22
|
|
|
Robert Griesemer
|
d8796b5670 |
[dev.typeparams] cmd/compile/internal/types2: report type of nil based on context
With this CL, the type reported for uses of the predeclared identifier nil changes from untyped nil to the type of the context within which nil is used, matching the behaviour of types2 for other untyped types. If an untyped nil value is assigned or converted to an interface, the nil expression is given the interface type. The predicate TypeAndValue.IsNil doesn't change in behavior, it still reports whether the relevant expression is a (typed or untyped) nil value. Change-Id: Id766468f3f3f2a53e4c55e1e6cd521e459c4a94f Reviewed-on: https://go-review.googlesource.com/c/go/+/284218 Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Cuong Manh Le
|
9423d50d53 |
[dev.regabi] cmd/compile: use '%q' for printing rune values less than 128
Fixes #43762 Change-Id: I51734c9b4ee2366a5dae53b2d27b363f4d5fe6c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/284592 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: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
6113db0bb4 |
[dev.regabi] cmd/compile: convert OPANIC argument to interface{} during typecheck
Currently, typecheck leaves arguments to OPANIC as their original
type. This CL changes it to insert implicit OCONVIFACE operations to
convert arguments to `interface{}` like how any other function call
would be handled.
No immediate benefits, other than getting to remove a tiny bit of
special-case logic in order.go's handling of OPANICs. Instead, the
generic code path for handling OCONVIFACE is used, if necessary.
Longer term, this should be marginally helpful for #43753, as it
reduces the number of cases where we need values to be addressable for
runtime calls.
However, this does require adding some hacks to appease existing
tests:
1. We need yet another kludge in inline budgeting, to ensure that
reflect.flag.mustBe stays inlinable for cmd/compile/internal/test's
TestIntendedInlining.
2. Since the OCONVIFACE expressions are now being introduced during
typecheck, they're now visible to escape analysis. So expressions like
"panic(1)" are now seen as "panic(interface{}(1))", and escape
analysis warns that the "interface{}(1)" escapes to the heap. These
have always escaped to heap, just now we're accurately reporting about
it.
(Also, unfortunately fmt.go hides implicit conversions by default in
diagnostics messages, so instead of reporting "interface{}(1) escapes
to heap", it actually reports "1 escapes to heap", which is
confusing. However, this confusing messaging also isn't new.)
Change-Id: Icedf60e1d2e464e219441b8d1233a313770272af
Reviewed-on: https://go-review.googlesource.com/c/go/+/284412
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
|
|
|
Matthew Dempsky
|
ab523fc510 |
[dev.regabi] cmd/compile: don't promote Byval CaptureVars if Addrtaken
We decide during escape analysis whether to pass closure variables by value or reference. One of the factors that's considered is whether a variable has had its address taken. However, this analysis is based only on the user-written source code, whereas order+walk may introduce rewrites that take the address of a variable (e.g., passing a uint16 key by reference to the size-generic map runtime builtins). Typically this would be harmless, albeit suboptimal. But in #43701 it manifested as needing a stack object for a function where we didn't realize we needed one up front when we generate symbols. Probably we should just generate symbols on demand, now that those routines are all concurrent-safe, but this is a first fix. Thanks to Alberto Donizetti for reporting the issue, and Cuong Manh Le for initial investigation. Fixes #43701. Change-Id: I16d87e9150723dcb16de7b43f2a8f3cd807a9437 Reviewed-on: https://go-review.googlesource.com/c/go/+/284075 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
Junchen Li
|
d9b79e53bb |
cmd/compile: fix wrong complement for arm64 floating-point comparisons
Consider the following example,
func test(a, b float64, x uint64) uint64 {
if a < b {
x = 0
}
return x
}
func main() {
fmt.Println(test(1, math.NaN(), 123))
}
The output is 0, but the expectation is 123.
This is because the rewrite rule
(CSEL [cc] (MOVDconst [0]) y flag) => (CSEL0 [arm64Negate(cc)] y flag)
converts
FCMP NaN, 1
CSEL MI, 0, 123, R0 // if 1 < NaN then R0 = 0 else R0 = 123
to
FCMP NaN, 1
CSEL GE, 123, 0, R0 // if 1 >= NaN then R0 = 123 else R0 = 0
But both 1 < NaN and 1 >= NaN are false. So the output is 0, not 123.
The root cause is arm64Negate not handle negation of floating comparison
correctly. According to the ARM manual, the meaning of MI, GE, and PL
are
MI: Less than
GE: Greater than or equal to
PL: Greater than, equal to, or unordered
Because NaN cannot be compared with other numbers, the result of such
comparison is unordered. So when NaN is involved, unlike integer, the
result of !(a < b) is not a >= b, it is a >= b || a is NaN || b is NaN.
This is exactly what PL means. We add NotLessThanF to represent PL. Then
the negation of LessThanF is NotLessThanF rather than GreaterEqualF. The
same reason for the other floating comparison operations.
Fixes #43619
Change-Id: Ia511b0027ad067436bace9fbfd261dbeaae01bcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/283572
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
|
|
|
Matthew Dempsky
|
983ac4b086 |
[dev.regabi] cmd/compile: fix ICE when initializing blank vars
CL 278914 introduced NameOffsetExpr to avoid copying ONAME nodes and hacking up their offsets, but evidently staticinit subtly depended on the prior behavior to allow dynamic initialization of blank variables. This CL refactors the code somewhat to avoid using NameOffsetExpr with blank variables, and to instead create dynamic assignments directly to the global blank node. It also adds a check to NewNameOffsetExpr to guard against misuse like this, since I suspect there could be other cases still lurking within staticinit. (This code is overdue for an makeover anyway.) Thanks to thanm@ for bisect and test case minimization. Fixes #43677. Change-Id: Ic71cb5d6698382feb9548dc3bb9fd606b207a172 Reviewed-on: https://go-review.googlesource.com/c/go/+/283537 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> |
|
|
Matthew Dempsky
|
f065ff221b |
[dev.typeparams] all: merge dev.regabi (d9acf6f) into dev.typeparams
Conflicts: - src/cmd/compile/fmtmap_test.go Merge List: + 2021-01-12 |
|
|
Matthew Dempsky
|
cc90e7a51e |
[dev.regabi] cmd/compile: always use the compile queue
The compiler currently has two modes for compilation: one where it compiles each function as it sees them, and another where it enqueues them all into a work queue. A subsequent CL is going to reorder function compilation to ensure that functions are always compiled before any non-trivial function literals they enclose, and this will be easier if we always use the compile work queue. Also, fewer compilation modes makes things simpler to reason about. Change-Id: Ie090e81f7476c49486296f2b90911fa0a466a5dd Reviewed-on: https://go-review.googlesource.com/c/go/+/283313 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> |
|
|
Matthew Dempsky
|
12ee55ba7b |
[dev.regabi] cmd/compile: stop using Vargen for import/export
Historically, inline function bodies were exported as plain Go source code, and symbol mangling was a convenient hack because it allowed variables to be re-imported with largely the same names as they were originally exported as. However, nowadays we use a binary format that's more easily extended, so we can simply serialize all of a function's declared objects up front, and then refer to them by index later on. This also allows us to easily report unmangled names all the time (e.g., error message from issue7921.go). Fixes #43633. Change-Id: I46c88f5a47cb921f70ab140976ba9ddce38df216 Reviewed-on: https://go-review.googlesource.com/c/go/+/283193 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com> Trust: Dan Scales <danscales@google.com> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
099599662d |
[dev.typeparams] cmd/compile: refactor import logic
This CL refactors noder's package import logic so it's easier to reuse with types2 and gcimports. In particular, this allows the types2 integration to now support vendored packages. Change-Id: I1fd98ad612b4683d2e1ac640839e64de1fa7324b Reviewed-on: https://go-review.googlesource.com/c/go/+/282919 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Robert Griesemer <gri@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
8b2efa990b |
[dev.regabi] cmd/compile: deref PAUTOHEAPs during SSA construction
Currently, during walk we rewrite PAUTOHEAP uses into derefs of their corresponding Heapaddr, but we can easily do this instead during SSA construction. This does involve updating two test cases: * nilptr3.go This file had a test that we emit a "removed nil check" diagnostic for the implicit dereference from accessing a PAUTOHEAP variable. This CL removes this diagnostic, since it's not really useful to end users: from the user's point of view, there's no pointer anyway, so they needn't care about whether we check for nil or not. That's a purely internal detail. And with the PAUTOHEAP dereference handled during SSA construction, we can more robustly ensure this happens, rather than relying on setting a flag in walk and hoping that SSA sees it. * issue20780.go Previously, when PAUTOHEAPs were dereferenced during walk, it had a consequence that when they're passed as a function call argument, they would first get copied to the stack before being copied to their actual destination. Moving the dereferencing to SSA had a side-effect of eliminating this unnecessary temporary, and copying directly to the destination parameter. The test is updated to instead call "g(h(), h())" where h() returns a large value, as the first result will always need to be spilled somewhere will calling the second function. Maybe eventually we're smart enough to realize it can be spilled to the heap, but we don't do that today. Because I'm concerned that the direct copy-to-parameter optimization could interfere with race-detector instrumentation (e.g., maybe the copies were previously necessary to ensure they're not clobbered by inserted raceread calls?), I've also added issue20780b.go to exercise this in a few different ways. Change-Id: I720598cb32b17518bc10a03e555620c0f25fd28d Reviewed-on: https://go-review.googlesource.com/c/go/+/281293 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
Keith Randall
|
304f769ffc |
cmd/compile: don't short-circuit copies whose source is volatile
Current optimization: When we copy a->b and then b->c, we might as well copy a->c instead of b->c (then b might be dead and go away). *Except* if a is a volatile location (might be clobbered by a call). In that case, we really do want to copy a immediately, because there might be a call before we can do the a->c copy. User calls can't happen in between, because the rule matches up the memory states. But calls inserted for memory barriers, particularly runtime.typedmemmove, can. (I guess we could introduce a register-calling-convention version of runtime.typedmemmove, but that seems a bigger change than this one.) Fixes #43570 Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a Reviewed-on: https://go-review.googlesource.com/c/go/+/282492 Trust: Keith Randall <khr@golang.org> Trust: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com> |
|
|
Matthew Dempsky
|
5b9152de57 |
[dev.typeparams] all: merge dev.regabi (cb05a0a) into dev.typeparams
Merge List: + 2021-01-05 |
|
|
Cuong Manh Le
|
fa90aaca7d |
cmd/compile: fix late expand_calls leaf type for OpStructSelect/OpArraySelect
For the example in #43551, before late call expansion, the OpArg type is decomposed to int64. But the late call expansion is currently decompose it to "x.Key" instead. This CL make expand_calls decompose further for struct { 1-field type } and array [1]elem. This matches the previous rules for early decompose args: (StructSelect (StructMake1 x)) => x (ArraySelect (ArrayMake1 x)) => x Fixes #43551 Change-Id: I2f1ebe18cb81cb967f494331c3d237535d2859e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/282332 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: David Chase <drchase@google.com> |
|
|
Matthew Dempsky
|
cb05a0aa6a |
[dev.regabi] cmd/compile: remove toolstash scaffolding
Now that CaptureVars is gone, we can remove the extra code in escape analysis that only served to appease toolstash -cmp. Change-Id: I8c811834f3d966e76702e2d362e3de414c94bea6 Reviewed-on: https://go-review.googlesource.com/c/go/+/281544 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
fd43831f44 |
[dev.regabi] cmd/compile: reimplement capture analysis
Currently we rely on the type-checker to do some basic data-flow analysis to help decide whether function literals should capture variables by value or reference. However, this analysis isn't done by go/types, and escape analysis already has a better framework for doing this more precisely. This CL extends escape analysis to recalculate the same "byval" as CaptureVars and check that it matches. A future CL will remove CaptureVars in favor of escape analysis's calculation. Notably, escape analysis happens after deadcode removes obviously unreachable code, so it sees the AST without any unreachable assignments. (Also without unreachable addrtakens, but ComputeAddrtaken already happens after deadcode too.) There are two test cases where a variable is only reassigned on certain CPUs. This CL changes them to reassign the variables unconditionally (as no-op reassignments that avoid triggering cmd/vet's self-assignment check), at least until we remove CaptureVars. Passes toolstash -cmp. Change-Id: I7162619739fedaf861b478fb8d506f96a6ac21f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/281535 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
fb69c67cad |
[dev.regabi] test: enable finalizer tests on !amd64
The gc implementation has had precise GC for a while now, so we can enable these tests more broadly. Confirmed that they still fail with gccgo 10.2.1. Change-Id: Ic1c0394ab832024a99e34163c422941a3706e1a2 Reviewed-on: https://go-review.googlesource.com/c/go/+/281542 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
a8fe098a12 |
[dev.typeparams] all: merge dev.regabi (77365c5) into dev.typeparams
Conflicts: - src/cmd/compile/internal/gc/main.go - test/fixedbugs/issue15055.go Merge List: + 2021-01-05 |
|
|
Matthew Dempsky
|
d89705e087 |
[dev.regabi] cmd/compile: fix re-export of parameters
When exporting signature types, we include the originating package, because it's exposed via go/types's API. And as a consistency check, we ensure that the parameter names came from that same package. However, we were getting this wrong in the case of exported variables that were initialized with a method value using an imported method. In this case, when we created the method value wrapper function's type (which is reused as the variable's type if none is explicitly provided in the variable declaration), we were reusing the original (i.e., imported) parameter names, but the newly created signature type was associated with the current package instead. The correct fix here is really to preserve the original signature type's package (along with position and name for its parameters), but that's awkward to do at the moment because the DeclFunc API requires an ir representation of the function signature, whereas we only provide a way to explicitly set packages via the type constructor APIs. As an interim fix, we associate the parameters with the current package, to be consistent with the signature type's package. Fixes #43479. Change-Id: Id45a10f8cf64165c9bc7d9598f0a0ee199a5e752 Reviewed-on: https://go-review.googlesource.com/c/go/+/281292 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
290b4154b7 |
[dev.regabi] cmd/compile: fix ICE due to large uint64 constants
It's an error to call Int64Val on constants that don't fit into int64. CL 272654 made the compiler stricter about detecting misuse, and revealed that we were using it improperly in detecting consecutive integer-switch cases. That particular usage actually did work in practice, but it's easy and best to just fix it. Fixes #43480. Change-Id: I56f722d75e83091638ac43b80e45df0b0ad7d48d Reviewed-on: https://go-review.googlesource.com/c/go/+/281272 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
907a4bfdc7 |
[dev.regabi] cmd/compile: fix map assignment order
After the previous cleanup/optimization CLs, ascompatee now correctly handles map assignments too. So remove the code from order.mapAssign, which causes us to assign to the map at the wrong point during execution. It's not every day you get to fix an issue by only removing code. Thanks to Cuong Manh Le for test cases and continually following up on this issue. Passes toolstash -cmp. (Apparently the standard library never uses tricky map assignments. Go figure.) Fixes #23017. Change-Id: Ie0728103d59d884d00c1c050251290a2a46150f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/281172 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
68e6fa4f68 |
[dev.regabi] cmd/compile: fix package-initialization order
This CL fixes package initialization order by creating the init task before the general deadcode-removal pass. It also changes noder to emit zero-initialization assignments (i.e., OAS with nil RHS) for package-block variables, so that initOrder can tell the variables still need initialization. To allow this, we need to also extend the static-init code to recognize zero-initialization assignments. This doesn't pass toolstash -cmp, because it reorders some package initialization routines. Fixes #43444. Change-Id: I0da7996a62c85e15e97ce965298127e075390a7e Reviewed-on: https://go-review.googlesource.com/c/go/+/280976 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
477b049060 |
[dev.regabi] cmd/compile: fix printing of method expressions
OTYPE and OMETHEXPR were missing from OpPrec. So add them with the
same precedences as OT{ARRAY,MAP,STRUCT,etc} and
ODOT{,METH,INTER,etc}, respectively. However, ODEREF (which is also
used for pointer types *T) has a lower precedence than other types, so
pointer types need to be specially handled to assign them their
correct, lower precedence.
Incidentally, this also improves the error messages in issue15055.go,
where we were adding unnecessary parentheses around the types in
conversion expressions.
Thanks to Cuong Manh Le for writing the test cases for #43428.
Fixes #43428.
Change-Id: I57e7979babe3ed9ef8a8b5a2a3745e3737dd785f
Reviewed-on: https://go-review.googlesource.com/c/go/+/280873
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
|
Matthew Dempsky
|
0c1a899a6c |
[dev.regabi] cmd/compile: fix defined-pointer method call check
The compiler has logic to check whether we implicitly dereferenced a defined pointer while trying to select a method. However, rather than checking whether there were any implicit dereferences of a defined pointer, it was finding the innermost dereference/selector expression and checking whether that was dereferencing a named pointer. Moreover, it was only checking defined pointer declared in the package block. This CL restructures the code to match go/types and gccgo's behavior. Fixes #43384. Change-Id: I7bddfe2515776d9480eb2c7286023d4c15423888 Reviewed-on: https://go-review.googlesource.com/c/go/+/280392 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Robert Griesemer <gri@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
a800acaae1 |
[dev.typeparams] all: merge dev.regabi (07569da) into dev.typeparams
Conflicts: - test/fixedbugs/issue27595.go - test/fixedbugs/issue30087.go - test/used.go Merge List: + 2020-12-28 |
|
|
Matthew Dempsky
|
e6c973198d |
[dev.regabi] cmd/compile: stop mangling SelectorExpr.Sel for ODOTMETH
ODOTMETH is unique among SelectorExpr expressions, in that Sel gets mangled so that it no longer has the original identifier that was selected (e.g., just "Foo"), but instead the qualified symbol name for the selected method (e.g., "pkg.Type.Foo"). This is rarely useful, and instead results in a lot of compiler code needing to worry about undoing this change. This CL changes ODOTMETH to leave the original symbol in place. The handful of code locations where the mangled symbol name is actually wanted are updated to use ir.MethodExprName(n).Sym() or (equivalently) ir.MethodExprName(n).Func.Sym() instead. Historically, the compiler backend has mistakenly used types.Syms where it should have used ir.Name/ir.Funcs. And this change in particular may risk breaking something, as the SelectorExpr.Sel will no longer point at a symbol that uniquely identifies the called method. However, I expect CL 280294 (desugar OCALLMETH into OCALLFUNC) to have substantially reduced this risk, as ODOTMETH expressions are now replaced entirely earlier in the compiler. Passes toolstash -cmp. Change-Id: If3c9c3b7df78ea969f135840574cf89e1d263876 Reviewed-on: https://go-review.googlesource.com/c/go/+/280436 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |