-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Implement split_pattern on slices #131340
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
/// Allows splitting a slice by another slice | ||
(unstable, split_pattern, "CURRENT_RUSTC_VERSION", Some(49036)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Allows splitting a slice by another slice | |
(unstable, split_pattern, "CURRENT_RUSTC_VERSION", Some(49036)), |
Library features shouldn't (and don't need to) be declared inside the compiler.
compiler/rustc_span/src/symbol.rs
Outdated
@@ -1864,6 +1864,7 @@ symbols! { | |||
soft, | |||
specialization, | |||
speed, | |||
split_pattern, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split_pattern, |
(see above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This UI test should be removed. It's only required to exist right now by tidy because you declared a library feature as a compiler feature, too. The doctest should be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! I was following the rustc-dev-guide and thought that all unstable features should be declared in the compiler, thanks for clarifying!
This should get proposed via an ACP, which is one of the issue templates at https://github.com/rust-lang/libs-team/issues. That gives the libs-api team a chance to figure out if the new API makes sense before you put too much effort into the implementation.
As fmease mentioned, library features don't really need to touch anything outside of |
Since it's a library feature it shouldn't be declared inside the compiler
Thank you so much for your help I issued an APC at rust-lang/libs-team#457 and will be going from there |
From #49036, adds a
split_pattern
method on slices which takes a second slice and splits using that one.I was unsure about where to add additional tests, specifically to test reverse iteration using the
next_back
method, my guess would be in library/core/tests/slice.rs, however when looking at the tests I saw no test for unstable features, so I was unsure if I should add them somewhere else.