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

Added the Option::unzip() method #87636

Merged
merged 4 commits into from
Aug 11, 2021
Merged

Added the Option::unzip() method #87636

merged 4 commits into from
Aug 11, 2021

Conversation

Kixiron
Copy link
Member

@Kixiron Kixiron commented Jul 30, 2021 •
edited
Loading

  • Adds the Option::unzip() method to turn an Option<(T, U)> into (Option<T>, Option<U>) under the unzip_option feature
  • Adds tests for both Option::unzip() and Option::zip(), I noticed that .zip() didn't have any
  • Adds #[inline] to a few of Option's methods that were missing it

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2021
@Kixiron Kixiron changed the title Added the Option::unzip() method Added the Option::unzip() method Jul 30, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@scottmcm scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2021
@scottmcm
Copy link
Member

CI is failing, so I've marked this as waiting-on-author.

You can use rustbot to mark it for review again once things are fixed.

@Kixiron
Copy link
Member Author

Kixiron commented Jul 31, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 31, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Aug 4, 2021

Can you open a tracking issue? Thanks!

@Kixiron Kixiron mentioned this pull request Aug 5, 2021
Closed
3 tasks
@Kixiron
Copy link
Member Author

Kixiron commented Aug 5, 2021

Opened #87800

@jhpratt
Copy link
Member

jhpratt commented Aug 9, 2021

I don't think zip and zip_with need #[inline] because they're generic.

@scottmcm
Copy link
Member

scottmcm commented Aug 9, 2021

@Kixiron Could you remove 73762bf from this commit, please?

I'm happy to take the unzip method, but as @jhpratt mentions, the unrelated inlines are a different thing, so shouldn't be part of this commit. Do feel free to send them as another PR if you'd like, though. (Since generic methods are inline-eligible without the attribute, though, historically core hasn't used them, though, so I'm not sure that libs-impl wants them added.)

And for future reference on the tests, the doctests are run as part of CI, so simple methods don't always need #[test]s as well. Often they're more for tedious cases, or particular edge cases that don't read well in docs.

@scottmcm scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 9, 2021
@Kixiron
Copy link
Member Author

Kixiron commented Aug 9, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 9, 2021
@scottmcm
Copy link
Member

scottmcm commented Aug 9, 2021

Thanks, @Kixiron !

@bors r+ rollup=maybe

@bors
Copy link
Contributor

bors commented Aug 9, 2021

📌 Commit ab2c590 has been approved by scottmcm

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 9, 2021
@bors
Copy link
Contributor

bors commented Aug 9, 2021

⌛ Testing commit ab2c590 with merge d50a3379ae23f3730df6b1a25c00d5cfa7b69819...

@bors
Copy link
Contributor

bors commented Aug 9, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 9, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Aug 9, 2021

@Kixiron: 🔑 Insufficient privileges: not in try users

@scottmcm
Copy link
Member

scottmcm commented Aug 9, 2021

@bors retry

@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 Aug 9, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 10, 2021
@JohnTitor
Rollup merge of rust-lang#87636 - Kixiron:unzip-option, r=scottmcm
28e84e4 
Added the `Option::unzip()` method

* Adds the `Option::unzip()` method to turn an `Option<(T, U)>` into `(Option<T>, Option<U>)` under the `unzip_option` feature
* Adds tests for both `Option::unzip()` and `Option::zip()`, I noticed that `.zip()` didn't have any
* Adds `#[inline]` to a few of `Option`'s methods that were missing it
@JohnTitor JohnTitor mentioned this pull request Aug 10, 2021
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 10, 2021
@JohnTitor
Rollup merge of rust-lang#87636 - Kixiron:unzip-option, r=scottmcm
ef52055 
Added the `Option::unzip()` method

* Adds the `Option::unzip()` method to turn an `Option<(T, U)>` into `(Option<T>, Option<U>)` under the `unzip_option` feature
* Adds tests for both `Option::unzip()` and `Option::zip()`, I noticed that `.zip()` didn't have any
* Adds `#[inline]` to a few of `Option`'s methods that were missing it
@JohnTitor JohnTitor mentioned this pull request Aug 10, 2021
Closed
@JohnTitor JohnTitor mentioned this pull request Aug 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 10, 2021
@bors
Auto merge of rust-lang#87923 - JohnTitor:rollup-id54fyz, r=JohnTitor
e8e1b32 
Rollup of 14 pull requests

Successful merges:

 - rust-lang#86840 (Constify implementations of `(Try)From` for int types)
 - rust-lang#87582 (Implement `Printer` for `&mut SymbolPrinter`)
 - rust-lang#87636 (Added the `Option::unzip()` method)
 - rust-lang#87700 (Expand explanation of E0530)
 - rust-lang#87811 (Do not ICE on HIR based WF check when involving lifetimes)
 - rust-lang#87848 (removed references to parent/child from std::thread documentation)
 - rust-lang#87854 (correctly handle enum variants in `opt_const_param_of`)
 - rust-lang#87861 (Fix heading colours in Ayu theme)
 - rust-lang#87865 (Clarify terms in rustdoc book)
 - rust-lang#87876 (add `windows` count test)
 - rust-lang#87880 (Remove duplicate trait bounds in `rustc_data_structures::graph`)
 - rust-lang#87881 (Proper table row formatting in platform support)
 - rust-lang#87889 (Use smaller spans when suggesting method call disambiguation)
 - rust-lang#87895 (typeck: don't suggest inaccessible fields in struct literals and suggest ignoring inaccessible fields in struct patterns)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bdc92f1 into rust-lang:master Aug 11, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 11, 2021
@Kixiron Kixiron deleted the unzip-option branch June 3, 2022 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@scottmcm scottmcm

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.56.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Kixiron @rust-highfive @rust-log-analyzer @scottmcm @m-ou-se @jhpratt @bors @rustbot