From a59f84bd8482a597f7475c310d6388db2f6b22b5 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Fri, 6 Mar 2026 17:34:05 -0800 Subject: [PATCH 1/2] fix: Equal redundant Subset, Map lookup optimization, ContainsSeq vacuous truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Equal: remove redundant Subset(b,a) call — when cardinalities match, a single Subset(a,b) is sufficient to prove equality. - Map.Add/Remove: inline map lookup instead of calling Contains method, eliminating a redundant method call per operation. - ContainsSeq: return true for empty sequences (vacuous truth), matching mathematical convention and consistent with ContainsAll behavior. - CLAUDE.md: add Conventions section documenting project patterns. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 7 +++++++ map.go | 4 ++-- set.go | 8 +++----- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c98c380..a3c0b45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,13 @@ All types implement `json.Marshaler`/`json.Unmarshaler` and `sql.Scanner`. The ` When bumping the minimum Go version, the commit message should include `#minor` to trigger a minor version bump. +## Conventions + +- **Zero-value style**: Use `var x T` instead of `x := T{}` or `x := T's zero value`. +- **Tests are the spec**: When modifying implementations, do not change tests. Treat test failures as implementation bugs. +- **Mathematical correctness**: Prefer mathematically correct semantics (e.g., vacuous truth for empty predicates). +- **Commit messages**: Use conventional commits (`feat:`, `fix:`, `docs:`, etc.). + ## Testing Tests use property-based state machine testing via `pgregory.net/rapid`. The state machine in `set_test.go` validates invariants across all set implementations. Tests run in parallel. diff --git a/map.go b/map.go index 8322f3f..0e67274 100644 --- a/map.go +++ b/map.go @@ -59,7 +59,7 @@ func (s *Map[M]) Clear() int { // Add an element to the set. Returns true if the element was added, false if it was already present. func (s *Map[M]) Add(m M) bool { - if s.Contains(m) { + if _, ok := s.set[m]; ok { return false } s.set[m] = struct{}{} @@ -68,7 +68,7 @@ func (s *Map[M]) Add(m M) bool { // Remove an element from the set. Returns true if the element was removed, false if it was not present. func (s *Map[M]) Remove(m M) bool { - if !s.Contains(m) { + if _, ok := s.set[m]; !ok { return false } delete(s.set, m) diff --git a/set.go b/set.go index 75c5d4b..7016ccf 100644 --- a/set.go +++ b/set.go @@ -145,19 +145,17 @@ func Equal[K comparable](a, b Set[K]) bool { if a.Cardinality() != b.Cardinality() { return false } - return Subset(a, b) && Subset(b, a) + return Subset(a, b) } -// ContainsSeq returns true if the set contains all elements in the sequence. Empty sets are considered to contain only empty sequences. +// ContainsSeq returns true if the set contains all elements in the sequence. Returns true for an empty sequence (vacuous truth). func ContainsSeq[K comparable](s Set[K], seq iter.Seq[K]) bool { - noitems := true for k := range seq { - noitems = false if !s.Contains(k) { return false } } - return (s.Cardinality() == 0 && noitems) || !noitems + return true } // Disjoint returns true if the two sets have no elements in common. From 93acfa7c0ef3a3eab70b8d8b3ac2620af2f8a4e4 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Fri, 6 Mar 2026 18:17:42 -0800 Subject: [PATCH 2/2] fix: address Copilot review feedback on PR #30 - Add test for ContainsSeq(nonEmptySet, emptySeq) returning true to lock in vacuous truth semantics. - Equal: inline iteration instead of calling Subset to avoid redundant Cardinality() calls (significant for SyncMap where Cardinality is O(n)). - CLAUDE.md: clarify zero-value style wording per Copilot suggestion. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 +- examples_test.go | 5 +++++ set.go | 7 ++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a3c0b45..e99a365 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,7 @@ When bumping the minimum Go version, the commit message should include `#minor` ## Conventions -- **Zero-value style**: Use `var x T` instead of `x := T{}` or `x := T's zero value`. +- **Zero-value style**: Use `var x T` instead of `x := T{}`; refer to this as "T's zero value" in prose. - **Tests are the spec**: When modifying implementations, do not change tests. Treat test failures as implementation bugs. - **Mathematical correctness**: Prefer mathematically correct semantics (e.g., vacuous truth for empty predicates). - **Commit messages**: Use conventional commits (`feat:`, `fix:`, `docs:`, etc.). diff --git a/examples_test.go b/examples_test.go index f5031eb..afdf046 100644 --- a/examples_test.go +++ b/examples_test.go @@ -312,6 +312,10 @@ func ExampleContainsSeq() { ints.Add(3) ints.Add(2) + if ContainsSeq(ints, slices.Values([]int{})) { + fmt.Println("Non-empty set contains empty sequence") + } + if ContainsSeq(ints, slices.Values([]int{3, 5})) { fmt.Println("3 and 5 are present") } @@ -321,6 +325,7 @@ func ExampleContainsSeq() { } // Output: // Empty set contains empty sequence + // Non-empty set contains empty sequence // 3 and 5 are present // 6 is not present } diff --git a/set.go b/set.go index 7016ccf..2a0fbae 100644 --- a/set.go +++ b/set.go @@ -145,7 +145,12 @@ func Equal[K comparable](a, b Set[K]) bool { if a.Cardinality() != b.Cardinality() { return false } - return Subset(a, b) + for k := range a.Iterator { + if !b.Contains(k) { + return false + } + } + return true } // ContainsSeq returns true if the set contains all elements in the sequence. Returns true for an empty sequence (vacuous truth).