Skip to content
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_by #64295

Open
2 tasks
KodrAus opened this issue Sep 8, 2019 · 6 comments
Open
2 tasks

Tracking issue for iter_order_by #64295

KodrAus opened this issue Sep 8, 2019 · 6 comments
Labels
A-iterators Area: Iterators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Sep 8, 2019

Landed in #62205

Public API

pub mod core {
    pub mod iter {
        mod traits {
            mod iterator {
                pub trait Iterator {
                    fn cmp_by<I, F>(mut self, other: I, mut cmp: F) -> Ordering
                    where
                        Self: Sized,
                        I: IntoIterator,
                        F: FnMut(Self::Item, I::Item) -> Ordering,
                    {
                    }
                    fn partial_cmp_by<I, F>(
                        mut self,
                        other: I,
                        mut partial_cmp: F,
                    ) -> Option<Ordering>
                    where
                        Self: Sized,
                        I: IntoIterator,
                        F: FnMut(Self::Item, I::Item) -> Option<Ordering>,
                    {
                    }
                    fn eq_by<I, F>(mut self, other: I, mut eq: F) -> bool
                    where
                        Self: Sized,
                        I: IntoIterator,
                        F: FnMut(Self::Item, I::Item) -> bool,
                    {
                    }
                }
            }
        }
    }
}

Before stabilization:

  • Stabilization PR

Open questions

  • Should we use size_hint as an optimization for equality checking? That means buggy iterators may return incorrect results for eq_by. We could optimize for just a handful of TrustedLen iterators instead.
@KodrAus KodrAus added A-collections Area: `std::collection` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Sep 8, 2019
@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. A-iterators Area: Iterators and removed A-collections Area: `std::collection` labels Sep 10, 2019
Centril added a commit to Centril/rust that referenced this issue Sep 23, 2019
Document the unstable iter_order_by library feature

Tracking issue: rust-lang#64295

Follow-up for: rust-lang#62205

References the tracking issue and adds a page to the unstable book for the new unstable `iter_order_by` feature.
Centril added a commit to Centril/rust that referenced this issue Sep 24, 2019
Document the unstable iter_order_by library feature

Tracking issue: rust-lang#64295

Follow-up for: rust-lang#62205

References the tracking issue and adds a page to the unstable book for the new unstable `iter_order_by` feature.
Centril added a commit to Centril/rust that referenced this issue Sep 24, 2019
Document the unstable iter_order_by library feature

Tracking issue: rust-lang#64295

Follow-up for: rust-lang#62205

References the tracking issue and adds a page to the unstable book for the new unstable `iter_order_by` feature.
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 30, 2020
@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 18, 2021

A comment was made on Twitter about whether or not this should be able to use size_hint to determine whether or not the iterators have any chance of containing equal elements.

size_hint can't be relied upon for safety, but it seems like it could be reasonable to use it for short-circuiting eq_by.

@yoshuawuyts
Copy link
Member

An example usage of this is in rust-analyzer where it's part of its internal stdx crate which contains stdlib extensions.

@chrysn
Copy link
Contributor

chrysn commented Jun 10, 2022

TrustedLen is an unsafe trait, which I understand is there to catch cases when wrong lengths would result in soundness holes. The size hint is a bit more than a hint -- an iterator that does not adhere to the bounds is considered buggy by size_hint's documentation. I'd place returning false to eq_by on an erroneous iterator in a similar category as a run-time integer overflow that is not caught because the wrong index is not a soundness violation (just a follow-up program error from a previous program error).

Is that the only contested point about this interface? If not, what is needed to move this toward stabilization? (If it's user reports: it simplifies code in coap-handler-implementations quite a bit).

@Hiten-Tandon
Copy link

Function should be renamed to related_by, because, in the function we're not checking for equality, rather we're checking for relation between the values of the two iterators.

@orlp
Copy link
Contributor

orlp commented Sep 3, 2024

So is there any progress on this? What's blocking stabilization?

Regarding the size hint being used, that's currently not being done for the already stable Iterator::eq. I think there are really only two paths forward:

  1. Don't check the size hint.
  2. Check the size hint in all Iterator PartialEq-related methods. This means deciding it's not considered a breaking change to start doing this for Iterator::eq.

I really don't think there is ever a viable future for having some of the methods check the size hint and others do not, so I really believe the two above choices are the only ones. It's just too much of a footgun.

However regardless of which of these two choices you make, Iterator::eq_by's stabilization is not blocked by it. In scenario 1 the current implementation is fine. In scenario 2 you've already decided it's not a breaking change so it can be stabilized.

@leb-kuchen
Copy link

I think the question should be TrustedLen or ExaxtSizeIterator. ExaxtSizeIterator is also user implemented, but is supposed to be exact and not only a hint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants