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

fewer clippy warnings #11

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fewer clippy warnings #11

wants to merge 3 commits into from

Conversation

andrewgazelka
Copy link
Contributor

Background

  • fewer clippy warnings when using macros

Description

  • add #[must_use] and #[allow] as appropriate

Verification

  • clippy with firmware params works

Copy link
Contributor

@JarredAllen JarredAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change; also I suspect the issue you were having with missing safety blocks is called by instances of unsafe in the macros that don't have safety comments (Ctrl+F for "unsafe" in lib.rs to find them, there's a few and GitHub won't let me make a comment on a line of code that wasn't changed in this PR).

src/lib.rs Show resolved Hide resolved
impl $name {
const VARIANT_COUNT: usize = $crate::enum_impl!(COUNT $fst_field $(,$field)*);
}

#[allow(clippy::undocumented_unsafe_blocks, clippy::use_self)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to document these with a safety comment than put a clippy allowance (unless you tried that and it didn't work?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't seem to work for some odd reason. If you want to allocate time to try it and it works for you, I wouldn't mind. It seemed clippy::use_self also had false positives—I couldn't find anywhere I could use Self, but I wouldn't be surprised if I am just dumb.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mainly referring to the clippy::undocumented_unsafe_blocks allowance, which you should be able to resolve by putting a safety comment on this unsafe impl and the others like it.

Also, with the clippy::use_self, it seems like you missed the spot where the usage is happening because I'm still seeing that lint trigger with the allowances you added in.

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