-
Notifications
You must be signed in to change notification settings - Fork 270
edition 2024 and raise MSRV to 1.88 #731
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
edition 2024 and raise MSRV to 1.88 #731
Conversation
down to "only" fixing 32bit Windows unsafety |
96ee90e
to
21c02bd
Compare
note re: timing, it is possible another MSRV 1.82 release should land first. |
88970de
to
eb7f722
Compare
eb7f722
to
f141372
Compare
Could you rebase on master? Github's diff seems to be confused. |
This allows using let-chains in the code, but requires a repo-wide fmt.
f141372
to
7d52b2a
Compare
Done! It probably had trouble identifying the commits as being the same contentfully. |
f3e305c
to
3ae4456
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
// SAFETY: Name must be initialized by SymFromAddrW, but we only interact with it as a pointer anyways. | ||
let name_ptr = unsafe { (&raw const (*info).Name).cast::<u16>() }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this unsafe block during my post-push editing of #737 because I thought the compiler said it was superfluous, but now it is required for reasons that aren't clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I wondered about that but I trusted the compiler to complain if it's not needed, ha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may need to poke at this a bit more if it's an edition-based unsafety difference: that shouldn't really be the case. It could have just been human confusion on my end though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...I identified the diff in question removed deny(unsafe_op_in_unsafe_fn)
as well as the unsafe
block, so there is no edition-based problem aside from the lint being enabled or not.
With 1.90 being released, the stable - 2 policy now means 1.88 is fair game and we can use let-chains in the code. As we have lots of parsing code, this is a significant improvement. However, first, we have to also update to edition 2024. I have left out let-chain improvements from this PR to make it tractable to review.
Due to the somewhat adventuresome nature of updating the edition, this also includes cleaning up the win32 implementation a fair deal. Mostly this is the obvious unsafe-wrapping. That itself has been mostly split out to #735, modulo one unsafe block that turned out to be required after #737.