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

Clippy --fix panics with index error #8734

Closed
tvallotton opened this issue Apr 22, 2022 · 8 comments · Fixed by #9071
Closed

Clippy --fix panics with index error #8734

tvallotton opened this issue Apr 22, 2022 · 8 comments · Fixed by #9071
Labels
C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@tvallotton
Copy link

tvallotton commented Apr 22, 2022

Summary

cargo clippy --fix causes an index error.

Reproducer

I pushed the code that causes the error to this repo, so you can reproduce it. The error was triggerd by cargo clippy --fix. Note that this does not seem to panic in stable.

thread 'main' panicked at 'slice index starts at 1 but ends at 0', library/core/src/slice/index.rs:91:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: could not compile `chess`
warning: build failed, waiting for other jobs to finish...
thread 'main' panicked at 'slice index starts at 1 but ends at 0', library/core/src/slice/index.rs:91:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: could not compile `chess`

Version

rustc 1.61.0-nightly (76d770ac2 2022-04-02)
binary: rustc
commit-hash: 76d770ac21d9521db6a92a48c7b3d5b2cc535941
commit-date: 2022-04-02
host: aarch64-apple-darwin
release: 1.61.0-nightly
LLVM version: 14.0.0

Additional Labels

No response

@tvallotton tvallotton added the C-bug Category: Clippy is not doing the correct thing label Apr 22, 2022
@tvallotton
Copy link
Author

it looks like its related to #8718.

@xFrednet xFrednet added the I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️ label Apr 22, 2022
@xFrednet
Copy link
Member

Hey, thank you for the report. This looks like the linked issue. Could you provide the Stacktrace, by setting RUST_BACKTRACE=1? 🙃

@tvallotton
Copy link
Author

tvallotton commented Apr 22, 2022

You can also try to replicate it yourself on this repo. I get this:

thread 'main' panicked at 'slice index starts at 1 but ends at 0', library/core/src/slice/index.rs:91:5
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::slice::index::slice_index_order_fail_rt
   3: core::ops::function::FnOnce::call_once
   4: core::intrinsics::const_eval_select
   5: core::slice::index::slice_index_order_fail
   6: rustfix::parse_snippet
   7: rustfix::collect_span
   8: <alloc::vec::Vec<rustfix::Replacement> as alloc::vec::spec_from_iter::SpecFromIter<rustfix::Replacement, core::iter::adapters::filter_map::FilterMap<core::iter::adapters::filter::Filter<core::slice::iter::Iter<rustfix::diagnostics::DiagnosticSpan>, rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1}::{closure#0}>, rustfix::collect_span>>>::from_iter
   9: <&mut rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1} as core::ops::function::FnMut<(&rustfix::diagnostics::Diagnostic,)>>::call_mut
  10: <alloc::vec::Vec<rustfix::Solution> as alloc::vec::spec_from_iter::SpecFromIter<rustfix::Solution, core::iter::adapters::filter_map::FilterMap<core::slice::iter::Iter<rustfix::diagnostics::Diagnostic>, rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1}>>>::from_iter
  11: rustfix::collect_suggestions::<std::collections::hash::map::RandomState>
  12: cargo::ops::fix::fix_maybe_exec_rustc
  13: cargo::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: could not compile `chess`
