Removed rev_iter in favor of DoubleEndedIterator #13648
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
If we decide to merge this, the commit message will need to be updated with at least a mention that it is a breaking change, and it would be nice for the commit message to have more rationale and a clear explanation of how to upgrade code. You can find some more information here |
|
I may also recommend using |
|
Sorry about that - I didn't know that a new policy was in place. Should I then split this into two commits, one changing the behavior of |
|
If you could, that would be great, thanks! |
|
Okay, I broke this into two parts and added nice, informative commit messages. |
|
First of all, thank you! Those commit messages are excellent. Just making sure I understand this, the only remaining iterators which have first-class constructors are:
Is that it? I think I'd be ok with the treemap iterators for now, but perhaps the |
|
You are correct about |
|
@thestinger What do you think of this? |
added 2 commits
April 28, 2014 16:45
…[T]::split and &[T]::rsplit This makes the splitting functions in std::slice return DoubleEndedIterators. Unfortunately, splitn and rsplitn cannot provide such an interface and so must return different types. As a result, the following changes were made: * RevSplits was removed in favor of explicitly using Rev * Splits can no longer bound the number of splits done * Splits now implements DoubleEndedIterator * SplitsN was added, taking the role of what both Splits and RevSplits used to be * rsplit returns Rev<Splits<'a, T>> instead of RevSplits<'a, T> * splitn returns SplitsN<'a, T> instead of Splits<'a, T> * rsplitn returns SplitsN<'a, T> instead of RevSplits<'a, T> All functions that were previously implemented on each return value still are, so outside of changing of type annotations, existing code should work out of the box. In the rare case that code relied on the return types of split and splitn or of rsplit and rsplitn being the same, the previous behavior can be emulated by calling splitn or rsplitn with a bount of uint::MAX. The value of this change comes in multiple parts: * Consistency. The splitting code in std::str is structured similarly to the new slice splitting code, having separate CharSplits and CharSplitsN types. * Smaller API. Although this commit doesn't implement it, using a DoubleEndedIterator for splitting means that rsplit, path::RevComponents, path::RevStrComponents, Path::rev_components, and Path::rev_str_components are no longer needed - they can be emulated simply with .rev(). * Power. DoubleEndedIterators are able to traverse the list from both sides at once instead of only forwards or backwards. * Efficiency. For the common case of using split instead of splitn, the iterator is slightly smaller and slightly faster. [breaking-change]
…tor is provided (everywhere but treemap) This commit deprecates rev_iter, mut_rev_iter, move_rev_iter everywhere (except treemap) and also deprecates related functions like rsplit, rev_components, and rev_str_components. In every case, these functions can be replaced with the non-reversed form followed by a call to .rev(). To make this more concrete, a translation table for all functional changes necessary follows: * container.rev_iter() -> container.iter().rev() * container.mut_rev_iter() -> container.mut_iter().rev() * container.move_rev_iter() -> container.move_iter().rev() * sliceorstr.rsplit(sep) -> sliceorstr.split(sep).rev() * path.rev_components() -> path.components().rev() * path.rev_str_components() -> path.str_components().rev() In terms of the type system, this change also deprecates any specialized reversed iterator types (except in treemap), opting instead to use Rev directly if any type annotations are needed. However, since methods directly returning reversed iterators are now discouraged, the need for such annotations should be small. However, in those cases, the general pattern for conversion is to take whatever follows Rev in the original reversed name and surround it with Rev<>: * RevComponents<'a> -> Rev<Components<'a>> * RevStrComponents<'a> -> Rev<StrComponents<'a>> * RevItems<'a, T> -> Rev<Items<'a, T>> * etc. The reasoning behind this change is that it makes the standard API much simpler without reducing readability, performance, or power. The presence of functions such as rev_iter adds more boilerplate code to libraries (all of which simply call .iter().rev()), clutters up the documentation, and only helps code by saving two characters. Additionally, the numerous type synonyms that were used to make the type signatures look nice like RevItems add even more boilerplate and clutter up the docs even more. With this change, all that cruft goes away. [breaking-change]
|
Rebased. |
|
Oops, meant to comment earlier, sorry! We discussed this at this week's meeting and decided to merge. Thanks for being patient! |
bors
added a commit
that referenced
this pull request
Apr 30, 2014
In the process, `Splits` got changed to be more like `CharSplits` in `str` to present the DEI interface. Note that `treemap` still has a `rev_iter` function because it seems like it would be a significant interface change to expose a DEI - the iterator would have to gain an extra pointer, the completion checks would be more complicated, and it isn't easy to check that such an implementation is correct due to the use of unsafety to subvert the aliasing properties of `&mut`. This fixes #9391.
arcnmx
pushed a commit
to arcnmx/rust
that referenced
this pull request
Jan 9, 2023
…ler-diagnostics, r=ian-h-chamberlain Colorize `cargo check` diagnostics in VSCode via text decorations Fixes rust-lang#13648  Use ANSI control characters to display text decorations matching the VScode terminal theme, and strip them out when providing text content for rustc diagnostics. This adds the small [`anser`](https://www.npmjs.com/package/anser) library (MIT license, no dependencies) to parse the control codes, and it also supports HTML output so it should be fairly easy to switch to a rendered HTML/webview implementation in the future I also updated the default `cargo check` command to use the rendered ANSI diagnostics, although I'm not sure if it makes sense to put this kind of thing behind a feature flag, or whether it might have any issues on Windows (as I believe ANSI codes are not used for colorization there)?
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Nov 7, 2024
…ntri3 needless_continue: check labels consistency before warning changelog: [`needless_continue`]: check labels before warning about `continue` as the last statement in a loop body Fix rust-lang#13641
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the process,
Splitsgot changed to be more likeCharSplitsinstrto present the DEI interface.Note that
treemapstill has arev_iterfunction because it seems like it would be a significant interface change to expose a DEI - the iterator would have to gain an extra pointer, the completion checks would be more complicated, and it isn't easy to check that such an implementation is correct due to the use of unsafety to subvert the aliasing properties of&mut.This fixes #9391.