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

Stabilize the partition_point feature #81012

Merged
merged 2 commits into from
Feb 13, 2021

Conversation

VillSnow
Copy link
Contributor

Stabilize the partition_point feature.
Tracking Issue: #73831
First PR: #73577

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 14, 2021
@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 14, 2021
@dtolnay
Copy link
Member

dtolnay commented Jan 23, 2021

@rust-lang/libs
@rfcbot fcp merge

This method finds the start of the range of elements where pred=false, assuming all elements for which pred=true come before all elements for which pred=false, via binary search.

impl<T> [T] {
    pub fn partition_point<P>(&self, pred: P) -> usize
    where
        P: FnMut(&T) -> bool;
}

This is a special case of binary_search_by, but reasonably common and not all that clear expressed using binary_search_by.

slice.partition_point(pred)

// equivalent to
slice.binary_search_by(|x| if pred(x) { Less } else { Greater }).unwrap_err()

The polarity choice for whether false or true elements go first is consistent with the stable Iterator::partition (https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.partition), with STL's std::partition_point in C++ (https://en.cppreference.com/w/cpp/algorithm/partition_point), and with the unstable Iterator::is_partitioned (#62544, https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.is_partitioned).

@rfcbot

This comment has been minimized.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 23, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jan 23, 2021

We recently decided to do FCPs on the tracking issue instead of a stabilization PR if possible, to avoid splitting the discussion to two places. (Our new library tracking issue template reflects this.) There's valuable information in the discussion on the tracking issue, and it'd be good to keep that together with any discussion that follows now we're going for stabilization. So I'm moving the FCP to that issue, and close this for now. Please feel free to re-open this PR once stabilization is decided upon in the tracking issue. Thanks!

@rfcbot cancel

@rfcbot
Copy link

rfcbot commented Jan 23, 2021

@m-ou-se proposal cancelled.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 23, 2021
@m-ou-se m-ou-se mentioned this pull request Jan 23, 2021
5 tasks
@m-ou-se
Copy link
Member

m-ou-se commented Jan 23, 2021

@VillSnow Sorry for the confusion about the library stabilization process. The new info is in the new tracking issue template, but we should really fix the documentation too. ^^'

@matklad
Copy link
Member

matklad commented Feb 10, 2021

Yay, FCP on the tracking issue completed, this can be reopened. I think we also want to change 1.51 to 1.52 here, as the next release is almost here.

@m-ou-se m-ou-se reopened this Feb 10, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Feb 10, 2021

Yay, FCP on the tracking issue completed, this can be reopened. I think we also want to change 1.51 to 1.52 here, as the next release is almost here.

Yup, the version bump to 1.52 already happened last week: https://github.com/rust-lang/rust/blob/master/src/version

@rust-log-analyzer

This comment has been minimized.

@m-ou-se
Copy link
Member

m-ou-se commented Feb 11, 2021

@VillSnow Can you rebase/squash the commits? We generally try to avoid merge commits on branches/PRs.

@VillSnow
Copy link
Contributor Author

@VillSnow Can you rebase/squash the commits? We generally try to avoid merge commits on branches/PRs.

Can I push --force? I made a new branch after rebase and pushed it. I'll feel easy if someone review it before push --force.

@m-ou-se
Copy link
Member

m-ou-se commented Feb 12, 2021

Yes, git push --force is fine :)

Can you also squash the commits?

@VillSnow
Copy link
Contributor Author

Can you also squash the commits?

Is it means six commits 'stabilize partition_points' to 'update feature gate' into single commit like 'stabilize partition_points' ?
or two commits 'update documents' and 'stabilize partition_points' ?

@m-ou-se
Copy link
Member

m-ou-se commented Feb 12, 2021

Both are fine. Whatever you prefer :)

@matklad
Copy link
Member

matklad commented Feb 12, 2021

based on #73831 (comment)

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2021

📌 Commit afdc8c7 has been approved by matklad

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword)
 - rust-lang#81012 (Stabilize the partition_point feature)
 - rust-lang#81479 (Allow casting mut array ref to mut ptr)
 - rust-lang#81506 (HWAddressSanitizer support)
 - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…)
 - rust-lang#81850 (use RWlock when accessing os::env)
 - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id)
 - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String)
 - rust-lang#82023 (Remove unnecessary lint allow attrs on example)
 - rust-lang#82030 (Use `Iterator::all` instead of open-coding it)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8280abc into rust-lang:master Feb 13, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants