cmd/{cloner,viewer}: handle maps of views

Instead of trying to call View() on something that's already a View
type (or trying to Clone the view unnecessarily), we can re-use the
existing View values in a map[T]ViewType.

Fixes #17866

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

cmd/cloner/cloner.go

@@ -192,7 +192,16 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 				writef("\t\tdst.%s[k] = append([]%s{}, src.%s[k]...)", fname, n, fname)
 				writef("\t}")
 				writef("}")
-			} else if codegen.ContainsPointers(elem) {
+			} else if codegen.IsViewType(elem) || !codegen.ContainsPointers(elem) {
+				// If the map values are view types (which are
+				// immutable and don't need cloning) or don't
+				// themselves contain pointers, we can just
+				// clone the map itself.
+				it.Import("", "maps")
+				writef("\tdst.%s = maps.Clone(src.%s)", fname, fname)
+			} else {
+				// Otherwise we need to clone each element of
+				// the map.
 				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)
@@ -228,9 +237,6 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 
 				writef("\t}")
 				writef("}")
-			} else {
-				it.Import("", "maps")
-				writef("\tdst.%s = maps.Clone(src.%s)", fname, fname)
 			}
 		case *types.Interface:
 			// If ft is an interface with a "Clone() ft" method, it can be used to clone the field.

cmd/viewer/tests/tests.go

@@ -13,7 +13,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/viewer --type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct --clone-only-type=OnlyGetClone
+//go:generate go run tailscale.com/cmd/viewer --type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews --clone-only-type=OnlyGetClone
 
 type StructWithoutPtrs struct {
 	Int int
@@ -238,3 +238,7 @@ type GenericTypeAliasStruct[T integer, T2 views.ViewCloner[T2, V2], V2 views.Str
 	NonCloneable T
 	Cloneable    T2
 }
+
+type StructWithMapOfViews struct {
+	MapOfViews map[string]StructWithoutPtrsView
+}

cmd/viewer/tests/tests_clone.go

@@ -547,3 +547,20 @@ func _GenericTypeAliasStructCloneNeedsRegeneration[T integer, T2 views.ViewClone
 		Cloneable    T2
 	}{})
 }
+
+// Clone makes a deep copy of StructWithMapOfViews.
+// The result aliases no memory with the original.
+func (src *StructWithMapOfViews) Clone() *StructWithMapOfViews {
+	if src == nil {
+		return nil
+	}
+	dst := new(StructWithMapOfViews)
+	*dst = *src
+	dst.MapOfViews = maps.Clone(src.MapOfViews)
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithMapOfViewsCloneNeedsRegeneration = StructWithMapOfViews(struct {
+	MapOfViews map[string]StructWithoutPtrsView
+}{})

cmd/viewer/tests/tests_view.go

@@ -16,7 +16,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=false -type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=false -type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews
 
 // View returns a read-only view of StructWithPtrs.
 func (p *StructWithPtrs) View() StructWithPtrsView {
@@ -1053,3 +1053,79 @@ func _GenericTypeAliasStructViewNeedsRegeneration[T integer, T2 views.ViewCloner
 		Cloneable    T2
 	}{})
 }
+
+// View returns a read-only view of StructWithMapOfViews.
+func (p *StructWithMapOfViews) View() StructWithMapOfViewsView {
+	return StructWithMapOfViewsView{ж: p}
+}
+
+// StructWithMapOfViewsView provides a read-only view over StructWithMapOfViews.
+//
+// Its methods should only be called if `Valid()` returns true.
+type StructWithMapOfViewsView struct {
+	// ж is the underlying mutable value, named with a hard-to-type
+	// character that looks pointy like a pointer.
+	// It is named distinctively to make you think of how dangerous it is to escape
+	// to callers. You must not let callers be able to mutate it.
+	ж *StructWithMapOfViews
+}
+
+// Valid reports whether v's underlying value is non-nil.
+func (v StructWithMapOfViewsView) Valid() bool { return v.ж != nil }
+
+// AsStruct returns a clone of the underlying value which aliases no memory with
+// the original.
+func (v StructWithMapOfViewsView) AsStruct() *StructWithMapOfViews {
+	if v.ж == nil {
+		return nil
+	}
+	return v.ж.Clone()
+}
+
+// MarshalJSON implements [jsonv1.Marshaler].
+func (v StructWithMapOfViewsView) MarshalJSON() ([]byte, error) {
+	return jsonv1.Marshal(v.ж)
+}
+
+// MarshalJSONTo implements [jsonv2.MarshalerTo].
+func (v StructWithMapOfViewsView) MarshalJSONTo(enc *jsontext.Encoder) error {
+	return jsonv2.MarshalEncode(enc, v.ж)
+}
+
+// UnmarshalJSON implements [jsonv1.Unmarshaler].
+func (v *StructWithMapOfViewsView) UnmarshalJSON(b []byte) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	if len(b) == 0 {
+		return nil
+	}
+	var x StructWithMapOfViews
+	if err := jsonv1.Unmarshal(b, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom].
+func (v *StructWithMapOfViewsView) UnmarshalJSONFrom(dec *jsontext.Decoder) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	var x StructWithMapOfViews
+	if err := jsonv2.UnmarshalDecode(dec, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+func (v StructWithMapOfViewsView) MapOfViews() views.Map[string, StructWithoutPtrsView] {
+	return views.MapOf(v.ж.MapOfViews)
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithMapOfViewsViewNeedsRegeneration = StructWithMapOfViews(struct {
+	MapOfViews map[string]StructWithoutPtrsView
+}{})

cmd/viewer/viewer.go

@@ -367,14 +367,21 @@ func genView(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named, fie
 			case *types.Struct, *types.Named, *types.Alias:
 				strucT := u
 				args.FieldType = it.QualifiedName(fieldType)
-				if codegen.ContainsPointers(strucT) {
+
+				// We need to call View() unless the type is
+				// either a View itself or does not contain
+				// pointers (and can thus be shallow-copied).
+				//
+				// Otherwise, we need to create a View of the
+				// map value.
+				if codegen.IsViewType(strucT) || !codegen.ContainsPointers(strucT) {
+					template = "mapField"
+					args.MapValueType = it.QualifiedName(mElem)
+				} else {
 					args.MapFn = "t.View()"
 					template = "mapFnField"
 					args.MapValueType = it.QualifiedName(mElem)
 					args.MapValueView = appendNameSuffix(args.MapValueType, "View")
-				} else {
-					template = "mapField"
-					args.MapValueType = it.QualifiedName(mElem)
 				}
 			case *types.Basic:
 				template = "mapField"