Skip to content

Commit

Permalink
cmd/compile/internal/inline: revise closure inl position fix
Browse files Browse the repository at this point in the history
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 <thanm@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
  • Loading branch information
thanm committed Nov 24, 2021
1 parent 47db3bb commit 465b402
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
15 changes: 10 additions & 5 deletions src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions test/closure3.dir/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand Down
4 changes: 2 additions & 2 deletions test/inline.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down

0 comments on commit 465b402

Please sign in to comment.