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

fix rust 2024 breakage with cargo v1 action #144

Merged
merged 1 commit into from
May 22, 2024

Conversation

cosmicexplorer
Copy link

master is broken, seemingly by a change in the cargo action: https://github.com/zip-rs/zip2/actions/runs/9185032790/job/25258374442

/home/runner/.cargo/bin/cargo fuzz cmin --all-features fuzz_write fuzz/corpus/fuzz_write -- fuzz/corpus/new_seed
   Compiling zip-fuzz v0.0.0 (/home/runner/work/zip2/zip2/fuzz)
error: this method call resolves to `<&Box<[T]> as IntoIterator>::into_iter` (due to backwards compatibility), but will resolve to `<Box<[T]> as IntoIterator>::into_iter` in Rust 2024
Error:    --> fuzz_targets/fuzz_write.rs:134:52
    |
134 |     for (operation, abort) in test_case.operations.into_iter() {
    |                                                    ^^^^^^^^^
    |
    = warning: this changes meaning in Rust 2024
    = note: `-D boxed-slice-into-iter` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(boxed_slice_into_iter)]`
help: use `.iter()` instead of `.into_iter()` to avoid ambiguity
    |
134 |     for (operation, abort) in test_case.operations.iter() {
    |                                                    ~~~~
help: or remove `.into_iter()` to iterate by value
    |
134 -     for (operation, abort) in test_case.operations.into_iter() {
134 +     for (operation, abort) in test_case.operations {
    |

error: could not compile `zip-fuzz` (bin "fuzz_write") due to 1 previous error

This is breaking CI on other PRs. I believe this change produces the correct behavior. We could also simply change to .iter(), but this PR retains the current behavior of consuming the boxed slice.

@Pr0methean Pr0methean added this pull request to the merge queue May 22, 2024
Merged via the queue into zip-rs:master with commit c625554 May 22, 2024
38 checks passed
@Pr0methean
Copy link
Member

The Rust source says the current behavior is actually to implicitly borrow the slice and call into_iter() on the borrow, so it doesn't actually consume the slice. Which means the Rust 2024 new behavior is what I wanted -- and thought we already had -- all along.

@Pr0methean
Copy link
Member

The upstream discussion is here:

rust-lang/rust#59878

@Pr0methean
Copy link
Member

Pr0methean commented May 23, 2024

The reason for the old behavior AFAICT was so that e.g. [1,2,3][..].into_iter() would work as expected and not try to move anything out of the constant. I would've preferred if they just used iter() instead in for-loops when necessary, and teaching the programmer to call the right method when explicitly creating an Iterator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants