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

Structs with non-matching Index and Iter impls cause bad suggestions #2847

Open
Others opened this issue Jun 15, 2018 · 1 comment
Open

Structs with non-matching Index and Iter impls cause bad suggestions #2847

Others opened this issue Jun 15, 2018 · 1 comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@Others
Copy link

Others commented Jun 15, 2018

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.

@oli-obk oli-obk added the C-bug Category: Clippy is not doing the correct thing label Jun 15, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jun 15, 2018

Thanks for the bug report. This suggestion is indeed wrong. We'll try to figure out a way to not lint in such cases. For now just disable that specific lint

@phansch phansch added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Apr 19, 2019
@phansch phansch added the L-suggestion Lint: Improving, adding or fixing lint suggestions label Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

No branches or pull requests

3 participants