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

fix some clippy suggestions #20

Merged
merged 2 commits into from
Feb 5, 2022
Merged

Conversation

hellow554
Copy link
Contributor

This contains mostly using of cast instead of as *const _ and the second thing is using the addr_of! macro for easier reading and avoiding possible UB (which isn't the case in any of the places I replaced the code).

@fpagliughi
Copy link
Owner

Hey, thanks so much for the PR!

Unfortunately, I don't think I can merge it as-is right now. I wasn't clear about the Minimum Supported Rust Version on the README, but I personally still need to support rustc v1.46 for a production system. (Sorry, that's my mistake). And I wouldn't want to push the required version forward just for readability issues.

It looks like addr_of_mut!() requires v1.51. Most of the other stuff looks good, and useful, so maybe we can split the difference and get most of these changes in.

BTW: What clippy lint did you use? My defaults didn't catch most of these things.

@hellow554
Copy link
Contributor Author

I typically use cargo clippy -- -W clippy::pedantic -W clippy::nursery.
It gives a lot of warnings, especially docs, #[must_use] and const related stuff. But you can "disable" those with -A clippy::must_use_candidate for example. Give it a try ;)

I added a clippy toml and added the msrv of 1.46.0 as well as removed the addr_of! macro. Hope this is better :)

@fpagliughi
Copy link
Owner

Yeah. That sounds great. I will have a look over the weekend and get it merged. Thanks again!

@fpagliughi fpagliughi merged commit 1c799d0 into fpagliughi:master Feb 5, 2022
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