internal/refactor/inline: fix spurious caller mutation

A failing test in a later CL was rooted in a bug allowing the inliner to
mutate the caller syntax: during literalization we were clearing
positions after inserting the binding decl, and the binding decl may
refer to syntax in the caller.

This was not caught by tests, because the deepHash utility itself had a
bug: binary.Write is a no op for ints or uints, because they are not
fixed width (this was rather surprising behavior). This is fixed by
adding handling for Int and Uint values, and delegating to binary.Write
only in the cases that are known to be supported.

Change-Id: Icdf03f4d2cbc91b347b876cc39d8a48484c90c7c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/631295
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

internal/refactor/inline/inline.go

@@ -1354,6 +1354,10 @@ func (st *state) inlineCall() (*inlineCallResult, error) {
 		Type: calleeDecl.Type,
 		Body: calleeDecl.Body,
 	}
+	// clear positions before prepending the binding decl below, since the
+	// binding decl contains syntax from the caller and we must not mutate the
+	// caller. (This was a prior bug.)
+	clearPositions(funcLit)
 
 	// Literalization can still make use of a binding
 	// decl as it gives a more natural reading order:
@@ -1375,7 +1379,6 @@ func (st *state) inlineCall() (*inlineCallResult, error) {
 		Ellipsis: token.NoPos, // f(slice...) is always simplified
 		Args:     remainingArgs,
 	}
-	clearPositions(newCall.Fun)
 	res.old = caller.Call
 	res.new = newCall
 	return res, nil

internal/refactor/inline/inline_test.go

@@ -1486,6 +1486,12 @@ func TestSubstitutionPreservesParameterType(t *testing.T) {
 			`func _() { T.f(1) }`,
 			`func _() { T(1).g() }`,
 		},
+		{
+			"Implicit dereference is made explicit", // TODO(rfindley): fix unnecessary literalization.
+			`type T struct{ field int }; func (x T) f(y T) bool { return x == y && x.field == x.g() }; func (x T) g() int { return 0 }`,
+			`func _() { var t *T; var y T; _ = t.f(y) }`,
+			`func _() { var t *T; var y T; _ = func() bool { var x T = *t; return x == y && x.field == x.g() }() }`,
+		},
 		{
 			"Check for shadowing error on type used in the conversion.",
 			`func f(x T) { _ = &x == (*T)(nil) }; type T int16`,
@@ -1845,14 +1851,22 @@ func deepHash(n ast.Node) any {
 				visit(v.Elem())
 			}
 
-		case reflect.Array, reflect.Chan, reflect.Func, reflect.Map, reflect.UnsafePointer:
-			panic(v) // unreachable in AST
+		case reflect.String:
+			writeUint64(uint64(v.Len()))
+			hasher.Write([]byte(v.String()))
 
-		default: // bool, string, number
-			if v.Kind() == reflect.String { // proper framing
-				writeUint64(uint64(v.Len()))
-			}
+		case reflect.Int:
+			writeUint64(uint64(v.Int()))
+
+		case reflect.Uint:
+			writeUint64(uint64(v.Uint()))
+
+		case reflect.Bool, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+			// Bools and fixed width numbers can be handled by binary.Write.
 			binary.Write(hasher, le, v.Interface())
+
+		default: // reflect.Array, reflect.Chan, reflect.Func, reflect.Map, reflect.UnsafePointer, reflect.Uintptr
+			panic(v) // unreachable in AST
 		}
 	}
 	visit(reflect.ValueOf(n))
@@ -1861,3 +1875,23 @@ func deepHash(n ast.Node) any {
 	hasher.Sum(hash[:0])
 	return hash
 }
+
+func TestDeepHash(t *testing.T) {
+	// This test reproduces a bug in DeepHash that was encountered during work on
+	// the inliner.
+	//
+	// TODO(rfindley): consider replacing this with a fuzz test.
+	id := &ast.Ident{
+		NamePos: 2,
+		Name:    "t",
+	}
+	c := &ast.CallExpr{
+		Fun: id,
+	}
+	h1 := deepHash(c)
+	id.NamePos = 1
+	h2 := deepHash(c)
+	if h1 == h2 {
+		t.Fatal("bad")
+	}
+}