internal/refactor/inline: T{} is duplicable for struct/array
This change makes empty composite literals of aggregate
(struct/array) types duplicable. Nonempty literals remain
nonduplicable on grounds of verbosity; and map and slice
literals are nonduplicable because they allocate a new
variable.
Change-Id: I9e2d778e004fb4743fd242c4e81d00e55830a6bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534397
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go
index 1882c12..618b179 100644
--- a/internal/refactor/inline/inline.go
+++ b/internal/refactor/inline/inline.go
@@ -1998,10 +1998,28 @@
case *ast.UnaryExpr: // e.g. +1, -1
return (e.Op == token.ADD || e.Op == token.SUB) && duplicable(info, e.X)
+ case *ast.CompositeLit:
+ // Empty struct or array literals T{} are duplicable.
+ // (Non-empty literals are too verbose, and slice/map
+ // literals allocate indirect variables.)
+ if len(e.Elts) == 0 {
+ switch info.TypeOf(e).Underlying().(type) {
+ case *types.Struct, *types.Array:
+ return true
+ }
+ }
+ return false
+
case *ast.CallExpr:
// Don't treat a conversion T(x) as duplicable even
// if x is duplicable because it could duplicate
- // allocations. There may be cases to tease apart here.
+ // allocations.
+ //
+ // TODO(adonovan): there are cases to tease apart here:
+ // duplicating string([]byte) conversions increases
+ // allocation but doesn't change behavior, but the
+ // reverse, []byte(string), allocates a distinct array,
+ // which is observable
return false
case *ast.SelectorExpr:
diff --git a/internal/refactor/inline/inline_test.go b/internal/refactor/inline/inline_test.go
index 319ea54..189ac3f 100644
--- a/internal/refactor/inline/inline_test.go
+++ b/internal/refactor/inline/inline_test.go
@@ -391,6 +391,56 @@
})
}
+func TestDuplicable(t *testing.T) {
+ runTests(t, []testcase{
+ {
+ "Empty strings are duplicable.",
+ `func f(s string) { print(s, s) }`,
+ `func _() { f("") }`,
+ `func _() { print("", "") }`,
+ },
+ {
+ "Non-empty string literals are not duplicable.",
+ `func f(s string) { print(s, s) }`,
+ `func _() { f("hi") }`,
+ `func _() {
+ var s string = "hi"
+ print(s, s)
+}`,
+ },
+ {
+ "Empty array literals are duplicable.",
+ `func f(a [2]int) { print(a, a) }`,
+ `func _() { f([2]int{}) }`,
+ `func _() { print([2]int{}, [2]int{}) }`,
+ },
+ {
+ "Non-empty array literals are not duplicable.",
+ `func f(a [2]int) { print(a, a) }`,
+ `func _() { f([2]int{1, 2}) }`,
+ `func _() {
+ var a [2]int = [2]int{1, 2}
+ print(a, a)
+}`,
+ },
+ {
+ "Empty struct literals are duplicable.",
+ `func f(s S) { print(s, s) }; type S struct { x int }`,
+ `func _() { f(S{}) }`,
+ `func _() { print(S{}, S{}) }`,
+ },
+ {
+ "Non-empty struct literals are not duplicable.",
+ `func f(s S) { print(s, s) }; type S struct { x int }`,
+ `func _() { f(S{x: 1}) }`,
+ `func _() {
+ var s S = S{x: 1}
+ print(s, s)
+}`,
+ },
+ })
+}
+
func TestExprStmtReduction(t *testing.T) {
runTests(t, []testcase{
{
