Skip to content

Conversation

@estebank
Copy link
Contributor

@estebank estebank commented Dec 9, 2025

Split off #148837.

@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 9, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

some of these format kinda weirdly.

View changes since this review

Comment on lines 675 to 678
if let Some(new_secs) = secs.checked_add(1) {
secs = new_secs;
} else {
return None;
}
let Some(new_secs) = secs.checked_add(1) else { return None };
secs = new_secs;
Copy link
Member

Choose a reason for hiding this comment

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

I feel it is much harder to read the control flow in this case because we have hidden the return off to the right.

} else {
return Err(FromUtf16Error(()));
}
let Ok(c) = c else { return Err(FromUtf16Error(())) };
Copy link
Member

Choose a reason for hiding this comment

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

This also loses clarity due to let-else, IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you be ok with the let...else if the return block was on its own line?

Copy link
Member

Choose a reason for hiding this comment

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

basically yes.

let (ptr, layout) = if let Some(mem) = unsafe { self.current_memory(elem_layout) } {
mem
} else {
let Some((ptr, layout)) = (unsafe { self.current_memory(elem_layout) }) else {
Copy link
Member

Choose a reason for hiding this comment

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

this is obviously fine, to be clear.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

workingjubilee commented Dec 10, 2025

it's so weird to me that that matters. oh well.

@workingjubilee
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 10, 2025

📌 Commit 0488690 has been approved by workingjubilee

It is now in the queue for this repository.

@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 Dec 10, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 10, 2025
…bilee

Use `let`...`else` instead of `match foo { ... _ => return };` and `if let ... else return` in std

Split off rust-lang#148837.
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 10, 2025
…bilee

Use `let`...`else` instead of `match foo { ... _ => return };` and `if let ... else return` in std

Split off rust-lang#148837.
bors added a commit that referenced this pull request Dec 10, 2025
Rollup of 12 pull requests

Successful merges:

 - #147602 (Deduplicate higher-ranked lifetime capture errors in impl Trait)
 - #147725 (Remove -Zoom=panic)
 - #148294 (callconv: fix mips64 aggregate argument passing for C FFI)
 - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type)
 - #149417 (tidy: Detect outdated workspaces in workspace list)
 - #149458 (Run clippy on cg_gcc in CI)
 - #149679 (Restrict spe_acc to PowerPC SPE targets)
 - #149781 (Don't suggest wrapping attr in unsafe if it may come from proc macro)
 - #149795 (Use `let`...`else` instead of `match foo { ... _ => return };` and `if let ... else return` in std)
 - #149816 (Make typo in field and name suggestions verbose)
 - #149824 (Add a regression test for issue 145748)
 - #149826 (compiletest: tidy up `adb_path`/`adb_test_dir` handling)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Dec 10, 2025
Rollup of 10 pull requests

Successful merges:

 - #147725 (Remove -Zoom=panic)
 - #148294 (callconv: fix mips64 aggregate argument passing for C FFI)
 - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type)
 - #149458 (Run clippy on cg_gcc in CI)
 - #149679 (Restrict spe_acc to PowerPC SPE targets)
 - #149781 (Don't suggest wrapping attr in unsafe if it may come from proc macro)
 - #149795 (Use `let`...`else` instead of `match foo { ... _ => return };` and `if let ... else return` in std)
 - #149816 (Make typo in field and name suggestions verbose)
 - #149824 (Add a regression test for issue 145748)
 - #149826 (compiletest: tidy up `adb_path`/`adb_test_dir` handling)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3a3c7b5 into rust-lang:main Dec 10, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 10, 2025
rust-timer added a commit that referenced this pull request Dec 10, 2025
Rollup merge of #149795 - estebank:let-else-std, r=workingjubilee

Use `let`...`else` instead of `match foo { ... _ => return };` and `if let ... else return` in std

Split off #148837.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants