Turn first deepcopy parameter into a pointer

lavalamp · Jul 12, 2016 · 6049623 · 6049623
1 parent dbab1a8
commit 6049623
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 38 deletions.
diff --git a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go
@@ -425,7 +425,7 @@ func (g *genDeepCopy) GenerateType(c *generator.Context, t *types.Type, w io.Wri
 
 	sw := generator.NewSnippetWriter(w, c, "$", "$")
 	funcName := g.funcNameTmpl(t)
-	sw.Do(fmt.Sprintf("func %s(in $.type|raw$, out *$.type|raw$, c *$.Cloner|raw$) error {\n", funcName), g.withGlobals(argsFromType(t)))
+	sw.Do(fmt.Sprintf("func %s(in *$.type|raw$, out *$.type|raw$, c *$.Cloner|raw$) error {\n", funcName), g.withGlobals(argsFromType(t)))
 	g.generateFor(t, sw)
 	sw.Do("return nil\n", nil)
 	sw.Do("}\n\n", nil)
@@ -459,71 +459,71 @@ func (g *genDeepCopy) generateFor(t *types.Type, sw *generator.SnippetWriter) {
 }
 
 func (g *genDeepCopy) doBuiltin(t *types.Type, sw *generator.SnippetWriter) {
-	sw.Do("*out = in\n", nil)
+	sw.Do("*out = *in\n", nil)
 }
 
 func (g *genDeepCopy) doMap(t *types.Type, sw *generator.SnippetWriter) {
 	sw.Do("*out = make($.|raw$)\n", t)
 	if t.Key.IsAssignable() {
 		switch {
 		case hasDeepCopyMethod(t.Elem):
-			sw.Do("for key, val := range in {\n", nil)
+			sw.Do("for key, val := range *in {\n", nil)
 			sw.Do("(*out)[key] = val.DeepCopy()\n", nil)
 			sw.Do("}\n", nil)
 		case t.Elem.IsAnonymousStruct():
-			sw.Do("for key := range in {\n", nil)
+			sw.Do("for key := range *in {\n", nil)
 			sw.Do("(*out)[key] = struct{}{}\n", nil)
 			sw.Do("}\n", nil)
 		case t.Elem.IsAssignable():
-			sw.Do("for key, val := range in {\n", nil)
+			sw.Do("for key, val := range *in {\n", nil)
 			sw.Do("(*out)[key] = val\n", nil)
 			sw.Do("}\n", nil)
 		default:
-			sw.Do("for key, val := range in {\n", nil)
+			sw.Do("for key, val := range *in {\n", nil)
 			if g.copyableAndInBounds(t.Elem) {
 				sw.Do("newVal := new($.|raw$)\n", t.Elem)
 				funcName := g.funcNameTmpl(t.Elem)
-				sw.Do(fmt.Sprintf("if err := %s(val, newVal, c); err != nil {\n", funcName), argsFromType(t.Elem))
+				sw.Do(fmt.Sprintf("if err := %s(&val, newVal, c); err != nil {\n", funcName), argsFromType(t.Elem))
 				sw.Do("return err\n", nil)
 				sw.Do("}\n", nil)
 				sw.Do("(*out)[key] = *newVal\n", nil)
 			} else {
-				sw.Do("if newVal, err := c.DeepCopy(val); err != nil {\n", nil)
+				sw.Do("if newVal, err := c.DeepCopy(&val); err != nil {\n", nil)
 				sw.Do("return err\n", nil)
 				sw.Do("} else {\n", nil)
-				sw.Do("(*out)[key] = newVal.($.|raw$)\n", t.Elem)
+				sw.Do("(*out)[key] = *newVal.(*$.|raw$)\n", t.Elem)
 				sw.Do("}\n", nil)
 			}
 			sw.Do("}\n", nil)
 		}
 	} else {
 		// TODO: Implement it when necessary.
-		sw.Do("for range in {\n", nil)
+		sw.Do("for range *in {\n", nil)
 		sw.Do("// FIXME: Copying unassignable keys unsupported $.|raw$\n", t.Key)
 		sw.Do("}\n", nil)
 	}
 }
 
 func (g *genDeepCopy) doSlice(t *types.Type, sw *generator.SnippetWriter) {
-	sw.Do("*out = make($.|raw$, len(in))\n", t)
+	sw.Do("*out = make($.|raw$, len(*in))\n", t)
 	if t.Elem.Kind == types.Builtin {
-		sw.Do("copy(*out, in)\n", nil)
+		sw.Do("copy(*out, *in)\n", nil)
 	} else {
-		sw.Do("for i := range in {\n", nil)
+		sw.Do("for i := range *in {\n", nil)
 		if hasDeepCopyMethod(t.Elem) {
-			sw.Do("(*out)[i] = in[i].DeepCopy()\n", nil)
+			sw.Do("(*out)[i] = (*in)[i].DeepCopy()\n", nil)
 		} else if t.Elem.IsAssignable() {
-			sw.Do("(*out)[i] = in[i]\n", nil)
+			sw.Do("(*out)[i] = (*in)[i]\n", nil)
 		} else if g.copyableAndInBounds(t.Elem) {
 			funcName := g.funcNameTmpl(t.Elem)
-			sw.Do(fmt.Sprintf("if err := %s(in[i], &(*out)[i], c); err != nil {\n", funcName), argsFromType(t.Elem))
+			sw.Do(fmt.Sprintf("if err := %s(&(*in)[i], &(*out)[i], c); err != nil {\n", funcName), argsFromType(t.Elem))
 			sw.Do("return err\n", nil)
 			sw.Do("}\n", nil)
 		} else {
-			sw.Do("if newVal, err := c.DeepCopy(in[i]); err != nil {\n", nil)
+			sw.Do("if newVal, err := c.DeepCopy(&(*in)[i]); err != nil {\n", nil)
 			sw.Do("return err\n", nil)
 			sw.Do("} else {\n", nil)
-			sw.Do("(*out)[i] = newVal.($.|raw$)\n", t.Elem)
+			sw.Do("(*out)[i] = *newVal.(*$.|raw$)\n", t.Elem)
 			sw.Do("}\n", nil)
 		}
 		sw.Do("}\n", nil)
@@ -547,7 +547,7 @@ func (g *genDeepCopy) doStruct(t *types.Type, sw *generator.SnippetWriter) {
 			sw.Do("out.$.name$ = in.$.name$\n", args)
 		case types.Map, types.Slice, types.Pointer:
 			sw.Do("if in.$.name$ != nil {\n", args)
-			sw.Do("in, out := in.$.name$, &out.$.name$\n", args)
+			sw.Do("in, out := &in.$.name$, &out.$.name$\n", args)
 			g.generateFor(t, sw)
 			sw.Do("} else {\n", nil)
 			sw.Do("out.$.name$ = nil\n", args)
@@ -559,23 +559,23 @@ func (g *genDeepCopy) doStruct(t *types.Type, sw *generator.SnippetWriter) {
 				sw.Do("out.$.name$ = in.$.name$\n", args)
 			} else if g.copyableAndInBounds(t) {
 				funcName := g.funcNameTmpl(t)
-				sw.Do(fmt.Sprintf("if err := %s(in.$.name$, &out.$.name$, c); err != nil {\n", funcName), args)
+				sw.Do(fmt.Sprintf("if err := %s(&in.$.name$, &out.$.name$, c); err != nil {\n", funcName), args)
 				sw.Do("return err\n", nil)
 				sw.Do("}\n", nil)
 			} else {
-				sw.Do("if newVal, err := c.DeepCopy(in.$.name$); err != nil {\n", args)
+				sw.Do("if newVal, err := c.DeepCopy(&in.$.name$); err != nil {\n", args)
 				sw.Do("return err\n", nil)
 				sw.Do("} else {\n", nil)
-				sw.Do("out.$.name$ = newVal.($.type|raw$)\n", args)
+				sw.Do("out.$.name$ = *newVal.(*$.type|raw$)\n", args)
 				sw.Do("}\n", nil)
 			}
 		default:
 			sw.Do("if in.$.name$ == nil {\n", args)
 			sw.Do("out.$.name$ = nil\n", args)
-			sw.Do("} else if newVal, err := c.DeepCopy(in.$.name$); err != nil {\n", args)
+			sw.Do("} else if newVal, err := c.DeepCopy(&in.$.name$); err != nil {\n", args)
 			sw.Do("return err\n", nil)
 			sw.Do("} else {\n", nil)
-			sw.Do("out.$.name$ = newVal.($.type|raw$)\n", args)
+			sw.Do("out.$.name$ = *newVal.(*$.type|raw$)\n", args)
 			sw.Do("}\n", nil)
 		}
 	}
@@ -587,21 +587,23 @@ func (g *genDeepCopy) doInterface(t *types.Type, sw *generator.SnippetWriter) {
 }
 
 func (g *genDeepCopy) doPointer(t *types.Type, sw *generator.SnippetWriter) {
-	sw.Do("*out = new($.Elem|raw$)\n", t)
 	if hasDeepCopyMethod(t.Elem) {
-		sw.Do("**out = in.DeepCopy()\n", nil)
+		sw.Do("*out = new($.Elem|raw$)\n", t)
+		sw.Do("**out = (*in).DeepCopy()\n", nil)
 	} else if t.Elem.IsAssignable() {
-		sw.Do("**out = *in", nil)
+		sw.Do("*out = new($.Elem|raw$)\n", t)
+		sw.Do("**out = **in", nil)
 	} else if g.copyableAndInBounds(t.Elem) {
 		funcName := g.funcNameTmpl(t.Elem)
+		sw.Do("*out = new($.Elem|raw$)\n", t)
 		sw.Do(fmt.Sprintf("if err := %s(*in, *out, c); err != nil {\n", funcName), argsFromType(t.Elem))
 		sw.Do("return err\n", nil)
 		sw.Do("}\n", nil)
 	} else {
 		sw.Do("if newVal, err := c.DeepCopy(*in); err != nil {\n", nil)
 		sw.Do("return err\n", nil)
 		sw.Do("} else {\n", nil)
-		sw.Do("**out = newVal.($.|raw$)\n", t.Elem)
+		sw.Do("*out = newVal.($.|raw$)\n", t.Elem)
 		sw.Do("}\n", nil)
 	}
 }

diff --git a/pkg/conversion/cloner.go b/pkg/conversion/cloner.go
@@ -42,10 +42,10 @@ func NewCloner() *Cloner {
 }
 
 // Prevent recursing into every byte...
-func byteSliceDeepCopy(in []byte, out *[]byte, c *Cloner) error {
-	if in != nil {
-		*out = make([]byte, len(in))
-		copy(*out, in)
+func byteSliceDeepCopy(in *[]byte, out *[]byte, c *Cloner) error {
+	if *in != nil {
+		*out = make([]byte, len(*in))
+		copy(*out, *in)
 	} else {
 		*out = nil
 	}
@@ -63,10 +63,10 @@ func verifyDeepCopyFunctionSignature(ft reflect.Type) error {
 	if ft.NumOut() != 1 {
 		return fmt.Errorf("expected one 'out' param, got %v", ft)
 	}
-	if ft.In(1).Kind() != reflect.Ptr {
-		return fmt.Errorf("expected pointer arg for 'in' param 1, got: %v", ft)
+	if ft.In(0).Kind() != reflect.Ptr {
+		return fmt.Errorf("expected pointer arg for 'in' param 0, got: %v", ft)
 	}
-	if ft.In(1).Elem() != ft.In(0) {
+	if ft.In(1) != ft.In(0) {
 		return fmt.Errorf("expected 'in' param 0 the same as param 1, got: %v", ft)
 	}
 	var forClonerType Cloner
@@ -135,6 +135,13 @@ func (c *Cloner) DeepCopy(in interface{}) (interface{}, error) {
 func (c *Cloner) deepCopy(src reflect.Value) (reflect.Value, error) {
 	inType := src.Type()
 
+	switch src.Kind() {
+	case reflect.Interface, reflect.Ptr, reflect.Map, reflect.Slice:
+		if src.IsNil() {
+			return src, nil
+		}
+	}
+
 	if fv, ok := c.deepCopyFuncs[inType]; ok {
 		return c.customDeepCopy(src, fv)
 	}
@@ -145,15 +152,15 @@ func (c *Cloner) deepCopy(src reflect.Value) (reflect.Value, error) {
 }
 
 func (c *Cloner) customDeepCopy(src, fv reflect.Value) (reflect.Value, error) {
-	outValue := reflect.New(src.Type())
+	outValue := reflect.New(src.Type().Elem())
 	args := []reflect.Value{src, outValue, reflect.ValueOf(c)}
 	result := fv.Call(args)[0].Interface()
 	// This convolution is necessary because nil interfaces won't convert
 	// to error.
 	if result == nil {
-		return outValue.Elem(), nil
+		return outValue, nil
 	}
-	return outValue.Elem(), result.(error)
+	return outValue, result.(error)
 }
 
 func (c *Cloner) defaultDeepCopy(src reflect.Value) (reflect.Value, error) {