From 026e270279ef6ee499c852896137ab214503bd3c Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 7 Sep 2019 14:07:58 +0200 Subject: [PATCH 1/2] FIX: Fix incorrect proof in join_cover This is an API break, because the previous proof was unsound. Like the bug points out, in join cover we take (self, other) and extend self until the end of other; if other ends before self, we don't gain any "cover" from other, and thus the proof is the same as self (making the method quite useless). Added traits for checking the proof value in dynamic code, since completest really doesn't work well right now. --- src/indexing.rs | 42 +++++++++++++++++++++++++++++++++++++++++- src/proof.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/indexing.rs b/src/indexing.rs index c93c3ac..7e3821b 100644 --- a/src/indexing.rs +++ b/src/indexing.rs @@ -178,7 +178,28 @@ impl<'id, P> Range<'id, P> { } /// Extend the range to the end of `other`, including any space in between - pub fn join_cover(&self, other: Range<'id, Q>) -> Range<'id, <(P, Q) as ProofAdd>::Sum> + /// + /// + /// ```compile-fail + /// // compile-fail only enabled in 2018 edition + /// // Bug from https://github.com/bluss/indexing/issues/12 + /// use indexing::scope; + /// + /// let array = [0, 1, 2, 3, 4, 5]; + /// scope(&array[..], |arr| { + /// let left = arr.vet_range(0..2).unwrap(); + /// let left = left.nonempty().unwrap(); + /// let (_, right) = arr.range().frontiers(); + + /// let joined = right.join_cover(left); + /// let ix = joined.first(); + /// dbg!(arr[ix]); //~ ERROR Can't index by ix, because it's an edge index + /// }); + /// ``` + // Proof P: Extends at least as far as self, not necessarily using any part + // of other + // + pub fn join_cover(&self, other: Range<'id, Q>) -> Range<'id, P> where (P, Q): ProofAdd, { let end = cmp::max(self.end, other.end); @@ -188,6 +209,7 @@ impl<'id, P> Range<'id, P> { } /// Extend the range to start and end of `other`, including any space in between + // Proof Sum of P and Q: Covers the union, so must be nonempty if any are pub fn join_cover_both(&self, other: Range<'id, Q>) -> Range<'id, <(P, Q) as ProofAdd>::Sum> where (P, Q): ProofAdd, { @@ -563,3 +585,21 @@ fn test_frac_step() { assert_eq!(f.next(), None); } + +#[test] +fn test_join_cover() { + use scope; + + // Bug from https://github.com/bluss/indexing/issues/12 + let array = [0, 1, 2, 3, 4, 5]; + scope(&array[..], |arr| { + let left = arr.vet_range(0..2).unwrap(); + let left = left.nonempty().unwrap(); + let (_, right) = arr.range().frontiers(); + + let joined = right.join_cover(left); + let ix = joined.first(); + assert!(!ix.nonempty_proof()); + ix.integer() + }); +} diff --git a/src/proof.rs b/src/proof.rs index fb1172d..d8e0063 100644 --- a/src/proof.rs +++ b/src/proof.rs @@ -87,3 +87,31 @@ impl<'id, T, P> Provable for PSlice<'id, T, P> { } } +#[cfg(test)] +pub(crate) trait ProofType { + fn nonempty() -> bool; + fn unknown() -> bool { !Self::nonempty() } +} + +#[cfg(test)] +impl ProofType for Unknown { + fn nonempty() -> bool { false } +} + +#[cfg(test)] +impl ProofType for NonEmpty { + fn nonempty() -> bool { false } +} + + +#[cfg(test)] +impl<'id, P> Index<'id, P> { + pub(crate) fn nonempty_proof(&self) -> bool where P: ProofType + { P::nonempty() } +} + +#[cfg(test)] +impl<'id, P> Range<'id, P> { + pub(crate) fn nonempty_proof(&self) -> bool where P: ProofType + { P::nonempty() } +} From 2aa7b97669e453f9cf9175a072543b3cbd71b05b Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 7 Sep 2019 14:15:42 +0200 Subject: [PATCH 2/2] FIX: Use &mut properly for ContiguousMut begin_mut/end_mut This is the only sound way to derive proper raw mut pointers from slices. --- src/container_traits.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/container_traits.rs b/src/container_traits.rs index e8e281a..11e60a8 100644 --- a/src/container_traits.rs +++ b/src/container_traits.rs @@ -22,12 +22,8 @@ pub unsafe trait GetUncheckedMut : GetUnchecked { } pub unsafe trait ContiguousMut : Contiguous { - fn begin_mut(&self) -> *mut Self::Item { - self.begin() as _ - } - fn end_mut(&self) -> *mut Self::Item { - self.end() as _ - } + fn begin_mut(&mut self) -> *mut Self::Item; + fn end_mut(&mut self) -> *mut Self::Item; fn as_mut_slice(&mut self) -> &mut [Self::Item]; } @@ -52,6 +48,8 @@ unsafe impl<'a, C: ?Sized> Trustworthy for &'a mut C unsafe impl<'a, C: ?Sized> ContiguousMut for &'a mut C where C: ContiguousMut { + fn begin_mut(&mut self) -> *mut Self::Item { (**self).begin_mut() } + fn end_mut(&mut self) -> *mut Self::Item { (**self).end_mut() } fn as_mut_slice(&mut self) -> &mut [Self::Item] { (**self).as_mut_slice() } @@ -115,6 +113,14 @@ unsafe impl Trustworthy for [T] { } unsafe impl ContiguousMut for [T] { + fn begin_mut(&mut self) -> *mut Self::Item { + self.as_mut_ptr() + } + fn end_mut(&mut self) -> *mut Self::Item { + unsafe { + self.begin_mut().add(self.len()) + } + } fn as_mut_slice(&mut self) -> &mut [Self::Item] { self } @@ -138,7 +144,7 @@ unsafe impl Contiguous for [T] { } fn end(&self) -> *const Self::Item { unsafe { - self.begin().offset(self.len() as isize) + self.begin().add(self.len()) } } fn as_slice(&self) -> &[Self::Item] { @@ -155,6 +161,8 @@ mod vec_impls { } unsafe impl ContiguousMut for Vec { + fn begin_mut(&mut self) -> *mut Self::Item { (**self).begin_mut() } + fn end_mut(&mut self) -> *mut Self::Item { (**self).end_mut() } fn as_mut_slice(&mut self) -> &mut [Self::Item] { self }