internal/refactor/inline: use binding decl with literalization

Previously, literalization, the strategy of last resort,
didn't make use of a binding decl even when one was available.
But a binding decl can make literalization more readable
as it puts the arguments before the body, their natural place,
which is important especially when the body is longer.

   func(params) { body } (args)
   =>
   func() { var params = args; body }()

We don't yet attempt to do this if any named result is
referenced, because the binding decl would duplicate it;
teasing apart the params and results of the binding
decl is left for future work.

Plus tests.

Change-Id: I51da3016157c1531c2d57962c4bddb0203ac0946
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535456
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

internal/refactor/inline/doc.go

@@ -251,20 +251,13 @@ TODO(adonovan): future work:
     could be achieved by returning metadata alongside the result
     and having the client conditionally discard the change.
 
-  - Is it acceptable to skip effects that are limited to runtime
-    panics? Can we avoid evaluating an argument x.f
-    or a[i] when the corresponding parameter is unused?
-
   - Support inlining of generic functions, replacing type parameters
     by their instantiations.
 
   - Support inlining of calls to function literals ("closures").
     But note that the existing algorithm makes widespread assumptions
     that the callee is a package-level function or method.
 
-  - Eliminate parens and braces inserted conservatively when they
-    are redundant.
-
   - Eliminate explicit conversions of "untyped" literals inserted
     conservatively when they are redundant. For example, the
     conversion int32(1) is redundant when this value is used only as a

internal/refactor/inline/inline.go

@@ -989,15 +989,32 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
 	}
 
 	// Infallible general case: literalization.
+	//
+	//    func(params) { body }(args)
+	//
 	logf("strategy: literalization")
+	funcLit := &ast.FuncLit{
+		Type: calleeDecl.Type,
+		Body: calleeDecl.Body,
+	}
+
+	// Literalization can still make use of a binding
+	// decl as it gives a more natural reading order:
+	//
+	//    func() { var params = args; body }()
+	//
+	// TODO(adonovan): relax the allResultsUnreferenced requirement
+	// by adding a parameter-only (no named results) binding decl.
+	if bindingDeclStmt != nil && allResultsUnreferenced {
+		funcLit.Type.Params.List = nil
+		remainingArgs = nil
+		funcLit.Body.List = prepend(bindingDeclStmt, funcLit.Body.List...)
+	}
 
 	// Emit a new call to a function literal in place of
 	// the callee name, with appropriate replacements.
 	newCall := &ast.CallExpr{
-		Fun: &ast.FuncLit{
-			Type: calleeDecl.Type,
-			Body: calleeDecl.Body,
-		},
+		Fun:      funcLit,
 		Ellipsis: token.NoPos, // f(slice...) is always simplified
 		Args:     remainingArgs,
 	}
@@ -1536,11 +1553,12 @@ func updateCalleeParams(calleeDecl *ast.FuncDecl, params []*parameter) {
 
 // createBindingDecl constructs a "binding decl" that implements
 // parameter assignment and declares any named result variables
-// referenced by the callee.
+// referenced by the callee. It returns nil if there were no
+// unsubstituted parameters.
 //
 // It may not always be possible to create the decl (e.g. due to
-// shadowing), in which case it returns nil; but if it succeeds, the
-// declaration may be used by reduction strategies to relax the
+// shadowing), in which case it also returns nil; but if it succeeds,
+// the declaration may be used by reduction strategies to relax the
 // requirement that all parameters have been substituted.
 //
 // For example, a call:
@@ -1682,14 +1700,19 @@ func createBindingDecl(logf func(string, ...any), caller *Caller, args []*argume
 		}
 	}
 
-	decl := &ast.DeclStmt{
+	if len(specs) == 0 {
+		logf("binding decl not needed: all parameters substituted")
+		return nil
+	}
+
+	stmt := &ast.DeclStmt{
 		Decl: &ast.GenDecl{
 			Tok:   token.VAR,
 			Specs: specs,
 		},
 	}
-	logf("binding decl: %s", debugFormatNode(caller.Fset, decl))
-	return decl
+	logf("binding decl: %s", debugFormatNode(caller.Fset, stmt))
+	return stmt
 }
 
 // lookup does a symbol lookup in the lexical environment of the caller.

internal/refactor/inline/inline_test.go

@@ -693,7 +693,7 @@ func TestVariadic(t *testing.T) {
 			"Variadic cancellation (basic).",
 			`func f(args ...any) { defer f(&args); println(args) }`,
 			`func _(slice []any) { f(slice...) }`,
-			`func _(slice []any) { func(args []any) { defer f(&args); println(args) }(slice) }`,
+			`func _(slice []any) { func() { var args []any = slice; defer f(&args); println(args) }() }`,
 		},
 		{
 			"Variadic cancellation (literalization with parameter elimination).",
@@ -797,6 +797,24 @@ func TestParameterBindingDecl(t *testing.T) {
 			`func _(x *T) { f(x, recover()) }`,
 			`func _(x *T) { x.g(recover()) }`,
 		},
+		{
+			"Literalization can make use of a binding decl (all params).",
+			`func f(x, y int) int { defer println(); return y + x }; func g(int) int`,
+			`func _() { println(f(g(1), g(2))) }`,
+			`func _() { println(func() int { var x, y int = g(1), g(2); defer println(); return y + x }()) }`,
+		},
+		{
+			"Literalization can make use of a binding decl (some params).",
+			`func f(x, y int) int { z := y + x; defer println(); return z }; func g(int) int`,
+			`func _() { println(f(g(1), g(2))) }`,
+			`func _() { println(func() int { var x int = g(1); z := g(2) + x; defer println(); return z }()) }`,
+		},
+		{
+			"Literalization can't yet use of a binding decl if named results.",
+			`func f(x, y int) (z int) { z = y + x; defer println(); return }; func g(int) int`,
+			`func _() { println(f(g(1), g(2))) }`,
+			`func _() { println(func(x int) (z int) { z = g(2) + x; defer println(); return }(g(1))) }`,
+		},
 	})
 }
 

internal/refactor/inline/testdata/basic-literal.txtar

@@ -23,7 +23,7 @@ func add(x, y int) int { defer print(); return x + y }
 package a
 
 func _() {
-	func(x int) int { defer print(); return x + 2 }(recover().(int)) //@ inline(re"add", add1)
+	func() int { var x int = recover().(int); defer print(); return x + 2 }() //@ inline(re"add", add1)
 }
 
 func add(x, y int) int { defer print(); return x + y }

internal/refactor/inline/testdata/issue62667.txtar

@@ -38,7 +38,7 @@ import (
 )
 
 func A() {
-	func(path string) { defer func() {}(); path0.Split(path) }(g()) //@ inline(re"Dir", result)
+	func() { var path string = g(); defer func() {}(); path0.Split(path) }() //@ inline(re"Dir", result)
 }
 
 func g() string