From 063a91c0abef445154df1ba34ffb500eeccfe8bc Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 30 Oct 2020 12:58:28 -0700 Subject: [PATCH] cmd/compile: fix recognition of unnamed return variables In golang.org/cl/266199, I reused the existing code in inlining that recognizes anonymous variables. However, it turns out that code mistakenly recognizes anonymous return parameters as named when inlining a function from the same package. The issue is funcargs (which is only used for functions parsed from source) synthesizes ~r names for anonymous return parameters, but funcargs2 (which is only used for functions imported from export data) does not. This CL fixes the behavior so that anonymous return parameters are handled identically whether a function is inlined within the same package or across packages. It also adds a proper cross-package test case demonstrating #33160 is fixed in both cases. Change-Id: Iaa39a23f5666979a1f5ca6d09fc8c398e55b784c Reviewed-on: https://go-review.googlesource.com/c/go/+/266719 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le Reviewed-by: David Chase Trust: Matthew Dempsky --- src/cmd/compile/internal/gc/inl.go | 2 +- src/cmd/compile/internal/logopt/logopt_test.go | 8 ++++---- test/fixedbugs/issue42284.dir/a.go | 9 ++++++++- test/fixedbugs/issue42284.dir/b.go | 5 ++++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 8a5c6d86666..253036fea64 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -1054,7 +1054,7 @@ func mkinlcall(n, fn *Node, maxCost int32, inlMap map[*Node]bool) *Node { var retvars []*Node for i, t := range fn.Type.Results().Fields().Slice() { var m *Node - if n := asNode(t.Nname); n != nil && !n.isBlank() { + if n := asNode(t.Nname); n != nil && !n.isBlank() && !strings.HasPrefix(n.Sym.Name, "~r") { m = inlvar(n) m = typecheck(m, ctxExpr) inlvars[n] = m diff --git a/src/cmd/compile/internal/logopt/logopt_test.go b/src/cmd/compile/internal/logopt/logopt_test.go index fca85c10fbb..51bab49518c 100644 --- a/src/cmd/compile/internal/logopt/logopt_test.go +++ b/src/cmd/compile/internal/logopt/logopt_test.go @@ -213,15 +213,15 @@ func s15a8(x *[15]int64) [15]int64 { `"relatedInformation":[`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: flow: y = z:"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from y := z (assign-pair)"},`+ - `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: flow: ~r1 = y:"},`+ + `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: flow: ~R0 = y:"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from y.b (dot of pointer)"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from \u0026y.b (address-of)"},`+ `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":9},"end":{"line":4,"character":9}}},"message":"inlineLoc"},`+ - `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from ~r1 = \u003cN\u003e (assign-pair)"},`+ - `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: flow: ~r2 = ~r1:"},`+ - `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: from return (*int)(~r1) (return)"}]}`) + `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from ~R0 = \u003cN\u003e (assign-pair)"},`+ + `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: flow: ~r2 = ~R0:"},`+ + `{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: from return (*int)(~R0) (return)"}]}`) }) } diff --git a/test/fixedbugs/issue42284.dir/a.go b/test/fixedbugs/issue42284.dir/a.go index e1271af32d0..ffe9310be35 100644 --- a/test/fixedbugs/issue42284.dir/a.go +++ b/test/fixedbugs/issue42284.dir/a.go @@ -9,12 +9,19 @@ type T int func (T) M() {} // ERROR "can inline T.M" +func E() I { // ERROR "can inline E" + return T(0) // ERROR "T\(0\) escapes to heap" +} + func F(i I) I { // ERROR "can inline F" "leaking param: i to result ~r1 level=0" i = nil return i } -func g() { // ERROR "can inline g" +func g() { + h := E() // ERROR "inlining call to E" "T\(0\) does not escape" + h.M() // ERROR "devirtualizing h.M to T" + // BAD: T(0) could be stack allocated. i := F(T(0)) // ERROR "inlining call to F" "T\(0\) escapes to heap" diff --git a/test/fixedbugs/issue42284.dir/b.go b/test/fixedbugs/issue42284.dir/b.go index 3305166db06..652aa321226 100644 --- a/test/fixedbugs/issue42284.dir/b.go +++ b/test/fixedbugs/issue42284.dir/b.go @@ -6,7 +6,10 @@ package b import "./a" -func g() { // ERROR "can inline g" +func g() { + h := a.E() // ERROR "inlining call to a.E" "a.I\(a.T\(0\)\) does not escape" + h.M() // ERROR "devirtualizing h.M to a.T" + // BAD: T(0) could be stack allocated. i := a.F(a.T(0)) // ERROR "inlining call to a.F" "a.T\(0\) escapes to heap"