rust-lang / rust-lang/rust-clippy
Structs with non-matching Index and Iter impls cause bad suggestions
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 13.5k
- Forks
- 2.2k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 32
Description
I'm using the llvm-alt library, which exports a Function type representing a function embedded in an llvm module. Calling iter on a function iterates through its basic blocks, but indexing into it accesses the function parameter values.
Yes, this is a bizarre design decision, but it's a wart on an otherwise great library.
Under this setup I wrote the following code:
for i in 0..llvm_f.get_signature().get_params().len() {
do_something(llvm_f[i]);
}
When I run clippy on my crate, I get the following diagnostic:
warning: the loop variable `i` is only used to index `llvm_f`.
--> src/codegen/function.rs:36:14
|
36 | for i in 0..llvm_f.get_signature().get_params().len() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.207/index.html#needless_range_loop
help: consider using an iterator
|
36 | for <item> in llvm_f.iter().take(llvm_f.get_signature().get_params().len()) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This warning and suggestion do not make sense, since the iter implementation would do something totally different. (Iterate through basic blocks.) Now I know this isn't a common case, or even a useful one. But it would be nice if Clippy didn't produce this spurious diagnostic. (This is bad library code, not badness in my code.)
I think this could probably be avoided if Clippy just compared the existing Iterator and Index implementations on the type, and made sure they both operated on the same kind of value. I don't know how hard this would be to implement. I can have a go at it myself if someone points me in the right direction.
Obviously this isn't a very important bug, but it appears to technically be a spurious diagnostic, so I thought I'd report it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Locate the needless_range_loop lint and its existing tests, then reproduce the case where a type's Index and Iterator implementations refer to different values. Verify that the diagnostic does not suggest replacing the range loop with that iterator when the implementations do not match.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100