aboutsummaryrefslogtreecommitdiff
path: root/test/fixedbugs
AgeCommit message (Collapse)Author
2023-01-25cmd: remove GOEXPERIMENT=nounified knobMatthew Dempsky
This CL removes the GOEXPERIMENT=nounified knob, and any conditional statements that depend on that knob. Further CLs to remove unreachable code follow this one. Updates #57410. Change-Id: I39c147e1a83601c73f8316a001705778fee64a91 Reviewed-on: https://go-review.googlesource.com/c/go/+/458615 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
2023-01-24test: remove TODO in issue20250.goMatthew Dempsky
This has been investigated and explained on the issue tracker. Fixes #54402. Change-Id: I4d8b971faa810591983ad028b7db16411f3b3b4a Reviewed-on: https://go-review.googlesource.com/c/go/+/461456 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
2023-01-19runtime: replace panic(nil) with panic(new(runtime.PanicNilError))Russ Cox
Long ago we decided that panic(nil) was too unlikely to bother making a special case for purposes of recover. Unfortunately, it has turned out not to be a special case. There are many examples of code in the Go ecosystem where an author has written panic(nil) because they want to panic and don't care about the panic value. Using panic(nil) in this case has the unfortunate behavior of making recover behave as though the goroutine isn't panicking. As a result, code like: func f() { defer func() { if err := recover(); err != nil { log.Fatalf("panicked! %v", err) } }() call1() call2() } looks like it guarantees that call2 has been run any time f returns, but that turns out not to be strictly true. If call1 does panic(nil), then f returns "successfully", having recovered the panic, but without calling call2. Instead you have to write something like: func f() { done := false defer func() { if err := recover(); !done { log.Fatalf("panicked! %v", err) } }() call1() call2() done = true } which defeats nearly the whole point of recover. No one does this, with the result that almost all uses of recover are subtly broken. One specific broken use along these lines is in net/http, which recovers from panics in handlers and sends back an HTTP error. Users discovered in the early days of Go that panic(nil) was a convenient way to jump out of a handler up to the serving loop without sending back an HTTP error. This was a bug, not a feature. Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature. Any lingering code that uses panic(nil) to abort an HTTP handler without a failure message should be changed to use http.ErrAbortHandler. Programs that need the old, unintended behavior from net/http or other packages can set GODEBUG=panicnil=1 to stop the run-time error. Uses of recover that want to detect panic(nil) in new programs can check for recover returning a value of type *runtime.PanicNilError. Because the new GODEBUG is used inside the runtime, we can't import internal/godebug, so there is some new machinery to cross-connect those in this CL, to allow a mutable GODEBUG setting. That won't be necessary if we add any other mutable GODEBUG settings in the future. The CL also corrects the handling of defaulted GODEBUG values in the runtime, for #56986. Fixes #25448. Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5 Reviewed-on: https://go-review.googlesource.com/c/go/+/461956 Reviewed-by: Robert Griesemer <gri@google.com> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Russ Cox <rsc@golang.org>
2023-01-18cmd/compile: fix unsafe.{SliceData,StringData} escape analysis memory corruptionCuong Manh Le
Fixes #57823 Change-Id: I54654d3ecb20b75afa9052c5c9db2072a86188d4 Reviewed-on: https://go-review.googlesource.com/c/go/+/461759 Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2023-01-17cmd/compile: fix static init inlining for hidden node fieldsMatthew Dempsky
Unified IR added several new IR fields for holding *runtime._type expressions. To avoid throwing off any frontend semantics (particularly inlining cost heuristics), they were marked as `mknode:"-"` so that code wouldn't visit them. Unfortunately, this has a bad interaction with the static init inlining optimization, because the latter relies on ir.EditChildren to substitute all parameters. This potentially includes dictionary parameters, which can appear within the new RType fields. This CL adds a new ir.EditChildrenWithHidden function that also edits these fields, and switches staticinit to use it. Longer term, we should unhide the RType fields so that ir.EditChildren visits them normally, but that's scarier so late in the release cycle. Fixes #57778. Change-Id: I98c1e8cf366156dc0c81a0cb79029cc5e59c476f Reviewed-on: https://go-review.googlesource.com/c/go/+/461686 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
2023-01-17cmd/compile: ensure temp register mask isn't emptyKeith Randall
We need to avoid nospill registers at this point in regalloc. Make sure that we don't restrict our register set to avoid registers desired by other instructions, if the resulting set includes only nospill registers. Fixes #57846 Change-Id: I05478e4513c484755dc2e8621d73dac868e45a27 Reviewed-on: https://go-review.googlesource.com/c/go/+/461685 Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2023-01-11go/types, types2: don't look up fields or methods when expecting a typeRobert Findley
As we have seen many times, the type checker must be careful to avoid accessing named type information before the type is fully set up. We need a more systematic solution to this problem, but for now avoid one case that causes a crash: checking a selector expression on an incomplete type when a type expression is expected. For golang/go#57522 Change-Id: I7ed31b859cca263276e3a0647d1f1b49670023a9 Reviewed-on: https://go-review.googlesource.com/c/go/+/461577 Run-TryBot: Robert Findley <rfindley@google.com> Auto-Submit: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
2023-01-09all: fix typos in go file commentsMarcel Meyer
These typos were found by executing grep, aspell, sort, and uniq in a pipe and searching the resulting list manually for possible typos. grep -r --include '*.go' -E '^// .*$' . | aspell list | sort | uniq Change-Id: I56281eda3b178968fbf104de1f71316c1feac64f GitHub-Last-Rev: e91c7cee340fadfa32b0c1773e4e5cd1ca567638 GitHub-Pull-Request: golang/go#57669 Reviewed-on: https://go-review.googlesource.com/c/go/+/460767 Run-TryBot: Ian Lance Taylor <iant@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-12-14cmd/compile: desugar OCALLMETH->OCALLFUNC within devirtualizationMatthew Dempsky
Devirtualization can turn OCALLINTER into OCALLMETH, but then we want to actually desugar into OCALLFUNC instead for later phases. Just needs a missing call to typecheck.FixMethodCall. Fixes #57309. Change-Id: I331fbd40804e1a370134ef17fa6dd501c0920ed3 Reviewed-on: https://go-review.googlesource.com/c/go/+/457715 Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-12-09cmd/compile: fix conditional select ruleKeith Randall
ARM64 maintains booleans in the low byte of registers. Upper parts of that register are junk. This rule is using all 32 bits of a boolean-containing register, which is wrong. Change the rule to only look at the low bit. Fixes #57184 Change-Id: Ibbef86b2be859df3d06d993db00e1231c481c428 Reviewed-on: https://go-review.googlesource.com/c/go/+/456556 Auto-Submit: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: Keith Randall <khr@golang.org>
2022-11-30cmd/compile: fix inline static init with derived typesCuong Manh Le
CL 450136 added handling for simple calls in staticinit. If there's any derived types conversion in the body of generic function called, that conversion will require runtime dictionary, thus the optimization could not happen. Fixes #56923 Change-Id: I498cee9f8ab4397812ef79a6c2ab6c55e0ee4aef Reviewed-on: https://go-review.googlesource.com/c/go/+/453315 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> Reviewed-by: Gabriel Morency (Amgc63spaming) <morencyvincent8@gmail.com>
2022-11-30cmd/compile: disallow CMOV optimization with ptr arithmetic as an argKeith Randall
if q != nil { p = &q.f } Which gets rewritten to a conditional move: tmp := &q.f p = Select q!=nil, tmp, p Unfortunately, we can't compute &q.f before we've checked if q is nil, because if it is nil, &q.f is an invalid pointer (if f's offset is nonzero but small). Normally this is not a problem because the tmp variable above immediately dies, and is thus not live across any safepoint. However, if later there is another &q.f computation, those two computations are CSEd, causing tmp to be used at both use points. That will extend tmp's lifetime, possibly across a call. Fixes #56990 Change-Id: I3ea31be93feae04fbe3304cb11323194c5df3879 Reviewed-on: https://go-review.googlesource.com/c/go/+/454155 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
2022-11-23cmd/compile: reenable inlstaticinitRuss Cox
This was disabled in CL 452676 out of an abundance of caution, but further analysis has shown that the failures were not being caused by this optimization. Instead the sequence of commits was: CL 450136 cmd/compile: handle simple inlined calls in staticinit ... CL 449937 archive/tar, archive/zip: return ErrInsecurePath for unsafe paths ... CL 451555 cmd/compile: fix static init for inlined calls The failures in question became compile failures in the first CL and started building again after the last CL. But in the interim the code had been broken by the middle CL. CL 451555 was just the first time that the tests could run and fail. For #30820. Change-Id: I65064032355b56fdb43d9731be2f9f32ef6ee600 Reviewed-on: https://go-review.googlesource.com/c/go/+/452817 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Russ Cox <rsc@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-22cmd/compile: add -d=inlstaticinit debug flagMatthew Dempsky
This CL adds -d=inlstaticinit to control whether static initialization of inlined function calls (added in CL 450136) is allowed. We've needed to fix it once already (CL 451555) and Google-internal testing is hitting additional failure cases, so putting this optimization behind a feature flag seems appropriate regardless. Also, while we diagnose and fix the remaining cases, this CL also disables the optimization to avoid miscompilations. Updates #56894. Change-Id: If52a358ad1e9d6aad1c74fac5a81ff9cfa5a3793 Reviewed-on: https://go-review.googlesource.com/c/go/+/452676 Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com>
2022-11-21cmd/compile: reject anonymous interface cyclesMatthew Dempsky
This CL changes cmd/compile to reject anonymous interface cycles like: type I interface { m() interface { I } } We don't anticipate any users to be affected by this change in practice. Nonetheless, this CL also adds a `-d=interfacecycles` compiler flag to suppress the error. And assuming no issue reports from users, we'll move the check into go/types and types2 instead. Updates #56103. Change-Id: I1f1dce2d7aa19fb388312cc020e99cc354afddcb Reviewed-on: https://go-review.googlesource.com/c/go/+/445598 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Matthew Dempsky <mdempsky@google.com>
2022-11-18cmd/compile: fix wrong optimization for eliding Not in PhiWayne Zuo
The previous rule may move the phi value into a wrong block. This CL make it only rewrite the phi value not the If block, so that the phi value will stay in old block. Fixes #56777 Change-Id: I9479a5c7f28529786968413d35b82a16181bb1f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/451496 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
2022-11-17cmd/compile: fix broken IR for iface -> efaceCuong Manh Le
For implementing interface to empty interface conversion, the compiler generate code like: var res *uint8 res = itab if res != nil { res = res.type } However, itab has type *uintptr, so the assignment is broken. The problem is not shown up, until CL 450215, which call typecheck on this broken assignment. To fix this, just cast itab to *uint8 when doing the conversion. Fixes #56768 Change-Id: Id42792d18e7f382578b40854d46eecd49673792c Reviewed-on: https://go-review.googlesource.com/c/go/+/451256 Reviewed-by: Keith Randall <khr@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2022-11-17cmd/compile: fix static init for inlined callsCuong Manh Le
CL 450136 made the compiler to be able to handle simple inlined calls in staticinit. However, it's missed a condition when checking substituting arg for param. If there's any non-trivial closures, it has captured one of the param, so the substitution could not happen. Fixes #56778 Change-Id: I427c9134e333e2f9af136c1a124da4d37d326f10 Reviewed-on: https://go-review.googlesource.com/c/go/+/451555 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
2022-11-15cmd/compile: fix missing typecheck for static initialization sliceCuong Manh Le
CL 440455 fixed missing walk pass for static initialization slice. However, slicelit may produce un-typechecked node, thus we need to do typecheck for sinit before calling walkStmtList. Fixes #56727 Change-Id: I40730cebcd09f2be4389d71c5a90eb9a060e4ab7 Reviewed-on: https://go-review.googlesource.com/c/go/+/450215 Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2022-11-11test: add regression test for issue 53439Cuong Manh Le
Fixes #53439 Change-Id: I425af0f78153511034e4a4648f32ef8c9378a325 Reviewed-on: https://go-review.googlesource.com/c/go/+/449756 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
2022-11-08cmd/compile: fix transitive inlining of generic functionsMatthew Dempsky
If an imported, non-generic function F transitively calls a generic function G[T], we may need to call CanInline on G[T]. While here, we can also take advantage of the fact that we know G[T] was already seen and compiled in an imported package, so we don't need to call InlineCalls or add it to typecheck.Target.Decls. This saves us from wasting compile time re-creating DUPOK symbols that we know already exist in the imported package's link objects. Fixes #56280. Change-Id: I3336786bee01616ee9f2b18908738e4ca41c8102 Reviewed-on: https://go-review.googlesource.com/c/go/+/443535 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Matthew Dempsky <mdempsky@google.com>
2022-11-03cmd/compile: allow ineffectual //go:linkname in -lang=go1.17 and olderMatthew Dempsky
Prior to Go 1.18, ineffectual //go:linkname directives (i.e., directives referring to an undeclared name, or to a declared type or constant) were treated as noops. In Go 1.18, we changed this into a compiler error to mitigate accidental misuse. However, the x/sys repo contained ineffectual //go:linkname directives up until go.dev/cl/274573, which has caused a lot of user confusion. It seems a bit late to worry about now, but to at least prevent further user pain, this CL changes the error message to only apply to modules using "go 1.18" or newer. (The x/sys repo declared "go 1.12" at the time go.dev/cl/274573 was submitted.) Fixes #55889. Change-Id: Id762fff96fd13ba0f1e696929a9e276dfcba2620 Reviewed-on: https://go-review.googlesource.com/c/go/+/447755 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
2022-10-20cmd/compile: in compiler errors, print more digits for floats close to an intKeith Randall
Error messages currently print floats with %.6g, which means that if you tried to convert something close to, but not quite, an integer, to an integer, the error you get looks like "cannot convert 1 to type int", when really you want "cannot convert 0.9999999 to type int". Add more digits to floats when printing them, to make it clear that they aren't quite integers. This helps for errors which are the result of not being an integer. For other errors, it won't hurt much. Fixes #56220 Change-Id: I7f5873af5993114a61460ef399d15316925a15a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/442935 Reviewed-by: Rob Pike <r@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Keith Randall <khr@golang.org>
2022-10-12test: update test/run.go and some tests to use importcfgMichael Matloob
Using importcfg instead of depending on the existence of .a files for standard library packages will enable us to remove the .a files in a future cl. Change-Id: I6108384224508bc37d82fd990fc4a8649222502c Reviewed-on: https://go-review.googlesource.com/c/go/+/440222 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-10-12all: prevent fakePC overflow on 386 in libfuzzer modeCuong Manh Le
fakePC uses hash.Sum32, which returns an uint32. However, libfuzzer trace/hook functions declare fakePC argument as int, causing overflow on 386 archs. Fixing this by changing fakePC argument to uint to prevent the overflow. Fixes #56141 Change-Id: I3994c461319983ab70065f90bf61539a363e0a2a Reviewed-on: https://go-review.googlesource.com/c/go/+/441996 Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2022-10-10test: add test case that caused a bogus error from gofrontendIan Lance Taylor
For #56109 Change-Id: I999763e463fac57732a92f5e396f8fa8c35bd2e1 Reviewed-on: https://go-review.googlesource.com/c/go/+/440297 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org>
2022-10-10cmd/compile: fix missing walk pass for static initialization sliceCuong Manh Le
CL 403995 fixed static init of literal contains dynamic exprs, by ensuring their init are ordered properly. However, we still need to walk the generated init codes before appending to parent init. Otherwise, codes that requires desugaring will be unhandled, causing the compiler backend crashing. Fixes #56105 Change-Id: Ic25fd4017473f5412c8e960a91467797a234edfd Reviewed-on: https://go-review.googlesource.com/c/go/+/440455 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org>
2022-09-30cmd/compile: eagerly create LSym for closuresMichael Pratt
The linker needs FuncInfo metadata for all inlined functions. This is typically handled by gc.enqueueFunc calling ir.InitLSym for all function declarations in typecheck.Target.Decls (ir.UseClosure adds all closures to Decls). However, non-trivial closures in Decls are ignored, and are insteaded enqueued when walk of the calling function discovers them. This presents a problem for direct calls to closures. Inlining will replace the entire closure definition with its body, which hides the closure from walk and thus suppresses symbol creation. Explicitly create a symbol early in this edge case to ensure we keep this metadata. InitLSym needs to move out of ssagen to avoid a circular dependency (it doesn't have anything to do with ssa anyway). There isn't a great place for it, so I placed it in ir, which seemed least objectionable. The added test triggers one of these inlined direct non-trivial closure calls, though the test needs CL 429637 to fail, which adds a FuncInfo assertion to the linker. Note that the test must use "run" instead of "compile" since the assertion is in the linker, and "compiler" doesn't run the linker. Fixes #54959. Change-Id: I0bd1db4f3539a78da260934cd968372b7aa92546 Reviewed-on: https://go-review.googlesource.com/c/go/+/436240 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-29go/types, types2: more concise error messages for cycle errorsRobert Griesemer
If a cycle has length 1, don't enumerate the single cycle entry; instead just mention "refers to itself". For instance, for an invalid recursive type T we now report: invalid recursive type: T refers to itself instead of: invalid recursive type T T refers to T Adjust tests to check for the different error messages. Change-Id: I5bd46f62fac0cf167f0d0c9a55f952981d294ff4 Reviewed-on: https://go-review.googlesource.com/c/go/+/436295 Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Robert Griesemer <gri@google.com>
2022-09-28cmd/compile: use "shifted operand %s (type %s) must be integer" for some ↵Robert Griesemer
shift errors This matches what go/types and types2 report and it also matches the compiler errors reported for some related shift problems. For #55326. Change-Id: Iee40e8d988d5a7f9ff2c49f019884d02485c9fdf Reviewed-on: https://go-review.googlesource.com/c/go/+/436177 Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-28cmd/compile: use "cannot use %s as %s value in %s: %s" error messageRobert Griesemer
This is close to what the compiler used to say, except now we say "as T value" rather than "as type T" which is closer to the truth (we cannot use a value as a type, after all). Also, place the primary error and the explanation (cause) on a single line. Make respective (single line) adjustment to the matching "cannot convert" error. Adjust various tests. For #55326. Change-Id: Ib646cf906b11f4129b7ed0c38cf16471f9266b88 Reviewed-on: https://go-review.googlesource.com/c/go/+/436176 Reviewed-by: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-27cmd/compile: use "method T.m already declared" for method redeclaration errorsRobert Griesemer
Compromise between old compiler error "T.m redeclared in this block" (where the "in this block" is not particularly helpful) and the old type-checker error "method m already declared for type T ...". In the case where we have position information for the original declaration, the error message is "method T.m already declared at <position>". The new message is both shorter and more precise. For #55326. Change-Id: Id4a7f326fe631b11db9e8030eccb417c72d6c7db Reviewed-on: https://go-review.googlesource.com/c/go/+/435016 Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
2022-09-27go/types, types2: use "unknown field f in struct literal of type S" in error ↵Robert Griesemer
messages This is a compromise of the error reported by the compiler (quotes around field name removed) and the error reported by the type checkers (added mention of struct type). For #55326. Change-Id: Iac4fb5c717f17c6713e90d327d39e68d3be40074 Reviewed-on: https://go-review.googlesource.com/c/go/+/434815 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
2022-09-27go/types, types2: use "and not used" instead of "but not used" in error messagesRobert Griesemer
This matches longstanding compiler behavior. Also, for unused packages, report: `"pkg" imported and not used` `"pkg" imported as X and not used` This matches the other `X declared and not used` errors. For #55326. Change-Id: Ie71cf662fb5f4648449c64fc51bede298a1bdcbf Reviewed-on: https://go-review.googlesource.com/c/go/+/432557 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com>
2022-09-24cmd/compile: use "missing method m" instead of "missing m method"cuiweixie
For #55326 Change-Id: I3d0ff7f820f7b2009d1b226abf701b2337fe8cbc Reviewed-on: https://go-review.googlesource.com/c/go/+/432635 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Run-TryBot: xie cui <523516579@qq.com> Reviewed-by: Robert Griesemer <gri@google.com>
2022-09-23cmd/compile: use "init... cycle" instead of "init... loop" in error messagesRobert Griesemer
For #55326. Change-Id: Ia3c1124305986dcd49ac769e700055b263cfbd59 Reviewed-on: https://go-review.googlesource.com/c/go/+/432615 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
2022-09-19cmd/compile: avoid using destination pointer base type in memmove optimizationKeith Randall
The type of the source and destination of a memmove call isn't always accurate. It will always be a pointer (or an unsafe.Pointer), but the base type might not be accurate. This comes about because multiple copies of a pointer with different base types are coalesced into a single value. In the failing example, the IData selector of the input argument is a *[32]byte in one branch of the type switch, and a *[]byte in the other branch. During the expand_calls pass both IDatas become just copies of the input register. Those copies are deduped and an arbitrary one wins (in this case, *[]byte is the unfortunate winner). Generally an op v can rely on v.Type during rewrite rules. But relying on v.Args[i].Type is discouraged. Fixes #55122 Change-Id: I348fd9accf2058a87cd191eec01d39cda612f120 Reviewed-on: https://go-review.googlesource.com/c/go/+/431496 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
2022-09-19cmd/compile/internal/typebits: relax alignment checkCuong Manh Le
Now we have 8-byte alignment types on 32-bit system, so in some rare case, e.g, generated wrapper for embedded interface, the function argument may need more than 4 byte alignment. We could pad somehow, but this is a rare case which makes it hard to ensure that we've got it right. So relaxing the check for argument and return value region of the stack. Fixes #54991 Change-Id: I34986e17a920254392a39439ad3dcb323da2ea8d Reviewed-on: https://go-review.googlesource.com/c/go/+/431098 Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
2022-09-07test: add failing test case for inlined type switchesMatthew Dempsky
The unified frontend ICEs when inlining a function that contains a function literal, which captures both a type switch case variable and another variable. Updates #54912. Change-Id: I0e16d371ed5df48a70823beb0bf12110a5a17266 Reviewed-on: https://go-review.googlesource.com/c/go/+/428917 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2022-09-07test: add regression test for issue 54911Cuong Manh Le
It was fixed by CL 422196, and have been already worked in unified IR. Fixes #54911 Change-Id: Ie69044a64b296f6961e667e7661d8c4d1a24d84e Reviewed-on: https://go-review.googlesource.com/c/go/+/428758 Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
2022-09-06cmd/compile: do not devirtualize defer/go callsCuong Manh Le
For defer/go calls, the function/method value are evaluated immediately. So after devirtualizing, it may trigger a panic when implicitly deref a nil pointer receiver, causing the program behaves unexpectedly. It's safer to not devirtualizing defer/go calls at all. Fixes #52072 Change-Id: I562c2860e3e577b36387dc0a12ae5077bc0766bf Reviewed-on: https://go-review.googlesource.com/c/go/+/428495 Reviewed-by: Michael Knyszek <mknyszek@google.com> 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>
2022-09-02runtime: drop function context from tracebackAustin Clements
Currently, gentraceback tracks the closure context of the outermost frame. This used to be important for "unstarted" calls to reflect function stubs, where "unstarted" calls are either deferred functions or the entry-point of a goroutine that hasn't run. Because reflect function stubs have a dynamic argument map, we have to reach into their closure context to fetch to map, and how to do this differs depending on whether the function has started. This was discovered in issue #25897. However, as part of the register ABI, "go" and "defer" were made much simpler, and any "go" or "defer" of a function that takes arguments or returns results gets wrapped in a closure that provides those arguments (and/or discards the results). Hence, we'll see that closure instead of a direct call to a reflect stub, and can get its static argument map without any trouble. The one case where we may still see an unstarted reflect stub is if the function takes no arguments and has no results, in which case the compiler can optimize away the wrapper closure. But in this case we know the argument map is empty: the compiler can apply this optimization precisely because the target function has no argument frame. As a result, we no longer need to track the closure context during traceback, so this CL drops all of that mechanism. We still have to be careful about the unstarted case because we can't reach into the function's locals frame to pull out its context (because it has no locals frame). We double-check that in this case we're at the function entry. I would prefer to do this with some in-code PCDATA annotations of where to find the dynamic argument map, but that's a lot of mechanism to introduce for just this. It might make sense to consider this along with #53609. Finally, we beef up the test for this so it more reliably forces the runtime down this path. It's fundamentally probabilistic, but this tweak makes it better. Scheduler testing hooks (#54475) would make it possible to write a reliable test for this. For #54466, but it's a nice clean-up all on its own. Change-Id: I16e4f2364ba2ea4b1fec1e27f971b06756e7b09f Reviewed-on: https://go-review.googlesource.com/c/go/+/424254 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
2022-09-02go/types,types2: move notinheap tests to fixedbugs directoryCuong Manh Le
So they can be added to ignored list, since the tests now require cgo.Incomplete, which is not recognized by go/types and types2. Updates #46731 Change-Id: I9f24e3c8605424d1f5f42ae4409437198f4c1326 Reviewed-on: https://go-review.googlesource.com/c/go/+/427142 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
2022-09-02cmd/link: only add dummy XCOFF reference if the symbol existsCherry Mui
On AIX when external linking, for some symbols we need to add dummy references to prevent the external linker from discarding them. Currently we add the reference unconditionally. But if the symbol doesn't exist, the linking fails in a later stage for generating external relocation of a nonexistent symbol. The symbols are special symbols that almost always exist, except that go:buildid may not exist if the linker is invoked without the -buildid flag. The go command invokes the linker with the flag, so this can only happen with manual linker invocation. Specifically, test/run.go does this in some cases. Fix this by checking the symbol existence before adding the reference. Re-enable tests on AIX. Perhaps the linker should always emit a dummy buildid even if the flag is not set... Fixes #54814. Change-Id: I43d81587151595309e189e38960cbda9a1c5ca32 Reviewed-on: https://go-review.googlesource.com/c/go/+/427620 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
2022-09-02cmd/compile: restrict //go:notinheap to runtime/internal/sysCuong Manh Le
So it won't be visible outside of runtime package. There are changes to make tests happy: - For test/directive*.go files, using "go:noinline" for testing misplaced directives instead. - Restrict test/fixedbugs/bug515.go for gccgo only. - For test/notinheap{2,3}.go, using runtime/cgo.Incomplete for marking the type as not-in-heap. Though it's somewhat clumsy, it's the easiest way to keep the test errors for not-in-heap types until we can cleanup further. - test/typeparam/mdempsky/11.go is about defined type in user code marked as go:notinheap, which can't happen after this CL, though. Fixes #46731 Change-Id: I869f5b2230c8a2a363feeec042e7723bbc416e8e Reviewed-on: https://go-review.googlesource.com/c/go/+/421882 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Joedian Reid <joedian@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
2022-09-01cmd/compile: avoid "not used" errors due to bad go/defer statementsRobert Griesemer
The syntax for go and defer specifies an arbitrary expression, not a call; the call requirement is spelled out in prose. Don't to the call check in the parser; instead move it to the type checker. This is simpler and also allows the type checker to check expressions that are not calls, and avoid "not used" errors due to such expressions. We would like to make the same change in go/parser and go/types but the change requires Go/DeferStmt nodes to hold an ast.Expr rather than an *ast.CallExpr. We cannot change that for backward- compatibility reasons. Since we don't test this behavior for the type checkers alone (only for the compiler), we get away with it for now. Follow-up on CL 425675 which introduced the extra errors in the first place. Change-Id: I90890b3079d249bdeeb76d5673246ba44bec1a7b Reviewed-on: https://go-review.googlesource.com/c/go/+/425794 Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
2022-09-01go/parser: adjustments to error messagesRobert Griesemer
- Use "expected X" rather then "expecting X". - Report a better error when a type argument list is expected. - Adjust various tests. For #54511. Change-Id: I0c5ca66ecbbdcae1a8f67377682aae6b0b6ab89a Reviewed-on: https://go-review.googlesource.com/c/go/+/425734 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com>
2022-09-01cmd/compile/internal/syntax: use BadExpr instead of fake CallExpr in bad ↵Robert Griesemer
go/defer If the go/defer syntax is bad, using a fake CallExpr may produce a follow-on error in the type checker. Instead store a BadExpr in the syntax tree (since an error has already been reported). Adjust various tests. For #54511. Change-Id: Ib2d25f8eab7d5745275188d83d11620cad6ef47c Reviewed-on: https://go-review.googlesource.com/c/go/+/425675 Reviewed-by: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
2022-09-01go/types,types2: exclude tests that need cgo.IncompleteCuong Manh Le
Since when go/types,types2 do not know about build constraints, and runtime/cgo.Incomplete is only available on platforms that support cgo. These tests are also failing on aix with failure from linker, so disable them on aix to make builder green. The fix for aix is tracked in #54814 Updates #46731 Updates #54814 Change-Id: I5d6f6e29a8196efc6c457ea64525350fc6b20309 Reviewed-on: https://go-review.googlesource.com/c/go/+/427394 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2022-09-01test: use cgo.Incomplete instead of go:notinheap for "run" testsCuong Manh Le
Same as CL 421880, but for test directory. Updates #46731 Change-Id: If8d18df013a6833adcbd40acc1a721bbc23ca6b2 Reviewed-on: https://go-review.googlesource.com/c/go/+/421881 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>