Define fn [_]::try_split_at(&self, usize) -> Option<(&Self, &Self)>

[strake]

No description provided.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[Centril]

Add a comment explaining why this is safe.

[hanna-kruppe]

More importantly, please justify using unsafe at all. While it's possible that something if mid > self.len() { None } else { (&self[..mid], &self[mid..]) } may end up with redundant bounds checks, in principle they can be optimized out and we should only use unsafe code if that optimization fails to fire.

[strake]

I just checked: it indeed elides the bounds checks and emits the same LLVM IR in either case.

[Centril]

Add a comment explaining why this is safe.

[strake]

I redefined these functions sans unsafe blocks. PTAL

[rust-highfive]

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:53:16] ...................................................................................................i 2200/4567
[00:53:20] .................................................................................................... 2300/4567
[00:53:24] .................................................................................................... 2400/4567
[00:53:27] .................................................................................................... 2500/4567
[00:53:31] ............iiiiiiiii............................................................................... 2600/4567
[00:53:37] .................................................................................................... 2800/4567
[00:53:41] .................................................................................................... 2900/4567
[00:53:44] ................................i................................................................... 3000/4567
[00:53:47] ............................................................................................i.i..ii. 3100/4567
---
[01:21:21] .................................................................................................... 1700/2167
[01:21:30] .................................................................................................... 1800/2167
[01:21:38] .................................................................................................... 1900/2167
[01:21:48] .................................................................................................... 2000/2167
[01:21:59] ....F............FF................................................................i................ 2100/2167
[01:22:05] failures:
[01:22:05] 
[01:22:05] ---- slice/mod.rs - slice::[T]::split_at_mut (line 872) stdout ----
[01:22:05] ---- slice/mod.rs - slice::[T]::split_at_mut (line 872) stdout ----
[01:22:05] error[E0658]: use of unstable library feature 'try_split_at' (see issue #54886)
[01:22:05]  --> slice/mod.rs:876:27
[01:22:05]   |
[01:22:05] 7 |     let (left, right) = v.try_split_at_mut(2);
[01:22:05]   |
[01:22:05]   |
[01:22:05]   = help: add #![feature(try_split_at)] to the crate attributes to enable
[01:22:05] error[E0308]: mismatched types
[01:22:05]  --> slice/mod.rs:876:9
[01:22:05]   |
[01:22:05]   |
[01:22:05] 7 |     let (left, right) = v.try_split_at_mut(2);
[01:22:05]   |         ^^^^^^^^^^^^^ expected enum `std::option::Option`, found tuple
[01:22:05]   |
[01:22:05]   = note: expected type `std::option::Option<(&mut [{integer}], &mut [{integer}])>`
[01:22:05]              found type `(_, _)`
[01:22:05] thread 'slice/mod.rs - slice::[T]::split_at_mut (line 872)' panicked at 'couldn't compile the test', librustdoc/test.rs:333:13
[01:22:05] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:22:05] 
[01:22:05] ---- slice/mod.rs - slice::[T]::try_split_at_mut (line 908) stdout ----
[01:22:05] ---- slice/mod.rs - slice::[T]::try_split_at_mut (line 908) stdout ----
[01:22:05] error: this file contains an un-closed delimiter
[01:22:05]   --> slice/mod.rs:922:2
[01:22:05]    |
[01:22:05] 3  | fn main() {
[01:22:05]    |           - un-closed delimiter
[01:22:05] ...
[01:22:05] 7  |     match v.split_at_mut(2) {
[01:22:05]    |                             - this delimiter might not be properly closed...
[01:22:05] 15 | }
[01:22:05] 15 | }
[01:22:05]    | - ...as it matches this but it has different indentation
[01:22:05] 16 | assert!(v == [1, 2, 3, 4, 5, 6]);
[01:22:05] 17 | }
[01:22:05]    |  ^
[01:22:05] error[E0308]: mismatched types
[01:22:05]  --> slice/mod.rs:913:9
[01:22:05]   |
[01:22:05] 8 |         Some((left, right)) => {
[01:22:05] 8 |         Some((left, right)) => {
[01:22:05]   |         ^^^^^^^^^^^^^^^^^^^ expected tuple, found enum `std::option::Option`
[01:22:05]   |
[01:22:05]   = note: expected type `(&mut [{integer}], &mut [{integer}])`
[01:22:05] 
[01:22:05] error[E0308]: mismatched types
[01:22:05]   --> slice/mod.rs:919:9
[01:22:05]    |
[01:22:05]    |
[01:22:05] 14 |         None => assert!(false),
[01:22:05]    |         ^^^^ expected tuple, found enum `std::option::Option`
[01:22:05]    |
[01:22:05]    = note: expected type `(&mut [{integer}], &mut [{integer}])`
[01:22:05] 
[01:22:05] thread 'slice/mod.rs - slice::[T]::try_split_at_mut (line 908)' panicked at 'couldn't compile the test', librustdoc/test.rs:333:13
[01:22:05] 
[01:22:05] ---- slice/mod.rs - slice::[T]::try_split_at (line 817) stdout ----
[01:22:05] ---- slice/mod.rs - slice::[T]::try_split_at (line 817) stdout ----
[01:22:05] error[E0658]: use of unstable library feature 'try_split_at' (see issue #54886)
[01:22:05]  --> slice/mod.rs:821:13
[01:22:05]   |
[01:22:05] 7 |     match v.try_split_at(0) {
[01:22:05]   |
[01:22:05]   |
[01:22:05]   = help: add #![feature(try_split_at)] to the crate attributes to enable
[01:22:05] 
[01:22:05] error[E0658]: use of unstable library feature 'try_split_at' (see issue #54886)
[01:22:05]   --> slice/mod.rs:831:13
[01:22:05]    |
[01:22:05] 17 |     match v.try_split_at(2) {
[01:22:05]    |
[01:22:05]    |
[01:22:05]    = help: add #![feature(try_split_at)] to the crate attributes to enable
[01:22:05] 
[01:22:05] error[E0658]: use of unstable library feature 'try_split_at' (see issue #54886)
[01:22:05]   --> slice/mod.rs:841:13
[01:22:05]    |
[01:22:05] 27 |     match v.try_split_at(6) {
[01:22:05]    |
[01:22:05]    |
[01:22:05]    = help: add #![feature(try_split_at)] to the crate attributes to enable
[01:22:05] 
[01:22:05] error[E0599]: no method named `is_none` found for type `(&[{integer}], &[{integer}])` in the current scope
[01:22:05]   --> slice/mod.rs:851:27
[01:22:05]    |
[01:22:05] 37 |     assert!(v.split_at(7).is_none())
[01:22:05] 
[01:22:05] thread 'slice/mod.rs - slice::[T]::try_split_at (line 817)' panicked at 'couldn't compile the test', librustdoc/test.rs:333:13
[01:22:05] 
[01:22:05] 
---
[01:22:05] 
[01:22:05] error: test failed, to rerun pass '--doc'
[01:22:05] 
[01:22:05] 
[01:22:05] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "core" "--" "--quiet"
[01:22:05] 
[01:22:05] 
[01:22:05] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:22:05] Build completed unsuccessfully in 0:33:31
[01:22:05] Build completed unsuccessfully in 0:33:31
[01:22:05] Makefile:58: recipe for target 'check' failed
[01:22:05] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0268906b
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[scottmcm]

For my edification, can you elaborate why these are better than the caller just checking things? Should there also be try_ versions of other methods on slices that take indexes, like try_swap or try_rotate_left? Since these methods are safe, couldn't they live in a crate instead of core?

[strake]

@scottmcm

I just think it's easier to read and harder to misuse in certain use cases — in my experience, for example: parsing certain binary formats, i want to return notice of invalid data if the given slice is too short, but if it is written in terms of split_at rather than try_split_at it is too easy to delete the bounds check in a later refactorization and not notice. I already defined this function in a utility crate but i thought others might also find it useful enough to include in core, rather than depend on yet another utility crate and use an extension trait, which is noise.

I seldom find myself checking bounds before calling swap or rotate and these methods return unit anyhow so are easier to wrap in a simple if.

[alexcrichton]

I personally agree with @scottmcm where I don't think we should add this to the standard library at this time. We don't want to have a dual of all the functionality on slices, both fallible and infallible. Instead we, long ago, decided that we'd only have either infallible or fallible and for slices it's already all related to indexing, which is always infallible, so slices inherited that behavior.

Considering to reverse this decision by adding try_* variants of all the various methods I believe is RFC-worthy rather than doing one-off in a PR.

I agree that a local optimum can often be achieved with one extra method, but such a method can always be defined locally in a crate or as a library on crates.io.

[TimNN]

Ping from triage! Thank for your PR, @strake. Based on @alexcrichton's and @scottmcm's comments I'm going to close this PR as blocked on an RFC. Thanks for your contribution!

[Ben-Lichtman]

I know that this issue is long-closed, however I'd like to add that this would allow the removal of panicking code paths from critical code which may not panic, so I suggest a reconsideration.

Without this, one has to rely on unstable optimizations to remove panicking code.