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>

diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go
index a509d2d64824a2785d54701683a17fa2df45be51..ea31da9b15d12c349f0341ba3d03a03af8998db1 100644 (file)
--- a/src/cmd/compile/internal/gc/inl.go
+++ b/src/cmd/compile/internal/gc/inl.go
@@ -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

diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go
new file mode 100644 (file)
index 0000000..5629a52
--- /dev/null
+++ b/test/closure3.dir/main.go
@@ -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")
+                       }
+               }()
+       }
+}

diff --git a/test/closure3.go b/test/closure3.go
new file mode 100644 (file)
index 0000000..263d8fc
--- /dev/null
+++ b/test/closure3.go
@@ -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