Skip to content

Commit 5f29ce9

Browse files
committed
all: merge master (43c41b5) into gopls-release-branch.0.14
For golang/go#63220 Merge List: + 2023-10-16 43c41b5 internal/refactor/inline: reify implicit return conversions Change-Id: Ie2beb052f316efb38d52f7a35557f7c580c19da7
2 parents 28beb2d + 43c41b5 commit 5f29ce9

File tree

2 files changed

+46
-8
lines changed

2 files changed

+46
-8
lines changed

internal/refactor/inline/inline.go

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,16 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
760760
needBindingDecl := !allResultsUnreferenced ||
761761
exists(params, func(i int, p *parameter) bool { return p != nil })
762762

763+
// The two strategies below overlap for a tail call of {return exprs}:
764+
// The expr-context reduction is nice because it keeps the
765+
// caller's return stmt and merely switches its operand,
766+
// without introducing a new block, but it doesn't work with
767+
// implicit return conversions.
768+
//
769+
// TODO(adonovan): unify these cases more cleanly, allowing return-
770+
// operand replacement and implicit conversions, by adding
771+
// conversions around each return operand (if not a spread return).
772+
763773
// Special case: call to { return exprs }.
764774
//
765775
// Reduces to:
@@ -776,8 +786,7 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
776786
// callee's body expression, suitably substituted.
777787
if len(calleeDecl.Body.List) == 1 &&
778788
is[*ast.ReturnStmt](calleeDecl.Body.List[0]) &&
779-
len(calleeDecl.Body.List[0].(*ast.ReturnStmt).Results) > 0 && // not a bare return
780-
safeReturn(caller, calleeSymbol, callee) {
789+
len(calleeDecl.Body.List[0].(*ast.ReturnStmt).Results) > 0 { // not a bare return
781790
results := calleeDecl.Body.List[0].(*ast.ReturnStmt).Results
782791

783792
context := callContext(caller.path)
@@ -839,11 +848,24 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
839848

840849
if callee.NumResults == 1 {
841850
logf("strategy: reduce expr-context call to { return expr }")
851+
// (includes some simple tail-calls)
852+
853+
// Make implicit return conversion explicit.
854+
if callee.TrivialReturns < callee.TotalReturns {
855+
results[0] = convert(calleeDecl.Type.Results.List[0].Type, results[0])
856+
}
842857

843858
res.old = caller.Call
844859
res.new = results[0]
845-
} else {
860+
return res, nil
861+
862+
} else if callee.TrivialReturns == callee.TotalReturns {
846863
logf("strategy: reduce spread-context call to { return expr }")
864+
// There is no general way to reify conversions in a spread
865+
// return, hence the requirement above.
866+
//
867+
// TODO(adonovan): allow this reduction when no
868+
// conversion is required by the context.
847869

848870
// The call returns multiple results but is
849871
// not a standalone call statement. It must
@@ -880,8 +902,8 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
880902
default:
881903
return nil, fmt.Errorf("internal error: unexpected context %T for spread call", context)
882904
}
905+
return res, nil
883906
}
884-
return res, nil
885907
}
886908
}
887909

@@ -911,7 +933,7 @@ func inline(logf func(string, ...any), caller *Caller, callee *gobCallee) (*resu
911933
// or implicit) return.
912934
if ret, ok := callContext(caller.path).(*ast.ReturnStmt); ok &&
913935
len(ret.Results) == 1 &&
914-
safeReturn(caller, calleeSymbol, callee) &&
936+
tailCallSafeReturn(caller, calleeSymbol, callee) &&
915937
!callee.HasBareReturn &&
916938
(!needBindingDecl || bindingDeclStmt != nil) &&
917939
!hasLabelConflict(caller.path, callee.Labels) &&
@@ -2624,9 +2646,9 @@ func declares(stmts []ast.Stmt) map[string]bool {
26242646
return names
26252647
}
26262648

2627-
// safeReturn reports whether the callee's return statements may be safely
2649+
// tailCallSafeReturn reports whether the callee's return statements may be safely
26282650
// used to return from the function enclosing the caller (which must exist).
2629-
func safeReturn(caller *Caller, calleeSymbol *types.Func, callee *gobCallee) bool {
2651+
func tailCallSafeReturn(caller *Caller, calleeSymbol *types.Func, callee *gobCallee) bool {
26302652
// It is safe if all callee returns involve only trivial conversions.
26312653
if callee.TrivialReturns == callee.TotalReturns {
26322654
return true

internal/refactor/inline/inline_test.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,16 @@ func TestBasics(t *testing.T) {
397397
var _ = 1
398398
var _ = 2
399399
var _ = 3
400+
}`,
401+
},
402+
{
403+
// (a regression test for a missing conversion)
404+
"Implicit return conversions are inserted in expr-context reduction.",
405+
`func f(x int) error { return nil }`,
406+
`func _() { if err := f(0); err != nil {} }`,
407+
`func _() {
408+
if err := error(nil); err != nil {
409+
}
400410
}`,
401411
},
402412
})
@@ -673,7 +683,7 @@ func TestTailCallStrategy(t *testing.T) {
673683
"Tail call with non-trivial return conversion (caller.sig != callee.sig).",
674684
`func f() error { return E{} }; type E struct{error}`,
675685
`func _() any { return f() }`,
676-
`func _() any { return func() error { return E{} }() }`,
686+
`func _() any { return error(E{}) }`,
677687
},
678688
})
679689
}
@@ -714,6 +724,12 @@ func TestSpreadCalls(t *testing.T) {
714724
`func _() (int, error) { return f() }`,
715725
`func _() (int, error) { return 0, nil }`,
716726
},
727+
{
728+
"Implicit return conversions defeat reduction of spread returns, for now.",
729+
`func f(x int) (_, _ error) { return nil, nil }`,
730+
`func _() { _, _ = f(0) }`,
731+
`func _() { _, _ = func() (_, _ error) { return nil, nil }() }`,
732+
},
717733
})
718734
}
719735

0 commit comments

Comments
 (0)