From 7d6b218899dfd9d5a12e3f5621e675e29b8292cf Mon Sep 17 00:00:00 2001
From: Moritz Hoffmann <antiguru@gmail.com>
Date: Mon, 26 Feb 2024 22:37:35 -0500
Subject: [PATCH] Remove CopyOnto requirement for ReadItem

Remove the requirement that `Region::ReadItem` implements `CopyOnto<Self>`
because it infects the region with the type bounds of the specific
implementation without adding much value. It still is a good idea that
implementations offer this functionality, but it seems overly restrictive
to enforce this property for all implementations.

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
---
 src/impls/codec.rs       |  1 -
 src/impls/columns.rs     |  3 ++-
 src/impls/deduplicate.rs |  6 +-----
 src/impls/string.rs      | 12 +++++++-----
 src/lib.rs               |  9 ++++++---
 5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/impls/codec.rs b/src/impls/codec.rs
index 0e3d0d3..ba73961 100644
--- a/src/impls/codec.rs
+++ b/src/impls/codec.rs
@@ -80,7 +80,6 @@ pub struct CodecRegion<C: Codec, R = CopyRegion<u8>> {
 impl<C: Codec, R> Region for CodecRegion<C, R>
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R>,
 {
     type ReadItem<'a> = &'a [u8]
     where
diff --git a/src/impls/columns.rs b/src/impls/columns.rs
index 703faf3..02c815b 100644
--- a/src/impls/columns.rs
+++ b/src/impls/columns.rs
@@ -238,9 +238,10 @@ where
     }
 }
 
-impl<'a, R> CopyOnto<ColumnsRegion<R>> for ReadColumns<'a, R>
+impl<R> CopyOnto<ColumnsRegion<R>> for ReadColumns<'_, R>
 where
     R: Region,
+    for<'a> R::ReadItem<'a>: CopyOnto<R>,
 {
     fn copy_onto(self, target: &mut ColumnsRegion<R>) -> <ColumnsRegion<R> as Region>::Index {
         // Ensure all required regions exist.
diff --git a/src/impls/deduplicate.rs b/src/impls/deduplicate.rs
index d64fbff..90b304d 100644
--- a/src/impls/deduplicate.rs
+++ b/src/impls/deduplicate.rs
@@ -36,10 +36,7 @@ impl<R: Region> Default for CollapseSequence<R> {
     }
 }
 
-impl<R: Region> Region for CollapseSequence<R>
-where
-    for<'a, 'b> R::ReadItem<'a>: PartialEq<R::ReadItem<'b>>,
-{
+impl<R: Region> Region for CollapseSequence<R> {
     type ReadItem<'a> = R::ReadItem<'a> where Self: 'a;
     type Index = R::Index;
 
@@ -78,7 +75,6 @@ where
 impl<R: Region, T: CopyOnto<R>> CopyOnto<CollapseSequence<R>> for T
 where
     for<'a> T: PartialEq<R::ReadItem<'a>>,
-    for<'a, 'b> R::ReadItem<'a>: PartialEq<R::ReadItem<'b>>,
 {
     fn copy_onto(self, target: &mut CollapseSequence<R>) -> <CollapseSequence<R> as Region>::Index {
         if let Some(last_index) = target.last_index {
diff --git a/src/impls/string.rs b/src/impls/string.rs
index 68ff9a5..43bff29 100644
--- a/src/impls/string.rs
+++ b/src/impls/string.rs
@@ -11,6 +11,9 @@ use crate::{Containerized, CopyOnto, Region, ReserveItems};
 /// Delegates to a region `R` to store `u8` slices. By default, it uses a [`CopyRegion`], but a
 /// different region can be provided, as long as it absorbs and reads items as `&[u8]`.
 ///
+/// Note that all implementations of `CopyOnto<StringRegion>` must only accept valid utf-8 data
+/// because the region does not validate the contents when indexing.
+///
 /// # Examples
 ///
 /// We fill some data into a string region and use extract it later.
@@ -32,7 +35,6 @@ use crate::{Containerized, CopyOnto, Region, ReserveItems};
 pub struct StringRegion<R = CopyRegion<u8>>
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R>,
 {
     inner: R,
 }
@@ -40,7 +42,6 @@ where
 impl<R> Region for StringRegion<R>
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R>,
 {
     type ReadItem<'a> = &'a str where Self: 'a ;
     type Index = R::Index;
@@ -56,6 +57,7 @@ where
 
     #[inline]
     fn index(&self, index: Self::Index) -> Self::ReadItem<'_> {
+        // SAFETY: All CopyOnto implementations only accept correct utf8 data
         unsafe { std::str::from_utf8_unchecked(self.inner.index(index)) }
     }
 
@@ -110,7 +112,7 @@ where
 impl<R> ReserveItems<StringRegion<R>> for &String
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R> + ReserveItems<R>,
+    for<'a> &'a [u8]: ReserveItems<R>,
 {
     fn reserve_items<I>(target: &mut StringRegion<R>, items: I)
     where
@@ -145,7 +147,7 @@ where
 impl<R> ReserveItems<StringRegion<R>> for &str
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R> + ReserveItems<R>,
+    for<'a> &'a [u8]: ReserveItems<R>,
 {
     fn reserve_items<I>(target: &mut StringRegion<R>, items: I)
     where
@@ -158,7 +160,7 @@ where
 impl<R> ReserveItems<StringRegion<R>> for &&str
 where
     for<'a> R: Region<ReadItem<'a> = &'a [u8]> + 'a,
-    for<'a> &'a [u8]: CopyOnto<R> + ReserveItems<R>,
+    for<'a> &'a [u8]: ReserveItems<R>,
 {
     fn reserve_items<I>(target: &mut StringRegion<R>, items: I)
     where
diff --git a/src/lib.rs b/src/lib.rs
index 16c92b9..cb7ed60 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -41,7 +41,7 @@ impl<T: Copy> Index for T {}
 /// Implement the [`CopyOnto`] trait for all types that can be copied into a region.
 pub trait Region: Default {
     /// The type of the data that one gets out of the container.
-    type ReadItem<'a>: CopyOnto<Self>
+    type ReadItem<'a>
     where
         Self: 'a;
 
@@ -287,7 +287,10 @@ impl<R: Region, T: CopyOnto<R>> FromIterator<T> for FlatStack<R> {
     }
 }
 
-impl<R: Region> Clone for FlatStack<R> {
+impl<R: Region> Clone for FlatStack<R>
+where
+    for<'a> R::ReadItem<'a>: CopyOnto<R>,
+{
     fn clone(&self) -> Self {
         let mut clone = Self::merge_capacity(std::iter::once(self));
         clone.extend(self.iter());
@@ -514,7 +517,7 @@ mod tests {
         where
             T: CopyOnto<R>,
             // Make sure that types are debug, even if we don't use this in the test.
-            for<'a> R::ReadItem<'a>: Debug,
+            for<'a> R::ReadItem<'a>: Debug + CopyOnto<R>,
         {
             let mut c = FlatStack::default();
             c.copy(t);