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

More let_else adoptions #93590

Merged
merged 1 commit into from
Feb 2, 2022
Merged

More let_else adoptions #93590

merged 1 commit into from
Feb 2, 2022

Conversation

est31
Copy link
Member

@est31 est31 commented Feb 2, 2022

Continuation of #89933, #91018, #91481, #93046.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 2, 2022
@rust-highfive
Copy link
Collaborator

r? @lcnr

(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 Feb 2, 2022
@lcnr
Copy link
Contributor

lcnr commented Feb 2, 2022

@bors r+ rollup

after seeing let_else a lot more, I have to admit i still think it's somewhat difficult to read, even after using it myself.
It definitely improves the general readability of the code and i can't really think of any sensible alternative though 😅

@bors
Copy link
Contributor

bors commented Feb 2, 2022

📌 Commit 670f5c6 has been approved by lcnr

@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 2, 2022
cjgillot
cjgillot reviewed Feb 2, 2022
View reviewed changes
compiler/rustc_attr/src/builtin.rs
@@ -217,85 +217,81 @@ where
let mut issue_num = None;
let mut is_soft = false;
for meta in metas {
if let Some(mi) = meta.meta_item() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is being refactored in #93017. Does c271373db4a58920c2a9629cebaab321b3e2ca57 need to use more let-else?

Copy link
Member Author

Choose a reason for hiding this comment

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

It still has a diverging block in the else case, so your refactored code can still benefit from let else adoption. But I can remove it here so that it doesn't cause conflicts. Does that sound like a plan to you?

@matthiaskrgr matthiaskrgr mentioned this pull request Feb 2, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2022
@bors
Auto merge of rust-lang#93594 - matthiaskrgr:rollup-lcvhpdv, r=matthi…
27f5d83 
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#92528 (Make `Fingerprint::combine_commutative` associative)
 - rust-lang#93221 ([borrowck] Fix help on mutating &self in async fns)
 - rust-lang#93542 (Prevent lifetime elision in type alias)
 - rust-lang#93546 (Validate that values in switch int terminator are unique)
 - rust-lang#93571 (better suggestion for duplicated `where` clause)
 - rust-lang#93574 (don't suggest adding `let` due to bad assignment expressions inside of `while` loop)
 - rust-lang#93590 (More let_else adoptions)
 - rust-lang#93592 (Remove unused dep from rustc_arena)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3388e6d into rust-lang:master Feb 2, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 2, 2022
@est31 est31 mentioned this pull request Feb 15, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 17, 2022
@matthiaskrgr
Rollup merge of rust-lang#94011 - est31:let_else, r=lcnr
637d8b8 
Even more let_else adoptions

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590.
This was referenced Feb 18, 2022
Merged
Merged
Merged
Merged
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2022
@matthiaskrgr
Rollup merge of rust-lang#94146 - est31:let_else, r=cjgillot
7f7f44b 
Adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs rust-lang#94139, rust-lang#94142, rust-lang#94143, rust-lang#94144.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2022
@matthiaskrgr
Rollup merge of rust-lang#94142 - est31:let_else_typeck, r=oli-obk
7ca1c48 
rustc_typeck: adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles rustc_typeck.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2022
@matthiaskrgr
Rollup merge of rust-lang#94146 - est31:let_else, r=cjgillot
f2d6770 
Adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs rust-lang#94139, rust-lang#94142, rust-lang#94143, rust-lang#94144.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 21, 2022
@matthiaskrgr
Rollup merge of rust-lang#94143 - est31:let_else_const_eval, r=lcnr
ea7f7f7 
rustc_const_eval: adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles rustc_const_eval.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 26, 2022
@matthiaskrgr
Rollup merge of rust-lang#94144 - est31:let_else_trait_selection, r=c…
0f95915 
…jgillot

rustc_trait_selection: adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles rustc_trait_selection.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2022
@bors
Auto merge of rust-lang#94144 - est31:let_else_trait_selection, r=cjg…
9323028 
…illot

rustc_trait_selection: adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles rustc_trait_selection.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2022
@bors
Auto merge of rust-lang#94139 - est31:let_else_rustdoc, r=notriddle
b751124 
librustdoc: adopt let else in more places

Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011.

I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles librustdoc.
@apiraino apiraino mentioned this pull request Feb 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

Assignees

@lcnr lcnr

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.60.0
Development

Successfully merging this pull request may close these issues.
6 participants
@est31 @rust-highfive @lcnr @bors @cjgillot @rustbot