From 8b5010f57188686f953d28c6de4e48f4f03184df Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 5 Apr 2017 13:48:37 +0200
Subject: [PATCH] use a temp variable for loop conversions on function calls

This avoids calling the function more than once.
---
 genmethod.go                       | 32 +++++++++++++++++++++++++-----
 internal/tests/mapconv/output.go   | 21 +++++++++++---------
 internal/tests/sliceconv/output.go | 21 +++++++++++---------
 3 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/genmethod.go b/genmethod.go
index f28d44c..7e1c671 100644
--- a/genmethod.go
+++ b/genmethod.go
@@ -286,8 +286,14 @@ func sliceKV(typ types.Type) kvType {
 	return kvType{typ, intType, slicetyp.Elem()}
 }
 
-func (m *marshalMethod) loopConv(from, to Expression, fromTyp, toTyp kvType) []Statement {
-	conv := []Statement{
+func (m *marshalMethod) loopConv(from, to Expression, fromTyp, toTyp kvType) (conv []Statement) {
+	if hasSideEffects(from) {
+		orig := from
+		from = Name(m.scope.newIdent("tmp"))
+		conv = []Statement{DeclareAndAssign{Lhs: from, Rhs: orig}}
+	}
+	// The actual conversion is a loop that assigns each element.
+	inner := []Statement{
 		Assign{Lhs: to, Rhs: makeExpr(toTyp.Type, from, m.scope.parent.qualify)},
 		Range{
 			Key:        m.iterKey,
@@ -302,12 +308,12 @@ func (m *marshalMethod) loopConv(from, to Expression, fromTyp, toTyp kvType) []S
 	// Preserve nil maps and slices when marshaling. This is not required for unmarshaling
 	// methods because the field is already nil-checked earlier.
 	if !m.isUnmarshal {
-		conv = []Statement{If{
+		inner = []Statement{If{
 			Condition: NotEqual{Lhs: from, Rhs: NIL},
-			Body:      conv,
+			Body:      inner,
 		}}
 	}
-	return conv
+	return append(conv, inner...)
 }
 
 func simpleConv(from Expression, fromtyp, totyp types.Type, qf types.Qualifier) Expression {
@@ -344,6 +350,22 @@ func errCheck(expr Expression) If {
 	}
 }
 
+// hasSideEffects returns whether an expression may have side effects.
+func hasSideEffects(expr Expression) bool {
+	switch expr := expr.(type) {
+	case Var:
+		return false
+	case Dotted:
+		return hasSideEffects(expr.Receiver)
+	case Star:
+		return hasSideEffects(expr.Value)
+	case Index:
+		return hasSideEffects(expr.Index) && hasSideEffects(expr.Value)
+	default:
+		return true
+	}
+}
+
 type stringLit struct {
 	V string
 }
diff --git a/internal/tests/mapconv/output.go b/internal/tests/mapconv/output.go
index 4593155..17c7105 100644
--- a/internal/tests/mapconv/output.go
+++ b/internal/tests/mapconv/output.go
@@ -29,9 +29,10 @@ func (x X) MarshalJSON() ([]byte, error) {
 	}
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make(map[replacedString]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make(map[replacedString]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[replacedString(k)] = replacedInt(v)
 		}
 	}
@@ -93,9 +94,10 @@ func (x X) MarshalYAML() (interface{}, error) {
 	}
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make(map[replacedString]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make(map[replacedString]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[replacedString(k)] = replacedInt(v)
 		}
 	}
@@ -157,9 +159,10 @@ func (x X) MarshalTOML() (interface{}, error) {
 	}
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make(map[replacedString]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make(map[replacedString]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[replacedString(k)] = replacedInt(v)
 		}
 	}
diff --git a/internal/tests/sliceconv/output.go b/internal/tests/sliceconv/output.go
index fd7d80a..c09ffe5 100644
--- a/internal/tests/sliceconv/output.go
+++ b/internal/tests/sliceconv/output.go
@@ -31,9 +31,10 @@ func (x X) MarshalJSON() ([]byte, error) {
 	enc.ByteString = []byte(x.ByteString)
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make([]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make([]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[k] = replacedInt(v)
 		}
 	}
@@ -101,9 +102,10 @@ func (x X) MarshalYAML() (interface{}, error) {
 	enc.ByteString = []byte(x.ByteString)
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make([]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make([]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[k] = replacedInt(v)
 		}
 	}
@@ -171,9 +173,10 @@ func (x X) MarshalTOML() (interface{}, error) {
 	enc.ByteString = []byte(x.ByteString)
 	enc.NoConv = x.NoConv
 	enc.NoConvNamed = x.NoConvNamed
-	if x.Func() != nil {
-		enc.Func = make([]replacedInt, len(x.Func()))
-		for k, v := range x.Func() {
+	tmp := x.Func()
+	if tmp != nil {
+		enc.Func = make([]replacedInt, len(tmp))
+		for k, v := range tmp {
 			enc.Func[k] = replacedInt(v)
 		}
 	}