internal/refactor/inline: eliminate unnecessary binding decl

In calls from one method to another with a pointer receiver,
the inliner was inserting an unnecessary binding decl for
the receiver var, because it relied on Selection.Indirect,
but no actual indirection through the pointer is going on,
so the receiver is pure.
Instead, clear the purity flag only if there's a load
through an embedded field.

Also, fix a latent bug in the BlockStmt brace eliding
logic: we forgot to include function params/results
in scope when the block is the body of a function.

Plus tests for both.

Change-Id: I7e149f55fb344e5f733cdd6811d060ef0dc42883
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532177
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

internal/refactor/inline/inline.go

@@ -110,7 +110,8 @@ func Inline(logf func(string, ...any), caller *Caller, callee *Callee) ([]byte,
 	// (see "statement theory").
 	elideBraces := false
 	if newBlock, ok := res.new.(*ast.BlockStmt); ok {
-		parent := caller.path[nodeIndex(caller.path, res.old)+1]
+		i := nodeIndex(caller.path, res.old)
+		parent := caller.path[i+1]
 		var body []ast.Stmt
 		switch parent := parent.(type) {
 		case *ast.BlockStmt:
@@ -121,11 +122,34 @@ func Inline(logf func(string, ...any), caller *Caller, callee *Callee) ([]byte,
 			body = parent.Body
 		}
 		if body != nil {
+			callerNames := declares(body)
+
+			// If BlockStmt is a function body,
+			// include its receiver, params, and results.
+			addFieldNames := func(fields *ast.FieldList) {
+				if fields != nil {
+					for _, field := range fields.List {
+						for _, id := range field.Names {
+							callerNames[id.Name] = true
+						}
+					}
+				}
+			}
+			switch f := caller.path[i+2].(type) {
+			case *ast.FuncDecl:
+				addFieldNames(f.Recv)
+				addFieldNames(f.Type.Params)
+				addFieldNames(f.Type.Results)
+			case *ast.FuncLit:
+				addFieldNames(f.Type.Params)
+				addFieldNames(f.Type.Results)
+			}
+
 			if len(callerLabels(caller.path)) > 0 {
 				// TODO(adonovan): be more precise and reject
 				// only forward gotos across the inlined block.
 				logf("keeping block braces: caller uses control labels")
-			} else if intersects(declares(newBlock.List), declares(body)) {
+			} else if intersects(declares(newBlock.List), callerNames) {
 				logf("keeping block braces: avoids name conflict")
 			} else {
 				elideBraces = true
@@ -1060,16 +1084,16 @@ func arguments(caller *Caller, calleeDecl *ast.FuncDecl, assign1 func(*types.Var
 						debugFormatNode(caller.Fset, caller.Call.Fun),
 						fld.Name())
 				}
+				if is[*types.Pointer](arg.typ.Underlying()) {
+					arg.pure = false // implicit *ptr operation => impure
+				}
 				arg.expr = &ast.SelectorExpr{
 					X:   arg.expr,
 					Sel: makeIdent(fld.Name()),
 				}
 				arg.typ = fld.Type()
 				arg.duplicable = false
 			}
-			if seln.Indirect() {
-				arg.pure = false // one or more implicit *ptr operation => impure
-			}
 
 			// Make * or & explicit.
 			argIsPtr := arg.typ != deref(arg.typ)
@@ -1083,6 +1107,7 @@ func arguments(caller *Caller, calleeDecl *ast.FuncDecl, assign1 func(*types.Var
 				arg.expr = &ast.StarExpr{X: arg.expr}
 				arg.typ = deref(arg.typ)
 				arg.duplicable = false
+				arg.pure = false
 			}
 		}
 	}

internal/refactor/inline/inline_test.go

@@ -732,6 +732,29 @@ func TestEmbeddedFields(t *testing.T) {
 			`func _(v V) { (*V).f(&v) }`,
 			`func _(v V) { print(&(&v).U.T) }`,
 		},
+		{
+			// x is a single-assign var, and x.f does not load through a pointer
+			// (despite types.Selection.Indirect=true), so x is pure.
+			"No binding decl is required for recv in method-to-method calls.",
+			`type T struct{}; func (x *T) f() { g(); print(*x) }; func g()`,
+			`func (x *T) _() { x.f() }`,
+			`func (x *T) _() {
+	g()
+	print(*x)
+}`,
+		},
+		{
+			"Same, with implicit &recv.",
+			`type T struct{}; func (x *T) f() { g(); print(*x) }; func g()`,
+			`func (x T) _() { x.f() }`,
+			`func (x T) _() {
+	{
+		var x *T = &x
+		g()
+		print(*x)
+	}
+}`,
+		},
 	})
 }