-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 iter_order
stabilization
#27737
Comments
One thing of note is that the origin of this API was to help libcollections, so that the collections didn't all have to reimplement correct PartialOrd eq, le, gt, etc handling*. With time, most of the collections maybe(?) will develop their own specialized versions and make it obsolete. As noted in #26884, these methods are inferior for equality checking since they cannot assume that the two iterators are of equal length. *Due to PartialOrd weirdness, every collection must implement all of lt, le, gt, ge. |
It seems like these should really be methods on pub trait Iterator {
....
fn eq<E, I>(self, other: I) -> bool where Self: Sized, Self::Item: Eq<E>, I: Iterator<Item = E> { ... }
// etc
} |
This does cause some breakage due to deficiencies in resolve - `path::Components` is both an `Iterator` and implements `Eq`, `Ord`, etc. If one calls e.g. `partial_cmp` on a `Components` and passes a `&Components` intending to target the `PartialOrd` impl, the compiler will select the `partial_cmp` from `Iterator` and then error out. I doubt anyone will run into breakage from `Components` specifically, but we should see if there are third party types that will run into issues. `iter::order::equals` wasn't moved to `Iterator` since it's exactly the same as `iter::order::eq` but with an `Eq` instead of `PartialEq` bound, which doensn't seem very useful. I also updated `le`, `gt`, etc to use `partial_cmp` which lets us drop the extra `PartialEq` bound. cc rust-lang#27737
This does cause some breakage due to deficiencies in resolve - `path::Components` is both an `Iterator` and implements `Eq`, `Ord`, etc. If one calls e.g. `partial_cmp` on a `Components` and passes a `&Components` intending to target the `PartialOrd` impl, the compiler will select the `partial_cmp` from `Iterator` and then error out. I doubt anyone will run into breakage from `Components` specifically, but we should see if there are third party types that will run into issues. `iter::order::equals` wasn't moved to `Iterator` since it's exactly the same as `iter::order::eq` but with an `Eq` instead of `PartialEq` bound, which doensn't seem very useful. I also updated `le`, `gt`, etc to use `partial_cmp` which lets us drop the extra `PartialEq` bound. cc #27737 r? @alexcrichton
Would like to stabilize these in the next cycle. |
By the way, is this the same as fn eq<I>(self, other: I) -> bool where Self: Sized, Self::Item: Eq<I::Item>, I: Iterator { ... } ? |
Your version's basically how it was actually defined when implemented: https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html#method.cmp. |
This issue is now entering its cycle-long FCP for stabilization in 1.5 |
Can’t these be implementations of PartialEq/PartialOrd for relevant iterators instead? |
These traits take |
The libs team discussed this during triage today and the decision was to stabilize. |
This commit stabilizes and deprecates library APIs whose FCP has closed in the last cycle, specifically: Stabilized APIs: * `fs::canonicalize` * `Path::{metadata, symlink_metadata, canonicalize, read_link, read_dir, exists, is_file, is_dir}` - all moved to inherent methods from the `PathExt` trait. * `Formatter::fill` * `Formatter::width` * `Formatter::precision` * `Formatter::sign_plus` * `Formatter::sign_minus` * `Formatter::alternate` * `Formatter::sign_aware_zero_pad` * `string::ParseError` * `Utf8Error::valid_up_to` * `Iterator::{cmp, partial_cmp, eq, ne, lt, le, gt, ge}` * `<[T]>::split_{first,last}{,_mut}` * `Condvar::wait_timeout` - note that `wait_timeout_ms` is not yet deprecated but will be once 1.5 is released. * `str::{R,}MatchIndices` * `str::{r,}match_indices` * `char::from_u32_unchecked` * `VecDeque::insert` * `VecDeque::shrink_to_fit` * `VecDeque::as_slices` * `VecDeque::as_mut_slices` * `VecDeque::swap_remove_front` - (renamed from `swap_front_remove`) * `VecDeque::swap_remove_back` - (renamed from `swap_back_remove`) * `Vec::resize` * `str::slice_mut_unchecked` * `FileTypeExt` * `FileTypeExt::{is_block_device, is_char_device, is_fifo, is_socket}` * `BinaryHeap::from` - `from_vec` deprecated in favor of this * `BinaryHeap::into_vec` - plus a `Into` impl * `BinaryHeap::into_sorted_vec` Deprecated APIs * `slice::ref_slice` * `slice::mut_ref_slice` * `iter::{range_inclusive, RangeInclusive}` * `std::dynamic_lib` Closes rust-lang#27706 Closes rust-lang#27725 cc rust-lang#27726 (align not stabilized yet) Closes rust-lang#27734 Closes rust-lang#27737 Closes rust-lang#27742 Closes rust-lang#27743 Closes rust-lang#27772 Closes rust-lang#27774 Closes rust-lang#27777 Closes rust-lang#27781 cc rust-lang#27788 (a few remaining methods though) Closes rust-lang#27790 Closes rust-lang#27793 Closes rust-lang#27796 Closes rust-lang#27810 cc rust-lang#28147 (not all parts stabilized)
This commit stabilizes and deprecates library APIs whose FCP has closed in the last cycle, specifically: Stabilized APIs: * `fs::canonicalize` * `Path::{metadata, symlink_metadata, canonicalize, read_link, read_dir, exists, is_file, is_dir}` - all moved to inherent methods from the `PathExt` trait. * `Formatter::fill` * `Formatter::width` * `Formatter::precision` * `Formatter::sign_plus` * `Formatter::sign_minus` * `Formatter::alternate` * `Formatter::sign_aware_zero_pad` * `string::ParseError` * `Utf8Error::valid_up_to` * `Iterator::{cmp, partial_cmp, eq, ne, lt, le, gt, ge}` * `<[T]>::split_{first,last}{,_mut}` * `Condvar::wait_timeout` - note that `wait_timeout_ms` is not yet deprecated but will be once 1.5 is released. * `str::{R,}MatchIndices` * `str::{r,}match_indices` * `char::from_u32_unchecked` * `VecDeque::insert` * `VecDeque::shrink_to_fit` * `VecDeque::as_slices` * `VecDeque::as_mut_slices` * `VecDeque::swap_remove_front` - (renamed from `swap_front_remove`) * `VecDeque::swap_remove_back` - (renamed from `swap_back_remove`) * `Vec::resize` * `str::slice_mut_unchecked` * `FileTypeExt` * `FileTypeExt::{is_block_device, is_char_device, is_fifo, is_socket}` * `BinaryHeap::from` - `from_vec` deprecated in favor of this * `BinaryHeap::into_vec` - plus a `Into` impl * `BinaryHeap::into_sorted_vec` Deprecated APIs * `slice::ref_slice` * `slice::mut_ref_slice` * `iter::{range_inclusive, RangeInclusive}` * `std::dynamic_lib` Closes #27706 Closes #27725 cc #27726 (align not stabilized yet) Closes #27734 Closes #27737 Closes #27742 Closes #27743 Closes #27772 Closes #27774 Closes #27777 Closes #27781 cc #27788 (a few remaining methods though) Closes #27790 Closes #27793 Closes #27796 Closes #27810 cc #28147 (not all parts stabilized)
The
iter
module has anorder
submodule for performing comparisons on iterators. Ideally, these would instead just be implementations of e.g. thePartialOrd
trait, but that trait passes by shared reference, and iterators must be mutated to be compared.It'd be good to explore this design space, and perhaps write an RFC, before stabilization.
cc @bluss
The text was updated successfully, but these errors were encountered: