cmd/cloner,*: revert: optimize nillable slice cloner

This reverts commit ee90cd02fd.

The outcome is not identical for empty slices. Cloner really needs
tests!

Updates #9601

Signed-off-by: James Tucker <james@tailscale.com>

cmd/cloner/cloner.go

@@ -122,7 +122,8 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 		case *types.Slice:
 			if codegen.ContainsPointers(ft.Elem()) {
 				n := it.QualifiedName(ft.Elem())
-				writef("dst.%s = append([]%s(nil), make([]%s, len(src.%s))...)", fname, n, n, fname)
+				writef("if src.%s != nil {", fname)
+				writef("dst.%s = make([]%s, len(src.%s))", fname, n, fname)
 				writef("for i := range dst.%s {", fname)
 				if ptr, isPtr := ft.Elem().(*types.Pointer); isPtr {
 					if _, isBasic := ptr.Elem().Underlying().(*types.Basic); isBasic {
@@ -137,6 +138,7 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 					writef("\tdst.%s[i] = *src.%s[i].Clone()", fname, fname)
 				}
 				writef("}")
+				writef("}")
 			} else {
 				writef("dst.%s = append(src.%s[:0:0], src.%s...)", fname, fname, fname)
 			}

cmd/viewer/tests/tests_clone.go

@@ -136,21 +136,29 @@ func (src *StructWithSlices) Clone() *StructWithSlices {
 	dst := new(StructWithSlices)
 	*dst = *src
 	dst.Values = append(src.Values[:0:0], src.Values...)
-	dst.ValuePointers = append([]*StructWithoutPtrs(nil), make([]*StructWithoutPtrs, len(src.ValuePointers))...)
+	if src.ValuePointers != nil {
-	for i := range dst.ValuePointers {
+		dst.ValuePointers = make([]*StructWithoutPtrs, len(src.ValuePointers))
-		dst.ValuePointers[i] = src.ValuePointers[i].Clone()
+		for i := range dst.ValuePointers {
+			dst.ValuePointers[i] = src.ValuePointers[i].Clone()
+		}
 	}
-	dst.StructPointers = append([]*StructWithPtrs(nil), make([]*StructWithPtrs, len(src.StructPointers))...)
+	if src.StructPointers != nil {
-	for i := range dst.StructPointers {
+		dst.StructPointers = make([]*StructWithPtrs, len(src.StructPointers))
-		dst.StructPointers[i] = src.StructPointers[i].Clone()
+		for i := range dst.StructPointers {
+			dst.StructPointers[i] = src.StructPointers[i].Clone()
+		}
 	}
-	dst.Structs = append([]StructWithPtrs(nil), make([]StructWithPtrs, len(src.Structs))...)
+	if src.Structs != nil {
-	for i := range dst.Structs {
+		dst.Structs = make([]StructWithPtrs, len(src.Structs))
-		dst.Structs[i] = *src.Structs[i].Clone()
+		for i := range dst.Structs {
+			dst.Structs[i] = *src.Structs[i].Clone()
+		}
 	}
-	dst.Ints = append([]*int(nil), make([]*int, len(src.Ints))...)
+	if src.Ints != nil {
-	for i := range dst.Ints {
+		dst.Ints = make([]*int, len(src.Ints))
-		dst.Ints[i] = ptr.To(*src.Ints[i])
+		for i := range dst.Ints {
+			dst.Ints[i] = ptr.To(*src.Ints[i])
+		}
 	}
 	dst.Slice = append(src.Slice[:0:0], src.Slice...)
 	dst.Prefixes = append(src.Prefixes[:0:0], src.Prefixes...)

tailcfg/tailcfg_clone.go

@@ -74,9 +74,11 @@ func (src *Node) Clone() *Node {
 	if dst.SelfNodeV6MasqAddrForThisPeer != nil {
 		dst.SelfNodeV6MasqAddrForThisPeer = ptr.To(*src.SelfNodeV6MasqAddrForThisPeer)
 	}
-	dst.ExitNodeDNSResolvers = append([]*dnstype.Resolver(nil), make([]*dnstype.Resolver, len(src.ExitNodeDNSResolvers))...)
+	if src.ExitNodeDNSResolvers != nil {
-	for i := range dst.ExitNodeDNSResolvers {
+		dst.ExitNodeDNSResolvers = make([]*dnstype.Resolver, len(src.ExitNodeDNSResolvers))
-		dst.ExitNodeDNSResolvers[i] = src.ExitNodeDNSResolvers[i].Clone()
+		for i := range dst.ExitNodeDNSResolvers {
+			dst.ExitNodeDNSResolvers[i] = src.ExitNodeDNSResolvers[i].Clone()
+		}
 	}
 	return dst
 }
@@ -235,9 +237,11 @@ func (src *DNSConfig) Clone() *DNSConfig {
 	}
 	dst := new(DNSConfig)
 	*dst = *src
-	dst.Resolvers = append([]*dnstype.Resolver(nil), make([]*dnstype.Resolver, len(src.Resolvers))...)
+	if src.Resolvers != nil {
-	for i := range dst.Resolvers {
+		dst.Resolvers = make([]*dnstype.Resolver, len(src.Resolvers))
-		dst.Resolvers[i] = src.Resolvers[i].Clone()
+		for i := range dst.Resolvers {
+			dst.Resolvers[i] = src.Resolvers[i].Clone()
+		}
 	}
 	if dst.Routes != nil {
 		dst.Routes = map[string][]*dnstype.Resolver{}
@@ -245,9 +249,11 @@ func (src *DNSConfig) Clone() *DNSConfig {
 			dst.Routes[k] = append([]*dnstype.Resolver{}, src.Routes[k]...)
 		}
 	}
-	dst.FallbackResolvers = append([]*dnstype.Resolver(nil), make([]*dnstype.Resolver, len(src.FallbackResolvers))...)
+	if src.FallbackResolvers != nil {
-	for i := range dst.FallbackResolvers {
+		dst.FallbackResolvers = make([]*dnstype.Resolver, len(src.FallbackResolvers))
-		dst.FallbackResolvers[i] = src.FallbackResolvers[i].Clone()
+		for i := range dst.FallbackResolvers {
+			dst.FallbackResolvers[i] = src.FallbackResolvers[i].Clone()
+		}
 	}
 	dst.Domains = append(src.Domains[:0:0], src.Domains...)
 	dst.Nameservers = append(src.Nameservers[:0:0], src.Nameservers...)
@@ -381,9 +387,11 @@ func (src *DERPRegion) Clone() *DERPRegion {
 	}
 	dst := new(DERPRegion)
 	*dst = *src
-	dst.Nodes = append([]*DERPNode(nil), make([]*DERPNode, len(src.Nodes))...)
+	if src.Nodes != nil {
-	for i := range dst.Nodes {
+		dst.Nodes = make([]*DERPNode, len(src.Nodes))
-		dst.Nodes[i] = src.Nodes[i].Clone()
+		for i := range dst.Nodes {
+			dst.Nodes[i] = src.Nodes[i].Clone()
+		}
 	}
 	return dst
 }
@@ -460,9 +468,11 @@ func (src *SSHRule) Clone() *SSHRule {
 	if dst.RuleExpires != nil {
 		dst.RuleExpires = ptr.To(*src.RuleExpires)
 	}
-	dst.Principals = append([]*SSHPrincipal(nil), make([]*SSHPrincipal, len(src.Principals))...)
+	if src.Principals != nil {
-	for i := range dst.Principals {
+		dst.Principals = make([]*SSHPrincipal, len(src.Principals))
-		dst.Principals[i] = src.Principals[i].Clone()
+		for i := range dst.Principals {
+			dst.Principals[i] = src.Principals[i].Clone()
+		}
 	}
 	dst.SSHUsers = maps.Clone(src.SSHUsers)
 	dst.Action = src.Action.Clone()

wgengine/filter/filter_clone.go

@@ -23,9 +23,11 @@ func (src *Match) Clone() *Match {
 	dst.IPProto = append(src.IPProto[:0:0], src.IPProto...)
 	dst.Srcs = append(src.Srcs[:0:0], src.Srcs...)
 	dst.Dsts = append(src.Dsts[:0:0], src.Dsts...)
-	dst.Caps = append([]CapMatch(nil), make([]CapMatch, len(src.Caps))...)
+	if src.Caps != nil {
-	for i := range dst.Caps {
+		dst.Caps = make([]CapMatch, len(src.Caps))
-		dst.Caps[i] = *src.Caps[i].Clone()
+		for i := range dst.Caps {
+			dst.Caps[i] = *src.Caps[i].Clone()
+		}
 	}
 	return dst
 }

wgengine/wgcfg/wgcfg_clone.go

@@ -24,9 +24,11 @@ func (src *Config) Clone() *Config {
 	*dst = *src
 	dst.Addresses = append(src.Addresses[:0:0], src.Addresses...)
 	dst.DNS = append(src.DNS[:0:0], src.DNS...)
-	dst.Peers = append([]Peer(nil), make([]Peer, len(src.Peers))...)
+	if src.Peers != nil {
-	for i := range dst.Peers {
+		dst.Peers = make([]Peer, len(src.Peers))
-		dst.Peers[i] = *src.Peers[i].Clone()
+		for i := range dst.Peers {
+			dst.Peers[i] = *src.Peers[i].Clone()
+		}
 	}
 	return dst
 }