From 57b0f7aa385b631c0be4702693a460cb93421702 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Thu, 2 Mar 2023 14:37:04 +0100 Subject: [PATCH] Add a Clear() function to generic sets This is useful when a given set is shared through pointers, and therefore can't be replaced with a new empty set. This replaces set.Delete(set.UnsortedList()...), and allows the compiler to optimize the function to a call to runtime.mapclear() when the set content type is reflexive for ==. That optimization *is* currently accessible using s := Set[...]{} ... for i := range s { delete(s, i) } but this circumvents the published API for sets; calling s.Delete(i) instead can't be optimized in this fashion. Alternatives considered but discarded include: * turning sets into a pointer type (this isn't possible because pointer types can't be receivers) * using a pointer receiver for the Clear() function, i.e. func (s *Set[T]) Clear() { *s = New[T]() } but this doesn't update shared references. Signed-off-by: Stephen Kitt --- .../k8s.io/apimachinery/pkg/util/sets/set.go | 14 +++++ .../pkg/util/sets/set_generic_test.go | 51 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/sets/set.go b/staging/src/k8s.io/apimachinery/pkg/util/sets/set.go index 99c292feda1..d50526f4262 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/sets/set.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/sets/set.go @@ -64,6 +64,20 @@ func (s Set[T]) Delete(items ...T) Set[T] { return s } +// Clear empties the set. +// It is preferable to replace the set with a newly constructed set, +// but not all callers can do that (when there are other references to the map). +// In some cases the set *won't* be fully cleared, e.g. a Set[float32] containing NaN +// can't be cleared because NaN can't be removed. +// For sets containing items of a type that is reflexive for ==, +// this is optimized to a single call to runtime.mapclear(). +func (s Set[T]) Clear() Set[T] { + for key := range s { + delete(s, key) + } + return s +} + // Has returns true if and only if item is contained in the set. func (s Set[T]) Has(item T) bool { _, contained := s[item] diff --git a/staging/src/k8s.io/apimachinery/pkg/util/sets/set_generic_test.go b/staging/src/k8s.io/apimachinery/pkg/util/sets/set_generic_test.go index a86cbb1743c..53f3383def7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/sets/set_generic_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/sets/set_generic_test.go @@ -84,6 +84,57 @@ func TestSetDeleteMultiples(t *testing.T) { } +func TestSetClear(t *testing.T) { + s := sets.Set[string]{} + s.Insert("a", "b", "c") + if s.Len() != 3 { + t.Errorf("Expected len=3: %d", s.Len()) + } + + s.Clear() + if s.Len() != 0 { + t.Errorf("Expected len=0: %d", s.Len()) + } +} + +func TestSetClearWithSharedReference(t *testing.T) { + s := sets.Set[string]{} + s.Insert("a", "b", "c") + if s.Len() != 3 { + t.Errorf("Expected len=3: %d", s.Len()) + } + + m := s + s.Clear() + if s.Len() != 0 { + t.Errorf("Expected len=0 on the cleared set: %d", s.Len()) + } + if m.Len() != 0 { + t.Errorf("Expected len=0 on the shared reference: %d", m.Len()) + } +} + +func TestSetClearInSeparateFunction(t *testing.T) { + s := sets.Set[string]{} + s.Insert("a", "b", "c") + if s.Len() != 3 { + t.Errorf("Expected len=3: %d", s.Len()) + } + + clearSetAndAdd(s, "d") + if s.Len() != 1 { + t.Errorf("Expected len=1: %d", s.Len()) + } + if !s.Has("d") { + t.Errorf("Unexpected contents: %#v", s) + } +} + +func clearSetAndAdd[T comparable](s sets.Set[T], a T) { + s.Clear() + s.Insert(a) +} + func TestNewSet(t *testing.T) { s := sets.New("a", "b", "c") if len(s) != 3 {