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.

Sign up for GitHub

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

Jump to bottom

Stabilize Option::zip #72938

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Stabilize Option::zip #72938

merged 1 commit into from
Jun 15, 2020

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Jun 3, 2020

This PR stabilizes the following API:

impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}

This API has real world usage as seen in https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust.

The zip_with method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc #70086

@rust-highfive
Copy link
Collaborator

r? @cramertj

(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 Jun 3, 2020
@tesuji tesuji mentioned this pull request Jun 3, 2020
Open
4 tasks
@tesuji
Copy link
Contributor Author

tesuji commented Jun 3, 2020

r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned cramertj Jun 3, 2020
@tesuji tesuji force-pushed the stabilize_option_zip branch from 156def6 to 2eb6598 Compare June 3, 2020 08:54
@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 Jun 3, 2020
@jonas-schievink jonas-schievink added this to the 1.45 milestone Jun 3, 2020
@sfackler
Copy link
Member

sfackler commented Jun 4, 2020

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jun 4, 2020 •
edited by Amanieu
Loading

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@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 Jun 4, 2020
dtolnay
dtolnay requested changes Jun 5, 2020
View reviewed changes
src/libcore/option.rs Outdated
@@ -967,7 +969,7 @@ impl<T> Option<T> {
/// assert_eq!(x.zip_with(y, Point::new), Some(Point { x: 17.5, y: 42.7 }));
/// assert_eq!(x.zip_with(None, Point::new), None);
/// ```
#[unstable(feature = "option_zip", issue = "70086")]
#[unstable(feature = "option_zip_with", issue = "70086")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we stabilize only a subset of a library feature, we change the name of the stabilized part, not the still unstable part. That way people only get unused feature warnings downstream, not build failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the stabilized feature to option_zip_option.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jun 5, 2020
@rfcbot
Copy link

rfcbot commented Jun 5, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jun 5, 2020
@sfackler
Copy link
Member

sfackler commented Jun 5, 2020

r=me with the feature names swapped around as @dtolnay mentioned

@tesuji tesuji force-pushed the stabilize_option_zip branch from 2eb6598 to 8b20928 Compare June 13, 2020 01:27
@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Jun 15, 2020
@rfcbot
Copy link

rfcbot commented Jun 15, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jun 15, 2020
@dtolnay
Copy link
Member

dtolnay commented Jun 15, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 15, 2020

📌 Commit 8b20928 has been approved by dtolnay

@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 Jun 15, 2020
@RalfJung
Copy link
Member

@bors rollup

RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
@RalfJung
Rollup merge of rust-lang#72938 - lzutao:stabilize_option_zip, r=dtolnay
aff5c55 
Stabilize Option::zip

This PR stabilizes the following API:

```rust
impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}
```

This API has real world usage as seen in <https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust>.

The `zip_with` method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc rust-lang#70086
This was referenced Jun 15, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2020
@bors
Auto merge of rust-lang#73369 - RalfJung:rollup-hl8g9zf, r=RalfJung
ff4a253 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#72707 (Use min_specialization in the remaining rustc crates)
 - rust-lang#72740 (On recursive ADT, provide indirection structured suggestion)
 - rust-lang#72879 (Miri: avoid tracking current location three times)
 - rust-lang#72938 (Stabilize Option::zip)
 - rust-lang#73086 (Rename "cyclone" to "apple-a7" per changes in upstream LLVM)
 - rust-lang#73104 (Example about explicit mutex dropping)
 - rust-lang#73139 (Add methods to go from a nul-terminated Vec<u8> to a CString)
 - rust-lang#73296 (Remove vestigial CI job msvc-aux.)
 - rust-lang#73304 (Revert heterogeneous SocketAddr PartialEq impls)
 - rust-lang#73331 (extend network support for HermitCore)

Failed merges:

r? @ghost
@bors bors merged commit 89eb74d into rust-lang:master Jun 15, 2020
@tesuji tesuji deleted the stabilize_option_zip branch June 15, 2020 17:20
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 23, 2020
@RalfJung
Rollup merge of rust-lang#72938 - lzutao:stabilize_option_zip, r=dtolnay
93696f4 
Stabilize Option::zip

This PR stabilizes the following API:

```rust
impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}
```

This API has real world usage as seen in <https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust>.

The `zip_with` method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc rust-lang#70086
@jplatte
Copy link
Contributor

jplatte commented Jul 1, 2020

Either the GitHub milestone or the stability attribute is wrong. Former says 1.45.0, latter says 1.46.0.

@dtolnay dtolnay modified the milestones: 1.45, 1.46 Jul 1, 2020
@dtolnay dtolnay self-assigned this Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@sfackler sfackler

@dtolnay dtolnay

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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
Milestone
1.46
Development

Successfully merging this pull request may close these issues.
10 participants
@tesuji @rust-highfive @sfackler @rfcbot @dtolnay @bors @RalfJung @jplatte @jonas-schievink @cramertj