Skip to content

Commit 8e819d2

Browse files
adonovangopherbot
authored andcommitted
internal/refactor/inline: built-ins may affect inference
This change fixes a bug that caused argument conversions to be neglected when inlining calls to functions whose body calls certain built-ins such as new: func f(x int64) *int64 { return new(x) } var _ *int64 = f(42) // => new(42), should be new(int64(42)) The solution is to consider built-in functions as well as ordinary non-method functions when computing whether the argument may affect type inference. This results in a slight style regression when dealing with complex numbers, but they are rare. + test Fixes golang/go#76287 Change-Id: I7bc763cd41a7ec39abf851cb25acd568477b658a Reviewed-on: https://go-review.googlesource.com/c/tools/+/726480 Commit-Queue: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Alan Donovan <[email protected]>
1 parent 61df39e commit 8e819d2

File tree

4 files changed

+95
-3
lines changed

4 files changed

+95
-3
lines changed

internal/refactor/inline/callee.go

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -725,8 +725,48 @@ func analyzeAssignment(info *types.Info, stack []ast.Node) (assignable, ifaceAss
725725
paramType := paramTypeAtIndex(sig, call, i)
726726
ifaceAssign := paramType == nil || types.IsInterface(paramType)
727727
affectsInference := false
728-
if fn := typeutil.StaticCallee(info, call); fn != nil {
729-
if sig2 := fn.Type().(*types.Signature); sig2.Recv() == nil {
728+
switch callee := typeutil.Callee(info, call).(type) {
729+
case *types.Builtin:
730+
// Consider this litmus test:
731+
//
732+
// func f(x int64) any { return max(x) }
733+
// func main() { fmt.Printf("%T", f(42)) }
734+
//
735+
// If we lose the implicit conversion from untyped int
736+
// to int64, the type inferred for the max(x) call changes,
737+
// resulting in a different dynamic behavior: it prints
738+
// int, not int64.
739+
//
740+
// Inferred result type affected:
741+
// new
742+
// complex, real, imag
743+
// min, max
744+
//
745+
// Dynamic behavior change:
746+
// append -- dynamic type of append([]any(nil), x)[0]
747+
// delete(m, x) -- dynamic key type where m is map[any]unit
748+
// panic -- dynamic type of panic value
749+
//
750+
// Unaffected:
751+
// recover
752+
// make
753+
// len, cap
754+
// clear
755+
// close
756+
// copy
757+
// print, println -- only uses underlying types (?)
758+
//
759+
// The dynamic type cases are all covered by
760+
// the ifaceAssign logic.
761+
switch callee.Name() {
762+
case "new", "complex", "real", "imag", "min", "max":
763+
affectsInference = true
764+
}
765+
766+
case *types.Func:
767+
// Only standalone (non-method) functions have type
768+
// parameters affected by the call arguments.
769+
if sig2 := callee.Signature(); sig2.Recv() == nil {
730770
originParamType := paramTypeAtIndex(sig2, call, i)
731771
affectsInference = originParamType == nil || new(typeparams.Free).Has(originParamType)
732772
}

internal/refactor/inline/falcon_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,9 @@ func TestFalconComplex(t *testing.T) {
341341
"Complex arithmetic (good).",
342342
`func f(re, im float64, z complex128) byte { return "x"[int(real(complex(re, im)*complex(re, -im)-z))] }`,
343343
`func _() { f(1, 2, 5+0i) }`,
344-
`func _() { _ = "x"[int(real(complex(1, 2)*complex(1, -2)-(5+0i)))] }`,
344+
// The float64 conversions are excessively conservative here
345+
// but in general may affect the type of complex produced.
346+
`func _() { _ = "x"[int(real(complex(float64(1), float64(2))*complex(float64(1), -2)-(5+0i)))] }`,
345347
},
346348
{
347349
"Complex arithmetic (bad).",

internal/refactor/inline/inline_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ func TestData(t *testing.T) {
7575
testenv.NeedsTool(t, "cgo")
7676
}
7777

78+
// Some tests need specific Go versions.
79+
// TODO(adonovan): remove when go1.26 is assured.
80+
if strings.HasSuffix(t.Name(), "newexpr.txtar") {
81+
testenv.NeedsGo1Point(t, 26)
82+
}
83+
7884
// Extract archive to temporary tree.
7985
ar, err := txtar.ParseFile(file)
8086
if err != nil {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
Regression test for https://go.dev/issue/76287:
2+
ensure that inlining a function that wraps go1.26's new(expr)
3+
correctly inserts a conversion as needed, just as it would for a
4+
declared generic function equivalent to 'new'.
5+
6+
- f uses a declared _new(expr) function.
7+
- g uses the built-in new(expr) function.
8+
9+
-- go.mod --
10+
module testdata
11+
go 1.26
12+
13+
-- common.go --
14+
package p
15+
16+
func _new[T any](x T) *T { return &x }
17+
18+
-- f.go --
19+
package p
20+
21+
func f(x int64) *int64 { return _new(x) }
22+
23+
var _ *int64 = f(42) //@ inline(re"f", f)
24+
25+
-- g.go --
26+
package p
27+
28+
func g(x int64) *int64 { return new(x) }
29+
30+
var _ *int64 = g(42) //@ inline(re"g", g)
31+
32+
-- f --
33+
package p
34+
35+
func f(x int64) *int64 { return _new(x) }
36+
37+
var _ *int64 = _new(int64(42)) //@ inline(re"f", f)
38+
39+
-- g --
40+
package p
41+
42+
func g(x int64) *int64 { return new(x) }
43+
44+
var _ *int64 = new(int64(42)) //@ inline(re"g", g)

0 commit comments

Comments
 (0)