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 +676
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.

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.

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-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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants