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.

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

Bump MSRV to 1.65 #1108

Merged
merged 4 commits into from
Oct 22, 2023
Merged

Bump MSRV to 1.65 #1108

merged 4 commits into from
Oct 22, 2023

Conversation

chklauser
Copy link
Contributor

The primary motivation is the option to use let-else for the C-like error handling that low level system APIs usually require.

I created this as a separate PR because I didn't want these changes and discussion around it to hold #1107 hostage.

While there are a lot of opportunities to use let-else in windows/disk.rs I ultimately didn't change that many cases, because more often than not, the code would have had the form

let Ok(()) = SomeWindowsApi(..) else {
  return Vec::new();
}

The let-else syntax does have the slight advantage that it forces you to diverge/return in the else block. That means you can't accidentally forget to return and fall through the else block. But compared to

if SomeWindowsApi(..).is_err() {
  return Vec::new()
}

it just reads weird ☹️.

In the process, I did find another interesting trick: Error::from_win32() internally calls GetLastError without wrapping it in a result. For cases where we don't expect a 0 success case from GetLastError, that seems simpler.

The primary motivation is the option to use let-else for the C-like error handling that low level system APIs usually require.
@GuillaumeGomez
Copy link
Owner

Looks good to me, thanks for the improvements! The CI is failing because you forgot to run cargo fmt. Please fix it and then I merge.

@GuillaumeGomez
Copy link
Owner

Thanks a lot!

@GuillaumeGomez GuillaumeGomez merged commit 911d230 into GuillaumeGomez:master Oct 22, 2023
67 checks passed
njouanin pushed a commit to njouanin/sysinfo that referenced this pull request Nov 4, 2023
* Raise MSRV to 1.65

The primary motivation is the option to use let-else for the C-like error handling that low level system APIs usually require.

* Simplify error handling in windows/disk.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants