From be5eb6f88e3e89059917f168123ba10dd2900f43 Mon Sep 17 00:00:00 2001 From: Barosl Lee Date: Fri, 16 Oct 2015 14:40:23 +0900 Subject: [PATCH 1/6] Add the text --- text/0000-panic-safe-slicing.md | 89 +++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 text/0000-panic-safe-slicing.md diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md new file mode 100644 index 00000000000..4794d491dee --- /dev/null +++ b/text/0000-panic-safe-slicing.md @@ -0,0 +1,89 @@ +- Feature Name: panic_safe_slicing +- Start Date: 2015-10-16 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary + +Add "panic-safe" or "total" alternatives to the existing panicking slicing syntax. + +# Motivation + +`SliceExt::get` and `SliceExt::get_mut` can be thought as non-panicking versions of the simple +slicing syntax, `a[idx]`. However, there is no such equivalent for `a[start..end]`, `a[start..]`, +or `a[..end]`. This RFC proposes such methods to fill the gap. + +# Detailed design + +Add `get_range`, `get_range_mut`, `get_range_unchecked`, `get_range_unchecked_mut` to `SliceExt`. + +`get_range` and `get_range_mut` may be implemented roughly as follows: + +```rust +use std::ops::{RangeFrom, RangeTo, Range}; +use std::slice::from_raw_parts; +use core::slice::SliceExt; + +trait Rangeable { + fn start(&self, slice: &T) -> usize; + fn end(&self, slice: &T) -> usize; +} + +impl Rangeable for RangeFrom { + fn start(&self, _: &T) -> usize { self.start } + fn end(&self, slice: &T) -> usize { slice.len() } +} + +impl Rangeable for RangeTo { + fn start(&self, _: &T) -> usize { 0 } + fn end(&self, _: &T) -> usize { self.end } +} + +impl Rangeable for Range { + fn start(&self, _: &T) -> usize { self.start } + fn end(&self, _: &T) -> usize { self.end } +} + +trait GetRangeExt: SliceExt { + fn get_range>(&self, range: R) -> Option<&[Self::Item]>; +} + +impl GetRangeExt for [T] { + fn get_range>(&self, range: R) -> Option<&[T]> { + let start = range.start(self); + let end = range.end(self); + + if start > end { return None; } + if end > self.len() { return None; } + + unsafe { Some(from_raw_parts(self.as_ptr().offset(start as isize), end - start)) } + } +} + +fn main() { + let a = [1, 2, 3, 4, 5]; + + assert_eq!(a.get_range(1..), Some(&a[1..])); + assert_eq!(a.get_range(..3), Some(&a[..3])); + assert_eq!(a.get_range(2..5), Some(&a[2..5])); + assert_eq!(a.get_range(..6), None); + assert_eq!(a.get_range(4..2), None); +} +``` + +`get_range_unchecked` and `get_range_unchecked_mut` should be the unchecked versions of the methods +above. + +# Drawbacks + +- Are these methods worth adding to `std`? Are such use cases common to justify such extention? + +# Alternatives + +- Stay as is. +- Could there be any other (and better!) total functions that serve the similar purpose? + +# Unresolved questions + +- Naming, naming, naming: Is `get_range` the most suitable name? How about `get_slice`, or just + `slice`? Or any others? From 12071f3f83e2195bdbc3fdd7e7b10564055ab4ee Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 15 Jul 2016 20:41:54 +0000 Subject: [PATCH 2/6] Rewrite to overload existing methods. --- text/0000-panic-safe-slicing.md | 105 ++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 45 deletions(-) diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md index 4794d491dee..c4e4b3e1bad 100644 --- a/text/0000-panic-safe-slicing.md +++ b/text/0000-panic-safe-slicing.md @@ -15,75 +15,90 @@ or `a[..end]`. This RFC proposes such methods to fill the gap. # Detailed design -Add `get_range`, `get_range_mut`, `get_range_unchecked`, `get_range_unchecked_mut` to `SliceExt`. - -`get_range` and `get_range_mut` may be implemented roughly as follows: - +Introduce a `SliceIndex` trait which is implemented by types which can index into a slice: ```rust -use std::ops::{RangeFrom, RangeTo, Range}; -use std::slice::from_raw_parts; -use core::slice::SliceExt; - -trait Rangeable { - fn start(&self, slice: &T) -> usize; - fn end(&self, slice: &T) -> usize; -} +pub trait SliceIndex { + type Output: ?Sized; -impl Rangeable for RangeFrom { - fn start(&self, _: &T) -> usize { self.start } - fn end(&self, slice: &T) -> usize { slice.len() } + fn get(self, slice: &[T]) -> Option<&Self::Output>; + fn get_mut(self, slice: &mut [T]) -> Option<&mut Self::Output>; + unsafe fn get_unchecked(self, slice: &[T]) -> &Self::Output; + unsafe fn get_mut_unchecked(self, slice: &[T]) -> &mut Self::Output; } -impl Rangeable for RangeTo { - fn start(&self, _: &T) -> usize { 0 } - fn end(&self, _: &T) -> usize { self.end } +impl SliceIndex for usize { + type Output = T; + // ... } -impl Rangeable for Range { - fn start(&self, _: &T) -> usize { self.start } - fn end(&self, _: &T) -> usize { self.end } +impl SliceIndex for R + where R: RangeArgument +{ + type Output = [T]; + // ... } +``` -trait GetRangeExt: SliceExt { - fn get_range>(&self, range: R) -> Option<&[Self::Item]>; -} +Alter the `Index`, `IndexMut`, `get`, `get_mut`, `get_unchecked`, and `get_mut_unchecked` +implementations to be generic over `SliceIndex`: +```rust +impl [T] { + pub fn get(&self, idx: I) -> Option + where I: SliceIndex + { + idx.get(self) + } -impl GetRangeExt for [T] { - fn get_range>(&self, range: R) -> Option<&[T]> { - let start = range.start(self); - let end = range.end(self); + pub fn get_mut(&mut self, idx: I) -> Option + where I: SliceIndex + { + idx.get_mut(self) + } - if start > end { return None; } - if end > self.len() { return None; } + pub unsafe fn get_unchecked(&self, idx: I) -> I::Output + where I: SliceIndex + { + idx.get_unchecked(self) + } - unsafe { Some(from_raw_parts(self.as_ptr().offset(start as isize), end - start)) } + pub unsafe fn get_mut_unchecked(&mut self, idx: I) -> I::Output + where I: SliceIndex + { + idx.get_mut_unchecked(self) } } -fn main() { - let a = [1, 2, 3, 4, 5]; +impl Index for [T] + where I: SliceIndex +{ + type Output = I::Output; - assert_eq!(a.get_range(1..), Some(&a[1..])); - assert_eq!(a.get_range(..3), Some(&a[..3])); - assert_eq!(a.get_range(2..5), Some(&a[2..5])); - assert_eq!(a.get_range(..6), None); - assert_eq!(a.get_range(4..2), None); + fn index(&self, idx: I) -> &I::Output { + self.get(idx).expect("out of bounds slice access") + } } -``` -`get_range_unchecked` and `get_range_unchecked_mut` should be the unchecked versions of the methods -above. +impl IndexMut for [T] + where I: SliceIndex +{ + fn index_mut(&self, idx: I) -> &mut I::Output { + self.get_mut(idx).expect("out of bounds slice access") + } +} +``` # Drawbacks -- Are these methods worth adding to `std`? Are such use cases common to justify such extention? +- The `SliceIndex` trait is unfortunate - it's tuned for exactly the set of methods it's used by. + It only exists because inherent methods cannot be overloaded the same way that trait + implementations can be. It would most likely remain unstable indefinitely. # Alternatives - Stay as is. -- Could there be any other (and better!) total functions that serve the similar purpose? +- A previous version of this RFC introduced new `get_slice` etc methods rather than overloading + `get` etc. This avoids the utility trait but is somewhat less ergonomic. # Unresolved questions -- Naming, naming, naming: Is `get_range` the most suitable name? How about `get_slice`, or just - `slice`? Or any others? +None From 17e621b24516f0b14a5baaddcb36db618e4cedaa Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 15 Jul 2016 20:45:52 +0000 Subject: [PATCH 3/6] slicing -> indexing --- text/0000-panic-safe-slicing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md index c4e4b3e1bad..9ef20555aef 100644 --- a/text/0000-panic-safe-slicing.md +++ b/text/0000-panic-safe-slicing.md @@ -5,12 +5,12 @@ # Summary -Add "panic-safe" or "total" alternatives to the existing panicking slicing syntax. +Add "panic-safe" or "total" alternatives to the existing panicking indexing syntax. # Motivation `SliceExt::get` and `SliceExt::get_mut` can be thought as non-panicking versions of the simple -slicing syntax, `a[idx]`. However, there is no such equivalent for `a[start..end]`, `a[start..]`, +indexing syntax, `a[idx]`. However, there is no such equivalent for `a[start..end]`, `a[start..]`, or `a[..end]`. This RFC proposes such methods to fill the gap. # Detailed design From bcb006e3a191c525c8f4b4dc6259d1e0466ade21 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 15 Jul 2016 20:51:24 +0000 Subject: [PATCH 4/6] Add an alternative around separate traits --- text/0000-panic-safe-slicing.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md index 9ef20555aef..79b109406a5 100644 --- a/text/0000-panic-safe-slicing.md +++ b/text/0000-panic-safe-slicing.md @@ -98,6 +98,10 @@ impl IndexMut for [T] - Stay as is. - A previous version of this RFC introduced new `get_slice` etc methods rather than overloading `get` etc. This avoids the utility trait but is somewhat less ergonomic. +- Instead of one trait amalgamating all of the required methods, we could have one trait per + method. This would open a more reasonable door to stabilizing those traits, but adds quite a lot + more surface area. Replacing an unstable `SliceIndex` trait with a collection would be + backwards compatible. # Unresolved questions From e6abfff15d485e39e32a99c5c4a0106d8a5ba05c Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Mon, 18 Jul 2016 21:09:45 +0200 Subject: [PATCH 5/6] Add a drawback about doc readability --- text/0000-panic-safe-slicing.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md index 79b109406a5..24e358e3add 100644 --- a/text/0000-panic-safe-slicing.md +++ b/text/0000-panic-safe-slicing.md @@ -92,6 +92,11 @@ impl IndexMut for [T] - The `SliceIndex` trait is unfortunate - it's tuned for exactly the set of methods it's used by. It only exists because inherent methods cannot be overloaded the same way that trait implementations can be. It would most likely remain unstable indefinitely. +- Documentation may suffer. Rustdoc output currently explicitly shows each of the ways you can + index a slice, while there will simply be a single generic implementation with this change. This + may not be that bad, though. The doc block currently seems to provided the most valuable + information to newcomers rather than the trait bound, and that will still be present with this + change. # Alternatives From 293adaf87cda6f538e7606f6f1242965f6ee7470 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Tue, 19 Jul 2016 23:56:33 +0200 Subject: [PATCH 6/6] Expand a bit and add index methods to trait --- text/0000-panic-safe-slicing.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/text/0000-panic-safe-slicing.md b/text/0000-panic-safe-slicing.md index 24e358e3add..df26ca4847a 100644 --- a/text/0000-panic-safe-slicing.md +++ b/text/0000-panic-safe-slicing.md @@ -10,12 +10,19 @@ Add "panic-safe" or "total" alternatives to the existing panicking indexing synt # Motivation `SliceExt::get` and `SliceExt::get_mut` can be thought as non-panicking versions of the simple -indexing syntax, `a[idx]`. However, there is no such equivalent for `a[start..end]`, `a[start..]`, -or `a[..end]`. This RFC proposes such methods to fill the gap. +indexing syntax, `a[idx]`, and `SliceExt::get_unchecked` and `SliceExt::get_unchecked_mut` can +be thought of as unsafe versions with bounds checks elided. However, there is no such equivalent for +`a[start..end]`, `a[start..]`, or `a[..end]`. This RFC proposes such methods to fill the gap. # Detailed design -Introduce a `SliceIndex` trait which is implemented by types which can index into a slice: +The `get`, `get_mut`, `get_unchecked`, and `get_unchecked_mut` will be made generic over `usize` +as well as ranges of `usize` like slice's `Index` implementation currently is. This will allow e.g. +`a.get(start..end)` which will behave analagously to `a[start..end]`. + +Because methods cannot be overloaded in an ad-hoc manner in the same way that traits may be +implemented, we introduce a `SliceIndex` trait which is implemented by types which can index into a +slice: ```rust pub trait SliceIndex { type Output: ?Sized; @@ -24,6 +31,8 @@ pub trait SliceIndex { fn get_mut(self, slice: &mut [T]) -> Option<&mut Self::Output>; unsafe fn get_unchecked(self, slice: &[T]) -> &Self::Output; unsafe fn get_mut_unchecked(self, slice: &[T]) -> &mut Self::Output; + fn index(self, slice: &[T]) -> &Self::Output; + fn index_mut(self, slice: &mut [T]) -> &mut Self::Output; } impl SliceIndex for usize { @@ -39,7 +48,7 @@ impl SliceIndex for R } ``` -Alter the `Index`, `IndexMut`, `get`, `get_mut`, `get_unchecked`, and `get_mut_unchecked` +And then alter the `Index`, `IndexMut`, `get`, `get_mut`, `get_unchecked`, and `get_mut_unchecked` implementations to be generic over `SliceIndex`: ```rust impl [T] { @@ -74,7 +83,7 @@ impl Index for [T] type Output = I::Output; fn index(&self, idx: I) -> &I::Output { - self.get(idx).expect("out of bounds slice access") + idx.index(self) } } @@ -82,7 +91,7 @@ impl IndexMut for [T] where I: SliceIndex { fn index_mut(&self, idx: I) -> &mut I::Output { - self.get_mut(idx).expect("out of bounds slice access") + idx.index_mut(self) } } ```