Use clippy #276
Use clippy #276
Conversation
since these functions are taking args coming from the command line, i dont think this is actually a valid lint, morally the arguments are all from one structure
this linter is buggy when the declared lifetime is used for another type constraint.
| relax_branch(&mut cur, offset, dest_offset, &encinfo, isa); | ||
| continue; | ||
| } | ||
| // This coubd be an out-of-range branch. |
| /// Deals with a Wasm instruction located in an unreachable portion of the code. Most of them | ||
| /// are dropped but special ones like `End` or `Else` signal the potential end of the unreachable | ||
| /// portion so the translation state muts be updated accordingly. | ||
| fn translate_unreachable_operator( | ||
| op: Operator, | ||
| op: &Operator, |
There was a problem hiding this comment.
I'm curious; which clippy lint prompted this change?
There was a problem hiding this comment.
check-clippy.sh
Outdated
| echo "* Please install clippy to lint rust code. *" | ||
| echo "********************************************************************" | ||
| echo "$0 --install" | ||
| sleep 1 |
There was a problem hiding this comment.
With the PR as it currently stands, we don't install clippy for Travis runs, so it seems like the code here should be a little less noisy, and not do "sleep 1". I think we're ok if someone doesn't have clippy installed.
There was a problem hiding this comment.
Reduced this to one-line echo.
lib/cretonne/src/lib.rs
Outdated
| assign_op_pattern, | ||
| unreadable_literal, | ||
| empty_line_after_outer_attr, | ||
| // From here on i think theyre solvable: |
There was a problem hiding this comment.
theyre -> they're. But also, I think it's useful to elaborate on the comment here. If these lints are solvable with reasonable effort, they could make good starter projects.
lib/cretonne/src/ir/function.rs
Outdated
| @@ -141,6 +141,8 @@ impl Function { | |||
| } | |||
|
|
|||
| /// Return an object that can display this function with correct ISA-specific annotations. | |||
| // Needless lifetimes lint false positive | |||
| #[cfg_attr(feature = "cargo-clippy", allow(needless_lifetimes))] | |||
There was a problem hiding this comment.
It feels like needless_lifetimes has enough false positives that it'd be better to disable this lint globally than clutter up the code disabling it locally every time.
remove overrides at the false positives
|
I fixed up the remaining trivial lints, documented the reasons for each lint that is currently |
|
Great, thanks for getting this infrastructure set up! |
This is a start at using clippy throughout Cretonne.
Current issues:
cargo +nightly clippyworks - stable cargo and clippy has a bug that seems to be related to the way arguments are passed when used in the root directory (works in all the other crates)Currently, there are a ton of lints turned off in the cretonne crate. A significant portion of them are fixable without much effort, but others would require doing work on the code generator.
Unfortunately, because generated code is made part of other source files via
include!(), I wasn't put top level clippy pragmas in the generated sources only, because it wouldn't be at the top of the source file.Fixes #264