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

if $c:expr { Some($r:expr) } else { None } =>> $c.then(|| $r) #108079

Merged
merged 1 commit into from
Feb 15, 2023
Merged

if $c:expr { Some($r:expr) } else { None } =>> $c.then(|| $r) #108079

merged 1 commit into from
Feb 15, 2023

Conversation

WaffleLapkin
Copy link
Member

TL;DR: turn

if condition {
    Some(expression)
} else {
   None
}

into

condition.then(|| expression)

This is part one of many of refactoring compiler to use .then where it makes code cleaner. This PR does only changes that are simple & clear wins (in my opinion anyway).

@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2023

r? @nagisa

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 15, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@matthiaskrgr
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 15, 2023
@bors
Copy link
Contributor

bors commented Feb 15, 2023

⌛ Trying commit 7319f5c with merge 47c0792...

oli-obk
oli-obk approved these changes Feb 15, 2023
View reviewed changes
compiler/rustc_ast_passes/src/ast_validation.rs
@@ -271,7 +271,7 @@ impl<'a> AstValidator<'a> {

self.session.emit_err(InvalidVisibility {
span: vis.span,
implied: if vis.kind.is_pub() { Some(vis.span) } else { None },
implied: vis.kind.is_pub().then(|| vis.span),
Copy link
Contributor

Choose a reason for hiding this comment

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

These cases may be simpler as then_some (even if longer 🙃 ). I don't have a strong opinion on then_some usage, beyond it generating less code

@oli-obk
Copy link
Contributor

oli-obk commented Feb 15, 2023

@bors r+ rollup

r? @oli-obk

@bors
Copy link
Contributor

bors commented Feb 15, 2023

📌 Commit 7319f5c has been approved by oli-obk

It is now in the queue for this repository.

@rustbot rustbot assigned oli-obk and unassigned nagisa Feb 15, 2023
@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 15, 2023
@bors
Copy link
Contributor

bors commented Feb 15, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 47c0792 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 15, 2023
@bors bors merged commit 47c0792 into rust-lang:master Feb 15, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 15, 2023
@bors bors mentioned this pull request Feb 15, 2023
Merged
@slanterns
Copy link
Contributor

slanterns commented Feb 15, 2023 •
edited
Loading

r+ when doing try build will cause the pr merged without going through testing🙃 And it blocks #101841.

This was referenced Feb 15, 2023
Merged
Merged
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Feb 15, 2023 •
edited
Loading

This PR's merge into master has been force pushed away since we didn't run tests in practice. Please re-open the PR (as a new PR).

@WaffleLapkin WaffleLapkin deleted the if_not_now_then_when branch February 15, 2023 17:35
@WaffleLapkin WaffleLapkin mentioned this pull request Feb 15, 2023
Merged
@matthiaskrgr
Copy link
Member

Perhaps rustbot could add the Blocked label when posting "Awaiting bors try build completion." comment and remove it again when posting the final perf summary

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 16, 2023
@matthiaskrgr
Rollup merge of rust-lang#108090 - WaffleLapkin:if_not_now_then_when……
64a4f70 
…, r=oli-obk

`if $c:expr { Some($r:expr) } else { None }` =>> `$c.then(|| $r)`

Resurrection of rust-lang#108079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.
9 participants
@WaffleLapkin @rustbot @matthiaskrgr @rust-timer @bors @oli-obk @slanterns @Mark-Simulacrum @nagisa