mirror of
https://github.com/golang/go
synced 2024-11-26 06:27:58 -07:00
cmd/compile: fix reassignment check
CL 65071 enabled inlining for local closures with no captures. To determine safety of inlining a call sites, we check whether the variable holding the closure has any assignments after its original definition. Unfortunately, that check did not catch OAS2MAPR and OAS2DOTTYPE, leading to incorrect inlining when a variable holding a closure was subsequently reassigned through a type conversion or a 2-valued map access. There was another more subtle issue wherein reassignment check would always return a false positive for closure calls inside other closures. This was caused by the Name.Curfn field of local variables pointing to the OCLOSURE node instead of the corresponding ODCLFUNC, which resulted in reassigned walking an empty Nbody and thus never seeing any reassignments. This CL fixes these oversights and adds many more tests for closure inlining which ensure not only that inlining triggers but also the correctness of the resulting code. Updates #15561 Change-Id: I74bdae849c4ecfa328546d6d62b512e8d54d04ce Reviewed-on: https://go-review.googlesource.com/75770 Reviewed-by: Hugues Bruant <hugues.bruant@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
parent
d593f85ebd
commit
483e298daa
@ -661,8 +661,17 @@ func reassigned(n *Node) (bool, *Node) {
|
||||
if n.Name.Curfn == nil {
|
||||
return true, nil
|
||||
}
|
||||
f := n.Name.Curfn
|
||||
// There just might be a good reason for this although this can be pretty surprising:
|
||||
// local variables inside a closure have Curfn pointing to the OCLOSURE node instead
|
||||
// of the corresponding ODCLFUNC.
|
||||
// We need to walk the function body to check for reassignments so we follow the
|
||||
// linkage to the ODCLFUNC node as that is where body is held.
|
||||
if f.Op == OCLOSURE {
|
||||
f = f.Func.Closure
|
||||
}
|
||||
v := reassignVisitor{name: n}
|
||||
a := v.visitList(n.Name.Curfn.Nbody)
|
||||
a := v.visitList(f.Nbody)
|
||||
return a != nil, a
|
||||
}
|
||||
|
||||
@ -680,7 +689,7 @@ func (v *reassignVisitor) visit(n *Node) *Node {
|
||||
return n
|
||||
}
|
||||
return nil
|
||||
case OAS2, OAS2FUNC:
|
||||
case OAS2, OAS2FUNC, OAS2MAPR, OAS2DOTTYPE:
|
||||
for _, p := range n.List.Slice() {
|
||||
if p == v.name && n != v.name.Name.Defn {
|
||||
return n
|
||||
|
173
test/closure3.dir/main.go
Normal file
173
test/closure3.dir/main.go
Normal file
@ -0,0 +1,173 @@
|
||||
// Copyright 2017 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
// Check correctness of various closure corner cases that
|
||||
// that are expected to be inlined
|
||||
|
||||
package main
|
||||
|
||||
var ok bool
|
||||
var sink int
|
||||
|
||||
func main() {
|
||||
{
|
||||
if x := func() int { // ERROR "can inline main.func1"
|
||||
return 1
|
||||
}(); x != 1 { // ERROR "inlining call to main.func1"
|
||||
panic("x != 1")
|
||||
}
|
||||
if x := func() int { // ERROR "can inline main.func2" "func literal does not escape"
|
||||
return 1
|
||||
}; x() != 1 { // ERROR "inlining call to main.func2"
|
||||
panic("x() != 1")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
if y := func(x int) int { // ERROR "can inline main.func3"
|
||||
return x + 2
|
||||
}(40); y != 42 { // ERROR "inlining call to main.func3"
|
||||
panic("y != 42")
|
||||
}
|
||||
if y := func(x int) int { // ERROR "can inline main.func4" "func literal does not escape"
|
||||
return x + 2
|
||||
}; y(40) != 42 { // ERROR "inlining call to main.func4"
|
||||
panic("y(40) != 42")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
y := func(x int) int { // ERROR "can inline main.func5" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y = func(x int) int { // ERROR "can inline main.func6" "func literal does not escape"
|
||||
return x + 1
|
||||
}
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
func() { // ERROR "func literal does not escape"
|
||||
y := func(x int) int { // ERROR "can inline main.func7.1" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y = func(x int) int { // ERROR "can inline main.func7.2" "func literal does not escape"
|
||||
return x + 1
|
||||
}
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
{
|
||||
y := func(x int) int { // ERROR "can inline main.func8" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, sink = func(x int) int { // ERROR "can inline main.func9" "func literal does not escape"
|
||||
return x + 1
|
||||
}, 42
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
func() { // ERROR "func literal does not escape"
|
||||
y := func(x int) int { // ERROR "can inline main.func10.1" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, sink = func(x int) int { // ERROR "can inline main.func10.2" "func literal does not escape"
|
||||
return x + 1
|
||||
}, 42
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
{
|
||||
y := func(x int) int { // ERROR "can inline main.func11" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, sink = func() (func(int)int, int) { // ERROR "func literal does not escape"
|
||||
return func(x int) int { // ERROR "can inline main.func12" "func literal escapes"
|
||||
return x + 1
|
||||
}, 42
|
||||
}()
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
func() { // ERROR "func literal does not escape"
|
||||
y := func(x int) int { // ERROR "can inline main.func13.1" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, sink = func() (func(int) int, int) { // ERROR "func literal does not escape"
|
||||
return func(x int) int { // ERROR "can inline main.func13.2" "func literal escapes"
|
||||
return x + 1
|
||||
}, 42
|
||||
}()
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
{
|
||||
y := func(x int) int { // ERROR "can inline main.func14" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, ok = map[int]func(int)int { // ERROR "does not escape"
|
||||
0: func (x int) int { return x + 1 }, // ERROR "can inline main.func15" "func literal escapes"
|
||||
}[0]
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
func() { // ERROR "func literal does not escape"
|
||||
y := func(x int) int { // ERROR "can inline main.func16.1" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, ok = map[int]func(int) int{// ERROR "does not escape"
|
||||
0: func(x int) int { return x + 1 }, // ERROR "can inline main.func16.2" "func literal escapes"
|
||||
}[0]
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
{
|
||||
y := func(x int) int { // ERROR "can inline main.func17" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, ok = interface{}(func (x int) int { // ERROR "can inline main.func18" "does not escape"
|
||||
return x + 1
|
||||
}).(func(int)int)
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
func() { // ERROR "func literal does not escape"
|
||||
y := func(x int) int { // ERROR "can inline main.func19.1" "func literal does not escape"
|
||||
return x + 2
|
||||
}
|
||||
y, ok = interface{}(func(x int) int { // ERROR "can inline main.func19.2" "does not escape"
|
||||
return x + 1
|
||||
}).(func(int) int)
|
||||
if y(40) != 41 {
|
||||
panic("y(40) != 41")
|
||||
}
|
||||
}()
|
||||
}
|
||||
}
|
10
test/closure3.go
Normal file
10
test/closure3.go
Normal file
@ -0,0 +1,10 @@
|
||||
// errorcheckandrundir -0 -m
|
||||
|
||||
// Copyright 2017 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
// Check correctness of various closure corner cases that
|
||||
// that are expected to be inlined
|
||||
|
||||
package ignored
|
Loading…
Reference in New Issue
Block a user