From 9171e73d551109be7afcc5914cab31f06fbe60b8 Mon Sep 17 00:00:00 2001
From: Moritz Hoffmann <antiguru@gmail.com>
Date: Thu, 23 May 2024 09:48:34 -0400
Subject: [PATCH] Semigroup generic over Rhs (#493)

Adds support for implementing semigroup for types that can plus_equals
from another type, for example vectors and slices.

Fixes a bug where the semigroup implementation for `Vec` would add
overhanging elements twice, i.e., `[1,2] + [1,1,1]` would result in
`[2,3,2`] instead of `[2,3,1]`.

This leaves one quirk where `is_zero` does not depend on `Rhs`, so Rust
cannot figure out which implementation to use when two are in scope,
forcing the caller to phrase it as `<R as Semigroup>::is_zero(&value)`.
This only applies if `R` implements `Semigroup + Semigroup<Other>`.

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
---
 src/difference.rs | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/difference.rs b/src/difference.rs
index fc6c8680b..46c7ba3eb 100644
--- a/src/difference.rs
+++ b/src/difference.rs
@@ -22,9 +22,9 @@ pub use self::Abelian as Diff;
 /// There is a light presumption of commutativity here, in that while we will largely perform addition
 /// in order of timestamps, for many types of timestamps there is no total order and consequently no
 /// obvious order to respect. Non-commutative semigroups should be used with care.
-pub trait Semigroup : ::std::marker::Sized + Data + Clone {
+pub trait Semigroup<Rhs: ?Sized = Self> : Data + Clone {
     /// The method of `std::ops::AddAssign`, for types that do not implement `AddAssign`.
-    fn plus_equals(&mut self, rhs: &Self);
+    fn plus_equals(&mut self, rhs: &Rhs);
     /// Returns true if the element is the additive identity.
     ///
     /// This is primarily used by differential dataflow to know when it is safe to delete an update.
@@ -233,22 +233,43 @@ mod vector {
 
     impl<R: Semigroup> Semigroup for Vec<R> {
         fn plus_equals(&mut self, rhs: &Self) {
-            // Ensure sufficient length to receive addition.
+            self.plus_equals(&rhs[..])
+        }
+        fn is_zero(&self) -> bool {
+            self.iter().all(|x| x.is_zero())
+        }
+    }
+
+    impl<R: Semigroup> Semigroup<[R]> for Vec<R> {
+        fn plus_equals(&mut self, rhs: &[R]) {
+            // Apply all updates to existing elements
+            for (index, update) in rhs.iter().enumerate().take(self.len()) {
+                self[index].plus_equals(update);
+            }
+
+            // Clone leftover elements from `rhs`
             while self.len() < rhs.len() {
                 let element = &rhs[self.len()];
                 self.push(element.clone());
             }
-
-            // As other is not longer, apply updates without tests.
-            for (index, update) in rhs.iter().enumerate() {
-                self[index].plus_equals(update);
-            }
         }
         fn is_zero(&self) -> bool {
             self.iter().all(|x| x.is_zero())
         }
     }
 
+    #[cfg(test)]
+    mod tests {
+        use crate::difference::Semigroup;
+
+        #[test]
+        fn test_semigroup_vec() {
+            let mut a = vec![1,2,3];
+            a.plus_equals([1,1,1,1].as_slice());
+            assert_eq!(vec![2,3,4,1], a);
+        }
+    }
+
     impl<R: Monoid> Monoid for Vec<R> {
         fn zero() -> Self {
             Self::new()