New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for `[T]::rsplit` and `[T]::rsplit_mut` #41020

Closed
jorendorff opened this Issue Apr 2, 2017 · 12 comments

Comments

Projects
None yet
5 participants
@jorendorff
Contributor

jorendorff commented Apr 2, 2017

Update (@SimonSapin): this is now a tracking issue for methods implemented in #41065.


In @brson's tech review of Programming Rust, when he got to the bit where we list all the slice methods, he wrote: "What about 'rsplit' (no 'n')?"

I thought, oh no, I missed a method. But there really isn't a [T]::rsplit method!

str [T]
.split(p) .split(p)
.rsplit(p) nope
.splitn(p) .splitn(p)
.rsplitn(p) .rsplitn(p)

Slices don't have .rsplit(p), I guess maybe because .split(p).rev() does exactly what .rsplit(p) would do. (The two are subtly different for strings, in cases where the pattern can have overlapping matches.)

But is that a good enough reason to "break symmetry"?

@brson either really expected rsplit to exist, or he was speaking up for a hypothetical reader, new to Rust, who might expect it. Either way, I think it should be added, just to reduce by 1 the number of curious wrinkles in libcore. I'll write an RFC if this seems reasonable.

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Apr 2, 2017

Contributor

I don't think this improvement needs an RFC. It seems like Rsplit needs to use a distinct iterator type for encapsulation so that it can be distinct when subslice splitting finally comes around?

Contributor

bluss commented Apr 2, 2017

I don't think this improvement needs an RFC. It seems like Rsplit needs to use a distinct iterator type for encapsulation so that it can be distinct when subslice splitting finally comes around?

@jorendorff

This comment has been minimized.

Show comment
Hide comment
@jorendorff

jorendorff Apr 4, 2017

Contributor

OK, no RFC. Working on a patch. @bluss Is a feature flag needed for this?

Contributor

jorendorff commented Apr 4, 2017

OK, no RFC. Working on a patch. @bluss Is a feature flag needed for this?

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Apr 4, 2017

Contributor

Yes, the usual way is that you invent a new feature name for it and introduce as unstable.

Contributor

bluss commented Apr 4, 2017

Yes, the usual way is that you invent a new feature name for it and introduce as unstable.

@jorendorff

This comment has been minimized.

Show comment
Hide comment
@jorendorff

jorendorff Apr 4, 2017

Contributor

OK, thanks. I don't need to add it to libsyntax/feature_gate.rs though, right?

Contributor

jorendorff commented Apr 4, 2017

OK, thanks. I don't need to add it to libsyntax/feature_gate.rs though, right?

jorendorff added a commit to jorendorff/rust that referenced this issue Apr 4, 2017

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Apr 4, 2017

Contributor

Correct, that's for language (this is library) feature gates.

Contributor

bluss commented Apr 4, 2017

Correct, that's for language (this is library) feature gates.

jorendorff added a commit to jorendorff/rust that referenced this issue Apr 4, 2017

jorendorff added a commit to jorendorff/rust that referenced this issue Apr 4, 2017

@jorendorff

This comment has been minimized.

Show comment
Hide comment
@jorendorff

jorendorff Apr 4, 2017

Contributor

PR #41065.

Contributor

jorendorff commented Apr 4, 2017

PR #41065.

arielb1 added a commit to arielb1/rust that referenced this issue Apr 5, 2017

@SimonSapin SimonSapin changed the title from Add methods `[T]::rsplit` and `[T]::rsplit_mut` to Tracking issue for `[T]::rsplit` and `[T]::rsplit_mut` Mar 17, 2018

@SimonSapin

This comment has been minimized.

Show comment
Hide comment
@SimonSapin

SimonSapin Mar 17, 2018

Contributor

This should not have been closed since it is the tracking issue for unstable APIs.

Contributor

SimonSapin commented Mar 17, 2018

This should not have been closed since it is the tracking issue for unstable APIs.

@SimonSapin SimonSapin reopened this Mar 17, 2018

@SimonSapin

This comment has been minimized.

Show comment
Hide comment
@SimonSapin

SimonSapin Mar 17, 2018

Contributor

By itself this API seems unnecessary to me as it is the same as split(…).rev(), but str::rsplit and [T]::rsplitn already exist, so adding it for consistency seems good.

This has been in nightly for 11 months without issue:

@rfcbot fcp merge

Contributor

SimonSapin commented Mar 17, 2018

By itself this API seems unnecessary to me as it is the same as split(…).rev(), but str::rsplit and [T]::rsplitn already exist, so adding it for consistency seems good.

This has been in nightly for 11 months without issue:

@rfcbot fcp merge

@SimonSapin

This comment has been minimized.

Show comment
Hide comment
@SimonSapin

SimonSapin Mar 17, 2018

Contributor

With proper labels…

@rfcbot fcp merge

Contributor

SimonSapin commented Mar 17, 2018

With proper labels…

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot Mar 17, 2018

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot commented Mar 17, 2018

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot Mar 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot commented Mar 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot Mar 29, 2018

The final comment period is now complete.

rfcbot commented Mar 29, 2018

The final comment period is now complete.

bors added a commit that referenced this issue Apr 18, 2018

Auto merge of #50017 - tinaun:stabilize-all-the-things, r=sfackler
stabilize a bunch of minor api additions

besides `ptr::NonNull::cast` (which is 4 days away from end of FCP) all of these have been finished with FCP for a few weeks now with minimal issues raised

* Closes #41020
* Closes #42818
* Closes #44030
* Closes #44400
* Closes #46507
* Closes #47653
* Closes #46344

the following functions will be stabilized in 1.27:
* `[T]::rsplit`
* `[T]::rsplit_mut`
* `[T]::swap_with_slice`
* `ptr::swap_nonoverlapping`
* `NonNull::cast`
* `Duration::from_micros`
* `Duration::from_nanos`
* `Duration::subsec_millis`
* `Duration::subsec_micros`
* `HashMap::remove_entry`

@bors bors closed this in #50017 Apr 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment