Revert "cmd/compile: allow more inlining of functions that construct closures"

This reverts commit f8162a0e726f4b3a9df60a37e8ca7883dde61914.

Reason for revert: https://github.com/golang/go/issues/59680

Change-Id: I91821c691a2d019ff0ad5b69509e32f3d56b8f67
Reviewed-on: https://go-review.googlesource.com/c/go/+/485498
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>

2 files changed, 19 insertions, 14 deletions

diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go
index 81aa200a84..24812c8c0d 100644
--- a/src/cmd/compile/internal/inline/inl.go
+++ b/src/cmd/compile/internal/inline/inl.go
@@ -506,8 +506,6 @@ func (v *hairyVisitor) tooHairy(fn *ir.Func) bool {
 	return false
 }
 
-// doNode visits n and its children, updates the state in v, and returns true if
-// n makes the current function too hairy for inlining.
 func (v *hairyVisitor) doNode(n ir.Node) bool {
 	if n == nil {
 		return false
@@ -639,10 +637,13 @@ func (v *hairyVisitor) doNode(n ir.Node) bool {
 		// TODO(danscales): Maybe make budget proportional to number of closure
 		// variables, e.g.:
 		//v.budget -= int32(len(n.(*ir.ClosureExpr).Func.ClosureVars) * 3)
-		// TODO(austin): However, if we're able to inline this closure into
-		// v.curFunc, then we actually pay nothing for the closure captures. We
-		// should try to account for that if we're going to account for captures.
 		v.budget -= 15
+		// Scan body of closure (which DoChildren doesn't automatically
+		// do) to check for disallowed ops in the body and include the
+		// body in the budget.
+		if doList(n.(*ir.ClosureExpr).Func.Body, v.do) {
+			return true
+		}
 
 	case ir.OGO,
 		ir.ODEFER,
diff --git a/src/cmd/compile/internal/test/inl_test.go b/src/cmd/compile/internal/test/inl_test.go
index 205b746dd8..2a16b21cef 100644
--- a/src/cmd/compile/internal/test/inl_test.go
+++ b/src/cmd/compile/internal/test/inl_test.go
@@ -180,15 +180,19 @@ func TestIntendedInlining(t *testing.T) {
 		"net": {
 			"(*UDPConn).ReadFromUDP",
 		},
-		"sync": {
-			// Both OnceFunc and its returned closure need to be inlinable so
-			// that the returned closure can be inlined into the caller of OnceFunc.
-			"OnceFunc",
-			"OnceFunc.func2", // The returned closure.
-			// TODO(austin): It would be good to check OnceValue and OnceValues,
-			// too, but currently they aren't reported because they have type
-			// parameters and aren't instantiated in sync.
-		},
+		// These testpoints commented out for now, since CL 479095
+		// had to be reverted. We can re-enable this once we roll
+		// forward with a new version of 479095.
+		/*
+			"sync": {
+				// Both OnceFunc and its returned closure need to be inlinable so
+				// that the returned closure can be inlined into the caller of OnceFunc.
+				"OnceFunc",
+				"OnceFunc.func2", // The returned closure.
+				// TODO(austin): It would be good to check OnceValue and OnceValues,
+				// too, but currently they aren't reported because they have type
+				// parameters and aren't instantiated in sync.
+			}, */
 		"sync/atomic": {
 			// (*Bool).CompareAndSwap handled below.
 			"(*Bool).Load",