From ab810f1f6d6fcb57068738b2212ff101cc3ac1c5 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 29 Sep 2023 18:31:45 -0700 Subject: [PATCH] cmd/cloner: add regression test for slice nil/empty semantics We had a misstep with the semantics when applying an optimization that showed up in the roll into corp. This test ensures that case and related cases must be retained. Updates #9410 Updates #9601 Signed-off-by: James Tucker --- cmd/cloner/cloner.go | 2 + cmd/cloner/cloner_test.go | 60 +++++++++++++++++++++++++++ cmd/cloner/clonerex/clonerex.go | 10 +++++ cmd/cloner/clonerex/clonerex_clone.go | 54 ++++++++++++++++++++++++ cmd/viewer/tests/tests_clone.go | 6 ++- 5 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 cmd/cloner/cloner_test.go create mode 100644 cmd/cloner/clonerex/clonerex.go create mode 100644 cmd/cloner/clonerex/clonerex_clone.go diff --git a/cmd/cloner/cloner.go b/cmd/cloner/cloner.go index 5a94fa97d..8e2944115 100644 --- a/cmd/cloner/cloner.go +++ b/cmd/cloner/cloner.go @@ -128,7 +128,9 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) { if ptr, isPtr := ft.Elem().(*types.Pointer); isPtr { if _, isBasic := ptr.Elem().Underlying().(*types.Basic); isBasic { it.Import("tailscale.com/types/ptr") + writef("if src.%s[i] == nil { dst.%s[i] = nil } else {", fname, fname) writef("\tdst.%s[i] = ptr.To(*src.%s[i])", fname, fname) + writef("}") } else { writef("\tdst.%s[i] = src.%s[i].Clone()", fname, fname) } diff --git a/cmd/cloner/cloner_test.go b/cmd/cloner/cloner_test.go new file mode 100644 index 000000000..43384fa48 --- /dev/null +++ b/cmd/cloner/cloner_test.go @@ -0,0 +1,60 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause +package main + +import ( + "reflect" + "testing" + + "tailscale.com/cmd/cloner/clonerex" +) + +func TestSliceContainer(t *testing.T) { + num := 5 + examples := []struct { + name string + in *clonerex.SliceContianer + }{ + { + name: "nil", + in: nil, + }, + { + name: "zero", + in: &clonerex.SliceContianer{}, + }, + { + name: "empty", + in: &clonerex.SliceContianer{ + Slice: []*int{}, + }, + }, + { + name: "nils", + in: &clonerex.SliceContianer{ + Slice: []*int{nil, nil, nil, nil, nil}, + }, + }, + { + name: "one", + in: &clonerex.SliceContianer{ + Slice: []*int{&num}, + }, + }, + { + name: "several", + in: &clonerex.SliceContianer{ + Slice: []*int{&num, &num, &num, &num, &num}, + }, + }, + } + + for _, ex := range examples { + t.Run(ex.name, func(t *testing.T) { + out := ex.in.Clone() + if !reflect.DeepEqual(ex.in, out) { + t.Errorf("Clone() = %v, want %v", out, ex.in) + } + }) + } +} diff --git a/cmd/cloner/clonerex/clonerex.go b/cmd/cloner/clonerex/clonerex.go new file mode 100644 index 000000000..2858e0a24 --- /dev/null +++ b/cmd/cloner/clonerex/clonerex.go @@ -0,0 +1,10 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:generate go run tailscale.com/cmd/cloner -clonefunc=true -type SliceContianer + +package clonerex + +type SliceContianer struct { + Slice []*int +} diff --git a/cmd/cloner/clonerex/clonerex_clone.go b/cmd/cloner/clonerex/clonerex_clone.go new file mode 100644 index 000000000..16bb6d611 --- /dev/null +++ b/cmd/cloner/clonerex/clonerex_clone.go @@ -0,0 +1,54 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by tailscale.com/cmd/cloner; DO NOT EDIT. + +package clonerex + +import ( + "tailscale.com/types/ptr" +) + +// Clone makes a deep copy of SliceContianer. +// The result aliases no memory with the original. +func (src *SliceContianer) Clone() *SliceContianer { + if src == nil { + return nil + } + dst := new(SliceContianer) + *dst = *src + if src.Slice != nil { + dst.Slice = make([]*int, len(src.Slice)) + for i := range dst.Slice { + if src.Slice[i] == nil { + dst.Slice[i] = nil + } else { + dst.Slice[i] = ptr.To(*src.Slice[i]) + } + } + } + return dst +} + +// A compilation failure here means this code must be regenerated, with the command at the top of this file. +var _SliceContianerCloneNeedsRegeneration = SliceContianer(struct { + Slice []*int +}{}) + +// Clone duplicates src into dst and reports whether it succeeded. +// To succeed, must be of types <*T, *T> or <*T, **T>, +// where T is one of SliceContianer. +func Clone(dst, src any) bool { + switch src := src.(type) { + case *SliceContianer: + switch dst := dst.(type) { + case *SliceContianer: + *dst = *src.Clone() + return true + case **SliceContianer: + *dst = src.Clone() + return true + } + } + return false +} diff --git a/cmd/viewer/tests/tests_clone.go b/cmd/viewer/tests/tests_clone.go index 2b41639fd..2d8c1ba31 100644 --- a/cmd/viewer/tests/tests_clone.go +++ b/cmd/viewer/tests/tests_clone.go @@ -157,7 +157,11 @@ func (src *StructWithSlices) Clone() *StructWithSlices { if src.Ints != nil { dst.Ints = make([]*int, len(src.Ints)) for i := range dst.Ints { - dst.Ints[i] = ptr.To(*src.Ints[i]) + if src.Ints[i] == nil { + dst.Ints[i] = nil + } else { + dst.Ints[i] = ptr.To(*src.Ints[i]) + } } } dst.Slice = append(src.Slice[:0:0], src.Slice...)