warning: build failed, waiting for other jobs to finish...
thread 'main' panicked at 'slice index starts at 1 but ends at 0', library/core/src/slice/index.rs:91:5
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::slice::index::slice_index_order_fail_rt
   3: core::ops::function::FnOnce::call_once
   4: core::intrinsics::const_eval_select
   5: core::slice::index::slice_index_order_fail
   6: rustfix::parse_snippet
   7: rustfix::collect_span
   8: <alloc::vec::Vec<rustfix::Replacement> as alloc::vec::spec_from_iter::SpecFromIter<rustfix::Replacement, core::iter::adapters::filter_map::FilterMap<core::iter::adapters::filter::Filter<core::slice::iter::Iter<rustfix::diagnostics::DiagnosticSpan>, rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1}::{closure#0}>, rustfix::collect_span>>>::from_iter
   9: <&mut rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1} as core::ops::function::FnMut<(&rustfix::diagnostics::Diagnostic,)>>::call_mut
  10: <alloc::vec::Vec<rustfix::Solution> as alloc::vec::spec_from_iter::SpecFromIter<rustfix::Solution, core::iter::adapters::filter_map::FilterMap<core::slice::iter::Iter<rustfix::diagnostics::Diagnostic>, rustfix::collect_suggestions<std::collections::hash::map::RandomState>::{closure#1}>>>::from_iter
  11: rustfix::collect_suggestions::<std::collections::hash::map::RandomState>
  12: cargo::ops::fix::fix_maybe_exec_rustc
  13: cargo::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: could not compile `chess`

@Serial-ATA
Copy link
Contributor

Looks like it's coming from the map_flatten lint here: https://github.com/tvallotton/chess/blob/98b01208e95173388cf4f781cc83bb414bb3bf55/src/table.rs#L73

It doesn't like the long multiline suggestions.

For example, this works:

[0u8, 1, 2, 3]
    .into_iter()
    .map(|n| match n {
        1 => [n.saturating_add(1)],
        n => [n],
    })
    .flatten();

While this causes the panic:

[0u8, 1, 2, 3]
    .into_iter()
    .map(|n| match n {
        1 => [n
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)
            .saturating_add(1)],
        n => [n],
    })
    .flatten();

Works just fine in stable, so it's likely #8520

@Serial-ATA
Copy link
Contributor

Serial-ATA commented Apr 23, 2022

Ah, just read into it. Looks like the real issue is rust-lang/rustfix#141. ...or is it? I'll have to look into it more.

@Serial-ATA
Copy link
Contributor

This is actually a Cargo issue. rust-lang/rustfix@635eef7 didn't make it into rustfix 0.6.0, which Cargo depends on. Just have to wait for a new release.

@droogmic
Copy link

I ran into this as well, I see we are waiting for an answer on rust-lang/rustfix#211.

@hellow554
Copy link
Contributor

The culprit should be solved by updating rustfix to 0.6.1, but @Serial-ATA discovered a very interesting case:

fn issue8734() {
    let _ = [0u8, 1, 2, 3]
        .into_iter()
        .map(|n| match n {
            1 => [n
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)],
            n => [n],
        })
        .flatten();
}

gets autocorrect to

fn issue8734() {
    let _ = [0u8, 1, 2, 3]
        .into_iter()
        .flat_map(|n| match n {
            1 => [n
                .saturating_add(1)
            1 => [n
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)
                .saturating_add(1)],
            n => [n],
        });
}

which is weird and incorrect.

Let's investigate this further

bors added a commit that referenced this issue May 24, 2022
Add some testcases for recent rustfix update

changelog: none

This adds a testcase for a bugfix that has been fixed by https://github.com/rust-lang/rustfix/tree/v0.6.1

`rustfix` is pulled in by `compiletest_rs`. So to test that the correct rustfix version is used, I added one (and a half) testcase.

I tried to add a testcase for #8734 as well, but interesting enough the rustfix is wrong:

```diff
 fn issue8734() {
     let _ = [0u8, 1, 2, 3]
         .into_iter()
-        .and_then(|n| match n {
+        .flat_map(|n| match n {
+            1 => [n
+                .saturating_add(1)
             1 => [n
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)
                 .saturating_add(1)],
             n => [n],
         });
 }
```

this needs some investigation and then this testcase needs to be enabled by commenting it out

closes #8878
related to #8734
Alexendoo added a commit to Alexendoo/rust-clippy that referenced this issue Jun 30, 2022
@bors bors closed this as completed in 4198013 Jun 30, 2022
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-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants