cmd/cloner: support cloning arbitrarily-nested maps

Fixes #17870

Signed-off-by: Andrew Dunham <andrew@tailscale.com>

cmd/cloner/cloner.go

@@ -201,40 +201,23 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 				writef("\tdst.%s = maps.Clone(src.%s)", fname, fname)
 			} else {
 				// Otherwise we need to clone each element of
-				// the map.
+				// the map using our recursive helper.
 				writef("if dst.%s != nil {", fname)
 				writef("\tdst.%s = map[%s]%s{}", fname, it.QualifiedName(ft.Key()), it.QualifiedName(elem))
 				writef("\tfor k, v := range src.%s {", fname)
 
-				switch elem := elem.Underlying().(type) {
+				// Use a recursive helper here; this handles
-				case *types.Pointer:
+				// arbitrarily nested maps in addition to
-					writef("\t\tif v == nil { dst.%s[k] = nil } else {", fname)
+				// simpler types.
-					if base := elem.Elem().Underlying(); codegen.ContainsPointers(base) {
+				writeMapValueClone(mapValueCloneParams{
-						if _, isIface := base.(*types.Interface); isIface {
+					Buf:        buf,
-							it.Import("", "tailscale.com/types/ptr")
+					It:         it,
-							writef("\t\t\tdst.%s[k] = ptr.To((*v).Clone())", fname)
+					Elem:       elem,
-						} else {
+					SrcExpr:    "v",
-							writef("\t\t\tdst.%s[k] = v.Clone()", fname)
+					DstExpr:    fmt.Sprintf("dst.%s[k]", fname),
-						}
+					BaseIndent: "\t",
-					} else {
+					Depth:      1,
-						it.Import("", "tailscale.com/types/ptr")
+				})
-						writef("\t\t\tdst.%s[k] = ptr.To(*v)", fname)
-					}
-					writef("}")
-				case *types.Interface:
-					if cloneResultType := methodResultType(elem, "Clone"); cloneResultType != nil {
-						if _, isPtr := cloneResultType.(*types.Pointer); isPtr {
-							writef("\t\tdst.%s[k] = *(v.Clone())", fname)
-						} else {
-							writef("\t\tdst.%s[k] = v.Clone()", fname)
-						}
-					} else {
-						writef(`panic("%s (%v) does not have a Clone method")`, fname, elem)
-					}
-				default:
-					writef("\t\tdst.%s[k] = *(v.Clone())", fname)
-				}
-
 				writef("\t}")
 				writef("}")
 			}
@@ -277,3 +260,99 @@ func methodResultType(typ types.Type, method string) types.Type {
 	}
 	return sig.Results().At(0).Type()
 }
+
+type mapValueCloneParams struct {
+	// Buf is the buffer to write generated code to
+	Buf *bytes.Buffer
+	// It is the import tracker for managing imports.
+	It *codegen.ImportTracker
+	// Elem is the type of the map value to clone
+	Elem types.Type
+	// SrcExpr is the expression for the source value (e.g., "v", "v2", "v3")
+	SrcExpr string
+	// DstExpr is the expression for the destination (e.g., "dst.Field[k]", "dst.Field[k][k2]")
+	DstExpr string
+	// BaseIndent is the "base" indentation string for the generated code
+	// (i.e. 1 or more tabs). Additional indentation will be added based on
+	// the Depth parameter.
+	BaseIndent string
+	// Depth is the current nesting depth (1 for first level, 2 for second, etc.)
+	Depth int
+}
+
+// writeMapValueClone generates code to clone a map value recursively.
+// It handles arbitrary nesting of maps, pointers, and interfaces.
+func writeMapValueClone(params mapValueCloneParams) {
+	indent := params.BaseIndent + strings.Repeat("\t", params.Depth)
+	writef := func(format string, args ...any) {
+		fmt.Fprintf(params.Buf, indent+format+"\n", args...)
+	}
+
+	switch elem := params.Elem.Underlying().(type) {
+	case *types.Pointer:
+		writef("if %s == nil { %s = nil } else {", params.SrcExpr, params.DstExpr)
+		if base := elem.Elem().Underlying(); codegen.ContainsPointers(base) {
+			if _, isIface := base.(*types.Interface); isIface {
+				params.It.Import("", "tailscale.com/types/ptr")
+				writef("\t%s = ptr.To((*%s).Clone())", params.DstExpr, params.SrcExpr)
+			} else {
+				writef("\t%s = %s.Clone()", params.DstExpr, params.SrcExpr)
+			}
+		} else {
+			params.It.Import("", "tailscale.com/types/ptr")
+			writef("\t%s = ptr.To(*%s)", params.DstExpr, params.SrcExpr)
+		}
+		writef("}")
+
+	case *types.Map:
+		// Recursively handle nested maps
+		innerElem := elem.Elem()
+		if codegen.IsViewType(innerElem) || !codegen.ContainsPointers(innerElem) {
+			// Inner map values don't need deep cloning
+			params.It.Import("", "maps")
+			writef("%s = maps.Clone(%s)", params.DstExpr, params.SrcExpr)
+		} else {
+			// Inner map values need cloning
+			keyType := params.It.QualifiedName(elem.Key())
+			valueType := params.It.QualifiedName(innerElem)
+			// Generate unique variable names for nested loops based on depth
+			keyVar := fmt.Sprintf("k%d", params.Depth+1)
+			valVar := fmt.Sprintf("v%d", params.Depth+1)
+
+			writef("if %s == nil {", params.SrcExpr)
+			writef("\t%s = nil", params.DstExpr)
+			writef("\tcontinue")
+			writef("}")
+			writef("%s = map[%s]%s{}", params.DstExpr, keyType, valueType)
+			writef("for %s, %s := range %s {", keyVar, valVar, params.SrcExpr)
+
+			// Recursively generate cloning code for the nested map value
+			nestedDstExpr := fmt.Sprintf("%s[%s]", params.DstExpr, keyVar)
+			writeMapValueClone(mapValueCloneParams{
+				Buf:        params.Buf,
+				It:         params.It,
+				Elem:       innerElem,
+				SrcExpr:    valVar,
+				DstExpr:    nestedDstExpr,
+				BaseIndent: params.BaseIndent,
+				Depth:      params.Depth + 1,
+			})
+
+			writef("}")
+		}
+
+	case *types.Interface:
+		if cloneResultType := methodResultType(elem, "Clone"); cloneResultType != nil {
+			if _, isPtr := cloneResultType.(*types.Pointer); isPtr {
+				writef("%s = *(%s.Clone())", params.DstExpr, params.SrcExpr)
+			} else {
+				writef("%s = %s.Clone()", params.DstExpr, params.SrcExpr)
+			}
+		} else {
+			writef(`panic("map value (%%v) does not have a Clone method")`, elem)
+		}
+
+	default:
+		writef("%s = *(%s.Clone())", params.DstExpr, params.SrcExpr)
+	}
+}

cmd/cloner/cloner_test.go

@@ -108,3 +108,109 @@ func TestInterfaceContainer(t *testing.T) {
 		})
 	}
 }
+
+func TestMapWithPointers(t *testing.T) {
+	num1, num2 := 42, 100
+	orig := &clonerex.MapWithPointers{
+		Nested: map[string]*int{
+			"foo": &num1,
+			"bar": &num2,
+		},
+		WithCloneMethod: map[string]*clonerex.SliceContainer{
+			"container1": {Slice: []*int{&num1, &num2}},
+			"container2": {Slice: []*int{&num1}},
+		},
+		CloneInterface: map[string]clonerex.Cloneable{
+			"impl1": &clonerex.CloneableImpl{Value: 123},
+			"impl2": &clonerex.CloneableImpl{Value: 456},
+		},
+	}
+
+	cloned := orig.Clone()
+	if !reflect.DeepEqual(orig, cloned) {
+		t.Errorf("Clone() = %v, want %v", cloned, orig)
+	}
+
+	// Mutate cloned.Nested pointer values
+	*cloned.Nested["foo"] = 999
+	if *orig.Nested["foo"] == 999 {
+		t.Errorf("Clone() aliased memory in Nested: original was modified")
+	}
+
+	// Mutate cloned.WithCloneMethod slice values
+	*cloned.WithCloneMethod["container1"].Slice[0] = 888
+	if *orig.WithCloneMethod["container1"].Slice[0] == 888 {
+		t.Errorf("Clone() aliased memory in WithCloneMethod: original was modified")
+	}
+
+	// Mutate cloned.CloneInterface values
+	if impl, ok := cloned.CloneInterface["impl1"].(*clonerex.CloneableImpl); ok {
+		impl.Value = 777
+		if origImpl, ok := orig.CloneInterface["impl1"].(*clonerex.CloneableImpl); ok {
+			if origImpl.Value == 777 {
+				t.Errorf("Clone() aliased memory in CloneInterface: original was modified")
+			}
+		}
+	}
+}
+
+func TestDeeplyNestedMap(t *testing.T) {
+	num := 123
+	orig := &clonerex.DeeplyNestedMap{
+		ThreeLevels: map[string]map[string]map[string]int{
+			"a": {
+				"b": {"c": 1, "d": 2},
+				"e": {"f": 3},
+			},
+			"g": {
+				"h": {"i": 4},
+			},
+		},
+		FourLevels: map[string]map[string]map[string]map[string]*clonerex.SliceContainer{
+			"l1a": {
+				"l2a": {
+					"l3a": {
+						"l4a": {Slice: []*int{&num}},
+						"l4b": {Slice: []*int{&num, &num}},
+					},
+				},
+			},
+		},
+	}
+
+	cloned := orig.Clone()
+	if !reflect.DeepEqual(orig, cloned) {
+		t.Errorf("Clone() = %v, want %v", cloned, orig)
+	}
+
+	// Mutate the clone's ThreeLevels map
+	cloned.ThreeLevels["a"]["b"]["c"] = 777
+	if orig.ThreeLevels["a"]["b"]["c"] == 777 {
+		t.Errorf("Clone() aliased memory in ThreeLevels: original was modified")
+	}
+
+	// Mutate the clone's FourLevels map at the deepest pointer level
+	*cloned.FourLevels["l1a"]["l2a"]["l3a"]["l4a"].Slice[0] = 666
+	if *orig.FourLevels["l1a"]["l2a"]["l3a"]["l4a"].Slice[0] == 666 {
+		t.Errorf("Clone() aliased memory in FourLevels: original was modified")
+	}
+
+	// Add a new top-level key to the clone's FourLevels map
+	newNum := 999
+	cloned.FourLevels["l1b"] = map[string]map[string]map[string]*clonerex.SliceContainer{
+		"l2b": {
+			"l3b": {
+				"l4c": {Slice: []*int{&newNum}},
+			},
+		},
+	}
+	if _, exists := orig.FourLevels["l1b"]; exists {
+		t.Errorf("Clone() aliased FourLevels map: new top-level key appeared in original")
+	}
+
+	// Add a new nested key to the clone's FourLevels map
+	cloned.FourLevels["l1a"]["l2a"]["l3a"]["l4c"] = &clonerex.SliceContainer{Slice: []*int{&newNum}}
+	if _, exists := orig.FourLevels["l1a"]["l2a"]["l3a"]["l4c"]; exists {
+		t.Errorf("Clone() aliased FourLevels map: new nested key appeared in original")
+	}
+}

cmd/cloner/clonerex/clonerex.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type SliceContainer,InterfaceContainer
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap
 
 // Package clonerex is an example package for the cloner tool.
 package clonerex
@@ -32,3 +32,15 @@ func (c *CloneableImpl) Clone() Cloneable {
 type InterfaceContainer struct {
 	Interface Cloneable
 }
+
+type MapWithPointers struct {
+	Nested          map[string]*int
+	WithCloneMethod map[string]*SliceContainer
+	CloneInterface  map[string]Cloneable
+}
+
+// DeeplyNestedMap tests arbitrary depth of map nesting (3+ levels)
+type DeeplyNestedMap struct {
+	ThreeLevels map[string]map[string]map[string]int
+	FourLevels  map[string]map[string]map[string]map[string]*SliceContainer
+}

cmd/cloner/clonerex/clonerex_clone.go

@@ -6,6 +6,8 @@
 package clonerex
 
 import (
+	"maps"
+
 	"tailscale.com/types/ptr"
 )
 
@@ -54,9 +56,114 @@ var _InterfaceContainerCloneNeedsRegeneration = InterfaceContainer(struct {
 	Interface Cloneable
 }{})
 
+// Clone makes a deep copy of MapWithPointers.
+// The result aliases no memory with the original.
+func (src *MapWithPointers) Clone() *MapWithPointers {
+	if src == nil {
+		return nil
+	}
+	dst := new(MapWithPointers)
+	*dst = *src
+	if dst.Nested != nil {
+		dst.Nested = map[string]*int{}
+		for k, v := range src.Nested {
+			if v == nil {
+				dst.Nested[k] = nil
+			} else {
+				dst.Nested[k] = ptr.To(*v)
+			}
+		}
+	}
+	if dst.WithCloneMethod != nil {
+		dst.WithCloneMethod = map[string]*SliceContainer{}
+		for k, v := range src.WithCloneMethod {
+			if v == nil {
+				dst.WithCloneMethod[k] = nil
+			} else {
+				dst.WithCloneMethod[k] = v.Clone()
+			}
+		}
+	}
+	if dst.CloneInterface != nil {
+		dst.CloneInterface = map[string]Cloneable{}
+		for k, v := range src.CloneInterface {
+			dst.CloneInterface[k] = v.Clone()
+		}
+	}
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _MapWithPointersCloneNeedsRegeneration = MapWithPointers(struct {
+	Nested          map[string]*int
+	WithCloneMethod map[string]*SliceContainer
+	CloneInterface  map[string]Cloneable
+}{})
+
+// Clone makes a deep copy of DeeplyNestedMap.
+// The result aliases no memory with the original.
+func (src *DeeplyNestedMap) Clone() *DeeplyNestedMap {
+	if src == nil {
+		return nil
+	}
+	dst := new(DeeplyNestedMap)
+	*dst = *src
+	if dst.ThreeLevels != nil {
+		dst.ThreeLevels = map[string]map[string]map[string]int{}
+		for k, v := range src.ThreeLevels {
+			if v == nil {
+				dst.ThreeLevels[k] = nil
+				continue
+			}
+			dst.ThreeLevels[k] = map[string]map[string]int{}
+			for k2, v2 := range v {
+				dst.ThreeLevels[k][k2] = maps.Clone(v2)
+			}
+		}
+	}
+	if dst.FourLevels != nil {
+		dst.FourLevels = map[string]map[string]map[string]map[string]*SliceContainer{}
+		for k, v := range src.FourLevels {
+			if v == nil {
+				dst.FourLevels[k] = nil
+				continue
+			}
+			dst.FourLevels[k] = map[string]map[string]map[string]*SliceContainer{}
+			for k2, v2 := range v {
+				if v2 == nil {
+					dst.FourLevels[k][k2] = nil
+					continue
+				}
+				dst.FourLevels[k][k2] = map[string]map[string]*SliceContainer{}
+				for k3, v3 := range v2 {
+					if v3 == nil {
+						dst.FourLevels[k][k2][k3] = nil
+						continue
+					}
+					dst.FourLevels[k][k2][k3] = map[string]*SliceContainer{}
+					for k4, v4 := range v3 {
+						if v4 == nil {
+							dst.FourLevels[k][k2][k3][k4] = nil
+						} else {
+							dst.FourLevels[k][k2][k3][k4] = v4.Clone()
+						}
+					}
+				}
+			}
+		}
+	}
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _DeeplyNestedMapCloneNeedsRegeneration = DeeplyNestedMap(struct {
+	ThreeLevels map[string]map[string]map[string]int
+	FourLevels  map[string]map[string]map[string]map[string]*SliceContainer
+}{})
+
 // Clone duplicates src into dst and reports whether it succeeded.
 // To succeed, <src, dst> must be of types <*T, *T> or <*T, **T>,
-// where T is one of SliceContainer,InterfaceContainer.
+// where T is one of SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap.
 func Clone(dst, src any) bool {
 	switch src := src.(type) {
 	case *SliceContainer:
@@ -77,6 +184,24 @@ func Clone(dst, src any) bool {
 			*dst = src.Clone()
 			return true
 		}
+	case *MapWithPointers:
+		switch dst := dst.(type) {
+		case *MapWithPointers:
+			*dst = *src.Clone()
+			return true
+		case **MapWithPointers:
+			*dst = src.Clone()
+			return true
+		}
+	case *DeeplyNestedMap:
+		switch dst := dst.(type) {
+		case *DeeplyNestedMap:
+			*dst = *src.Clone()
+			return true
+		case **DeeplyNestedMap:
+			*dst = src.Clone()
+			return true
+		}
 	}
 	return false
 }