From 465b4028082339bb7aa64ed6e30aef4c0b0413b4 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 23 Nov 2021 08:19:45 -0500 Subject: [PATCH] cmd/compile/internal/inline: revise closure inl position fix This patch revises the fix for issue 46234, fixing a bug that was accidentally introduced by CL 320913. When inlining a chunk of code with a closure expression, we want to avoid updating the source positions in the function being closed over, but we do want to update the position for the ClosureExpr itself (since it is part of the function we are inlining). CL 320913 unintentionally did away with the closure expr source position update; here we restore it again. Updates #46234. Fixes #49171. Change-Id: Iaa51bc498e374b9e5a46fa0acd7db520edbbbfca Reviewed-on: https://go-review.googlesource.com/c/go/+/366494 Trust: Than McIntosh Trust: Dan Scales Reviewed-by: Dan Scales --- src/cmd/compile/internal/inline/inl.go | 15 ++++++++++----- test/closure3.dir/main.go | 8 ++++---- test/inline.go | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 47b895f7e3..716a7fbcd9 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -1108,11 +1108,15 @@ func (subst *inlsubst) clovar(n *ir.Name) *ir.Name { // closure does the necessary substitions for a ClosureExpr n and returns the new // closure node. func (subst *inlsubst) closure(n *ir.ClosureExpr) ir.Node { - // Prior to the subst edit, set a flag in the inlsubst to - // indicated that we don't want to update the source positions in - // the new closure. If we do this, it will appear that the closure - // itself has things inlined into it, which is not the case. See - // issue #46234 for more details. + // Prior to the subst edit, set a flag in the inlsubst to indicate + // that we don't want to update the source positions in the new + // closure function. If we do this, it will appear that the + // closure itself has things inlined into it, which is not the + // case. See issue #46234 for more details. At the same time, we + // do want to update the position in the new ClosureExpr (which is + // part of the function we're working on). See #49171 for an + // example of what happens if we miss that update. + newClosurePos := subst.updatedPos(n.Pos()) defer func(prev bool) { subst.noPosUpdate = prev }(subst.noPosUpdate) subst.noPosUpdate = true @@ -1175,6 +1179,7 @@ func (subst *inlsubst) closure(n *ir.ClosureExpr) ir.Node { // Actually create the named function for the closure, now that // the closure is inlined in a specific function. newclo := newfn.OClosure + newclo.SetPos(newClosurePos) newclo.SetInit(subst.list(n.Init())) return typecheck.Expr(newclo) } diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go index 662a2e967b..7ef0a47595 100644 --- a/test/closure3.dir/main.go +++ b/test/closure3.dir/main.go @@ -94,10 +94,10 @@ func main() { return x + 2 } y, sink = func() (func(int) int, int) { // ERROR "can inline main.func12" - return func(x int) int { // ERROR "func literal does not escape" "can inline main.func12" + return func(x int) int { // ERROR "can inline main.func12" return x + 1 }, 42 - }() // ERROR "inlining call to main.func12" + }() // ERROR "func literal does not escape" "inlining call to main.func12" if y(40) != 41 { ppanic("y(40) != 41") } @@ -109,10 +109,10 @@ func main() { return x + 2 } y, sink = func() (func(int) int, int) { // ERROR "can inline main.func13.2" - return func(x int) int { // ERROR "func literal does not escape" "can inline main.func13.2" + return func(x int) int { // ERROR "can inline main.func13.2" return x + 1 }, 42 - }() // ERROR "inlining call to main.func13.2" + }() // ERROR "func literal does not escape" "inlining call to main.func13.2" if y(40) != 41 { ppanic("y(40) != 41") } diff --git a/test/inline.go b/test/inline.go index d0ebe84aa5..2780e10b19 100644 --- a/test/inline.go +++ b/test/inline.go @@ -92,9 +92,9 @@ func o() int { foo := func() int { return 1 } // ERROR "can inline o.func1" "func literal does not escape" func(x int) { // ERROR "can inline o.func2" if x > 10 { - foo = func() int { return 2 } // ERROR "func literal does not escape" "can inline o.func2" + foo = func() int { return 2 } // ERROR "can inline o.func2" } - }(11) // ERROR "inlining call to o.func2" + }(11) // ERROR "func literal does not escape" "inlining call to o.func2" return foo() } -- 2.50.0