arch: fix clippy warnings #4117
Conversation
These only happen when a specific target is selected.
This uses the configured target to check more code.
| @cd boards/nordic/nrf52840dk && cargo clippy -- -D warnings | ||
| @cd boards/hifive1 && cargo clippy -- -D warnings |
There was a problem hiding this comment.
I don't like this, I feel like this is destined to go out of sync. That being said, I don't have a better idea. This is, again, caused by conditional compilation, and so I don't have a better idea that does not involve adding a new #[cfg(any(clippy, ... -- for which I know there will be opposition (#4059 (comment)).
There was a problem hiding this comment.
Per usual there is a fix but it's not stable https://doc.rust-lang.org/cargo/reference/unstable.html#per-package-target
There was a problem hiding this comment.
I tried out that flag and it seems to be everything I want. cargo clippy and cargo check just work as expected, and cargo test fails because we don't have any way to test on an embedded device.
There was a problem hiding this comment.
I presume it would totally break hail on stable, as it would depend on the cortex-m crates and those can't then be compiled with a stable Cargo?
There was a problem hiding this comment.
Yes.
❯ make
error: failed to load manifest for workspace member `/Users/bradjc/git/tock/arch/cortex-m`
referenced by workspace at `/Users/bradjc/git/tock/Cargo.toml`
Caused by:
failed to parse manifest at `/Users/bradjc/git/tock/arch/cortex-m/Cargo.toml`
Caused by:
the cargo feature `per-package-target` requires a nightly version of Cargo, but this is the `stable` channel
See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.
See https://doc.rust-lang.org/cargo/reference/unstable.html#per-package-target for more information about using this feature.
make: *** [/Users/bradjc/git/tock/target/thumbv7em-none-eabi/release/hail] Error 101
```.
Pull Request Overview
For reasons, clippy does not fully check our arch crates. Once #4075 lands, running
cargo clippyin a board directly will check more code than we are today, and this fixes the new warnings that come up.This also adds two fairly arbitrarily selected boards to the clippy CI to automate these checks somewhat.
Testing Strategy
Running
cargo clippyon #4075TODO or Help Wanted
I picked one cortex-m board and one risc-v board. I'm not sure we want to run all boards in CI. Maybe this is good enough? Thoughts?
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.