Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Use slice::iter instead of into_iter to avoid future breakage #208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

martin-fink
Copy link

Use slice::iter instead of into_iter to avoid future breakage

an_array.into_iter() currently just works because of the autoref
feature, which then calls <[T] as IntoIterator>::into_iter. But
in the future, arrays will implement IntoIterator, too. In order
to avoid problems in the future, the call is replaced by iter()
which is shorter and more explicit.

A crater run showed that your crate is affected by a potential future
change. See rust-lang/rust#65819 for more information.

`an_array.into_iter()` currently just works because of the autoref
feature, which then calls `<[T] as IntoIterator>::into_iter`. But
in the future, arrays will implement `IntoIterator`, too. In order
to avoid problems in the future, the call is replaced by `iter()`
which is shorter and more explicit.
@tafia
Copy link
Contributor

tafia commented Nov 2, 2019

i think the into_iter are legit on the tests (vecs must be consumed as they are not used later).

@martin-fink
Copy link
Author

You are right, thanks!

@AtheMathmo
Copy link
Owner

I'm afraid I've been away from the rust ecosystem to understand this issue without digging deeper. Should this PR be closed following @tafia 's comment?

@LukasKalbertodt
Copy link

Hi there! This PR is indeed not necessary. The code changed in this PR does not lead to breakage as into_iter is called on slices, not arrays. While the change is not bad (iter() is shorter and does the same), it's not necessary.

rusty-machine regressed in our tests because of the use of matrix! from rulinalg (build log). There is already an open PR fixing the incorrect code in rulinalg: AtheMathmo/rulinalg#200
As this seems to be also your repository, it should be fine with getting it merged?

@AtheMathmo In short: close this PR, but merge this one.

Also: please consider Rust Bus to aid with maintaining your two crates. They do maintaining for you and will find a new maintainer.

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

Successfully merging this pull request may close these issues.

4 participants