Skip to content

useless conversion wrongly suggests removing .into_iter() when ExactSizeIterator is required #11300

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

Closed
asomers opened this issue Aug 6, 2023 · 1 comment · Fixed by #11301
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@asomers
Copy link

asomers commented Aug 6, 2023

Summary

I have a function that takes an argument implementing IntoIterator + ExactSizeIterator. When I call it, I supply an array with .into_iter(). Clippy wrongly suggests removing the .into_iter() on the grounds that it's automatic. However, if I do that then the code fails to compile because ExactSizeIterator is not satisfied.

Lint Name

useless_conversion

Reproducer

I tried this code:

pub fn foo<I>(i: I)
where I: IntoIterator<Item=i32> + ExactSizeIterator
{
    assert_eq!(i.len(), 3);
}

pub fn bar() {
    foo([1, 2, 3].into_iter());
}

I saw this happen:

warning: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
 --> src/lib.rs:8:9
  |
8 |     foo([1, 2, 3].into_iter());
  |         ^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `[1, 2, 3]`
  |
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
 --> src/lib.rs:2:10
  |
2 | where I: IntoIterator<Item=i32> + ExactSizeIterator
  |          ^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
  = note: `#[warn(clippy::useless_conversion)]` on by default

I expected to see this happen:
No errors.

If I apply Clippy's suggest and remove the .into_iter(), then this happens:

error[[E0277]](https://doc.rust-lang.org/stable/error_codes/E0277.html): the trait bound `[i32; 3]: ExactSizeIterator` is not satisfied
 --> src/lib.rs:8:9
  |
8 |     foo([1, 2, 3]);
  |     --- ^^^^^^^^^ the trait `ExactSizeIterator` is not implemented for `[i32; 3]`
  |     |
  |     required by a bound introduced by this call
  |
  = help: the following other types implement trait `ExactSizeIterator`:
            &mut I
            Args
            ArgsOs
            ArrayChunksMut<'_, T, N>
            ArrayWindows<'_, T, N>
            Box<I, A>
            Chunks<'_, T>
            ChunksExact<'_, T>
          and 109 others
note: required by a bound in `foo`
 --> src/lib.rs:2:35
  |
1 | pub fn foo<I>(i: I)
  |        --- required by a bound in this function
2 | where I: IntoIterator<Item=i32> + ExactSizeIterator
  |                                   ^^^^^^^^^^^^^^^^^ required by this bound in `foo`

Version

rustc 1.73.0-nightly (399b06823 2023-07-20)
binary: rustc
commit-hash: 399b068235ceea440540539b3bfd1aeb82214a28
commit-date: 2023-07-20
host: x86_64-unknown-freebsd
release: 1.73.0-nightly
LLVM version: 16.0.5

Additional Labels

@rustbot label +I-suggestion-causes-error

@asomers asomers added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Aug 6, 2023
@rustbot
Copy link
Collaborator

rustbot commented Aug 6, 2023

Error: Label I-suggestion-causes-error can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

asomers added a commit to asomers/fsx-rs that referenced this issue Aug 6, 2023
asomers added a commit to asomers/fsx-rs that referenced this issue Aug 6, 2023
asomers added a commit to asomers/fsx-rs that referenced this issue Aug 6, 2023
@Centri3 Centri3 added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Aug 6, 2023
bors added a commit that referenced this issue Sep 5, 2023
[`useless_conversion`]: don't lint if type parameter has unsatisfiable bounds for `.into_iter()` receiver

Fixes #11300.

Before this PR, clippy assumed that if it sees a `f(x.into_iter())` call and the type at that argument position is generic over any `IntoIterator`, then the `.into_iter()` call must be useless because `x` already implements `IntoIterator`, *however* this assumption is not right if the generic parameter has more than just the `IntoIterator` bound (because other traits can be implemented for the IntoIterator target type but not the IntoIterator implementor, as can be seen in the linked issue: `<[i32; 3] as IntoIterator>::IntoIter` satisfies `ExactSizeIterator`, but `[i32; 3]` does not).

So, this PR makes it check that the type parameter only has a single `IntoIterator` bound. It *might* be possible to check if the type of `x` in `f(x.into_iter())` satisfies all the bounds on the generic type parameter as defined on the function (which would allow removing the `.into_iter()` call even with multiple bounds), however I'm not sure how to do that, and the current fix should always work.

**Edit:** This PR has been changed to check if any of the bounds don't hold for the type of the `.into_iter()` receiver, so we can still lint in some cases.

changelog: [`useless_conversion`]: don't lint `.into_iter()` if type parameter has multiple bounds
@bors bors closed this as completed in 59636a2 Sep 16, 2023
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-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
3 participants