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

False positive in cast_ptr_alignment #5618

Closed
djugei opened this issue May 19, 2020 · 7 comments · Fixed by #5667
Closed

False positive in cast_ptr_alignment #5618

djugei opened this issue May 19, 2020 · 7 comments · Fixed by #5667
Assignees
Labels
good-first-issue These issues are a good way to get started with Clippy

Comments

@djugei
Copy link
Contributor

djugei commented May 19, 2020

if i cast a highly-aligned pointer to u8 to a mid-aligned pointer (in my case its 4096 -> 1 -> 8) clippy errors. this is wrong. there are many cases where casting from u8 is acceptable alignment-wise, including when you have manually checked that the alignment works out, statically or dynamically.

@flip1995
Copy link
Member

Do you have a code example for this?

@djugei
Copy link
Contributor Author

djugei commented May 19, 2020

@flip1995
Copy link
Member

flip1995 commented May 19, 2020

So a minimal example would like this:

let _ = (ptr as *const u8) as *const u64;

This may be ok for align_of(ptr) >= 8byte, which could be fixed for the example above. But then again, why would you cast to u8 first?

including when you have manually checked that the alignment works out, statically or dynamically.

In (special) cases where you disagree with a lint, or have manually checked that a lint is not applicable, you should use allow(clippy::lint) to signal that the following code is really intended.

IMO casts from u8 pointer is not a FP in general, but a special case where a cast to a more strictly aligned pointer is fine ➡️ allow.

@djugei
Copy link
Contributor Author

djugei commented May 19, 2020

why would you cast to u8 first

because i want to add an offset in bytes.

i honestly think this lint should be completely removed, clippy is not equiped to deal with this situation.
you can't really guess at the validity of pointer casts from a simple syntax analysis. i feel like this job is better suited for miri.

im reasonably sure that this lint will also error on the standard library, for example in slice

@flip1995
Copy link
Member

you can't really guess at the validity of pointer casts from a simple syntax analysis

That's a fair point. I don't think we should remove it completely, but rather move it to pedantic

@flip1995 flip1995 added the S-needs-discussion Status: Needs further discussion before merging or work can be started label May 20, 2020
@djugei
Copy link
Contributor Author

djugei commented May 31, 2020

i am not sure what further discussion is needed, i will open a pr.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy and removed S-needs-discussion Status: Needs further discussion before merging or work can be started labels May 31, 2020
@flip1995
Copy link
Member

Thanks! I marked it as need-discussion, to have it over for some more opinions. But this was 10 days ago, so I think moving this is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants