-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Clean up transmute lints' documentation #15710
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
base: master
Are you sure you want to change the base?
Conversation
1. Converted example calls of `transmute` from `core::intrinsics::transmute` to `std::mem::transmute`. 2. Eliminated caveat for `transmute_ptr_to_ref`, since dereferencing raw pointers in const contexts is now stable: rust-lang/rust#51911 3. Added missing `unsafe` block for `transmute_ptr_to_ref`'s example 4. Added more context to `useless_transmute`'s example 5. Added a missing space in a doc comment
3455f2e to
5d6e7c4
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.
Documentation changes don't need changelog lines in the PR description.
| /// ### Known problems | ||
| /// - `mem::transmute` in statics and constants is stable from Rust 1.46.0, | ||
| /// while dereferencing raw pointer is not stable yet. | ||
| /// If you need to do this in those places, | ||
| /// you would have to use `transmute` instead. | ||
| /// |
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.
The lint still doesn't take into account whether it's in a constant context. It would also need an MSRV check to fix this issue.
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.
Wait, I'm not sure I understand 100%.
If it doesn't check whether it's in a const context or not, but now it works properly in a const context, then this problem is effectively fixed, no?
I'm guessing this is what you mean about the MSRV, perhaps.
Though OTOH maybe the documentation could mention that "for rust versions prior to X release, this lint will have [this known issue] due to the fact that dereferencing raw pointers wasn't stable before then.". Would that be appropriate?
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.
Yes. It only works correctly if the MSRV is high enough to include const raw pointer dereferences.
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.
Ping @felix91gr. Just waiting on this. *const pointers can be dereferenced as of 1.58, *mut as of 1.83.
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.
@Jarcho ty for the ping. I got taken away for a bit. Where do I specify this? I think I've never have added a MSRV in Clippy before.
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.
For documenting it just keep it in the "known problems" section.
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.
@Jarcho so then, if I got you correctly, the only change needed to correct this is of documentation, right? I thought you meant that a MSRV config should be added to differentiate between all the cases.
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.
Either would be an acceptable PR. Actually adding the code to handle the MSRV would be the better fix.
If you want a to actually fix the issue you'll need to add the msrv to clippy_utils/src/msrvs.rs. These are named by feature so you'll add something like CONST_RAW_CONST_PTR_DEREF and CONST_RAW_MUT_PTR_DEREF. You can use is_in_const_context to check for constness.
Everything else is already done since the lint has an MSRV check for the suggestion.
Oh thank god, this is so much better! Thx, Jason! |
transmutefromcore::intrinsics::transmutetostd::mem::transmute.transmute_ptr_to_ref, since dereferencing raw pointers in const contexts is now stable:[tracking issue] dereferencing raw pointers inside constants (const_raw_ptr_deref) rust#51911
unsafeblock fortransmute_ptr_to_ref's exampleuseless_transmute's examplechangelog: none