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

Tracking Issue for edition-dependent IntoIterator for arrays #84513

Open
24 of 28 tasks
m-ou-se opened this issue Apr 24, 2021 · 10 comments
Open
24 of 28 tasks

Tracking Issue for edition-dependent IntoIterator for arrays #84513

m-ou-se opened this issue Apr 24, 2021 · 10 comments
Labels
A-array Area: `[T; N]` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-edition-2021 Area: The 2021 edition A-iterators Area: Iterators C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Apr 24, 2021

Once edition 2021 is stable:

  • Update examples like this to use arrays by value. (Some(&1)Some(1) etc.)
    • Update examples to no longer avoid iterating arrays. (Many doctest examples are using iterators over e.g. &i32 instead of just i32.)
  • Remove about 200 usages of .iter().copied() and .iter().cloned() from examples and tests.

Unresolved questions:

  • Do something about the 'copying large arrays' footgun. (Add IntoIterator implementation for Box<[T; N]> etc? Add lint about big copies?)
@m-ou-se m-ou-se added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-iterators Area: Iterators labels Apr 24, 2021
@cuviper
Copy link
Member

cuviper commented Apr 24, 2021

Add IntoIterator implementation for Box<[T; N]> etc?

I think it would be great if that could produce vec::IntoIter<T>, but it gave me an upstream coherence error when I tried in #59878 (comment), core being upstream of alloc.

     = note: conflicting implementation in crate `core`:
             - impl<I> IntoIterator for I
               where I: Iterator;
     = note: upstream crates may add a new impl of trait `core::iter::Iterator` for type `[_; _]` in future versions

@mominul
Copy link
Contributor

mominul commented May 20, 2021

I'd like to tackle the standard library changes first!
@rust-highfive assign

@weihanglo
Copy link
Member

Also suggest just removing .into_iter() in for a in [1, 2, 3].into_iter(), since that now works on all editions.

Since for loop are converted into loop in HIR, is it still possible implementing this as a late pass lint rule?

@k0pernicus
Copy link
Contributor

I could work on updating the array_into_iter lint.
However, as I never touched to this part before I may have some questions later...

@nikomatsakis
Copy link
Contributor

I'm going to break this issue apart into sub-issues to help in people signing up for specific bits!

@m-ou-se
Copy link
Member Author

m-ou-se commented May 24, 2021

I'd like to tackle the standard library changes first!

@mominul Which changes specifically do you mean? Have you started working on these?

@mominul
Copy link
Contributor

mominul commented May 24, 2021

@m-ou-se I haven't started yet, but I intend to work on the Update standard library checklist items:

  • Remove usages of array::IntoIter::new.
  • Update examples to no longer avoid iterating arrays. (Many doctest examples are using iterators over e.g. &i32 instead of just i32.)
  • Update expressions like like .chain(array.iter().cloned()) to just .chain(array) since that now works on all editions. (Same for zip, extend etc.)
  • Use .extend(array) and from_iter(array) wherever that simplifies things.
  • Remove arrays do not yet implement IntoIterator message from Iterator trait.

@m-ou-se
Copy link
Member Author

m-ou-se commented May 25, 2021

Thanks! Since we're running a bit short on time for the edition changes, I'm implementing the most important ones today. Hope you don't mind. I'll add the PR numbers in the first comment above, so take a look there before you start implementing anything to avoid doing work twice.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 26, 2021
Remove arrays/IntoIterator message from Iterator trait.

Arrays implement IntoIterator since 1.53.

cc rust-lang#84513
@mominul
Copy link
Contributor

mominul commented May 26, 2021

@m-ou-se Thanks, I will! 😊

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 26, 2021
Remove Iterator #[rustc_on_unimplemented]s that no longer apply.

Now that `IntoIterator` is implemented for arrays, all the `rustc_on_unimplemented` for arrays of ranges (e.g. `for _ in [1..3] {}`) no longer apply, since they are now valid Rust.

Separated these from rust-lang#85670, because we should discuss a potential new (clippy?) lint for these.

Until Rust 1.52, `for _ in [1..3] {}` produced:

```
error[E0277]: `[std::ops::Range<{integer}>; 1]` is not an iterator
 --> src/main.rs:2:14
  |
2 |     for _ in [1..3] {}
  |              ^^^^^^ if you meant to iterate between two values, remove the square brackets
  |
  = help: the trait `std::iter::Iterator` is not implemented for `[std::ops::Range<{integer}>; 1]`
  = note: `[start..end]` is an array of one `Range`; you might have meant to have a `Range` without the brackets: `start..end`
  = note: required by `std::iter::IntoIterator::into_iter`
```

But in Rust 1.53 and later, it compiles fine. It iterates over the array by value, for one iteration with the element `1..3`.

This is probably a mistake, which is no longer caught. Should we have a lint for it? Should Clippy have a lint for it?

cc `@estebank` `@flip1995`

cc rust-lang#84513
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 27, 2021
Remove Iterator #[rustc_on_unimplemented]s that no longer apply.

Now that `IntoIterator` is implemented for arrays, all the `rustc_on_unimplemented` for arrays of ranges (e.g. `for _ in [1..3] {}`) no longer apply, since they are now valid Rust.

Separated these from rust-lang#85670, because we should discuss a potential new (clippy?) lint for these.

Until Rust 1.52, `for _ in [1..3] {}` produced:

```
error[E0277]: `[std::ops::Range<{integer}>; 1]` is not an iterator
 --> src/main.rs:2:14
  |
2 |     for _ in [1..3] {}
  |              ^^^^^^ if you meant to iterate between two values, remove the square brackets
  |
  = help: the trait `std::iter::Iterator` is not implemented for `[std::ops::Range<{integer}>; 1]`
  = note: `[start..end]` is an array of one `Range`; you might have meant to have a `Range` without the brackets: `start..end`
  = note: required by `std::iter::IntoIterator::into_iter`
```

But in Rust 1.53 and later, it compiles fine. It iterates over the array by value, for one iteration with the element `1..3`.

This is probably a mistake, which is no longer caught. Should we have a lint for it? Should Clippy have a lint for it?

cc ``@estebank`` ``@flip1995``

cc rust-lang#84513
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 27, 2021
Remove Iterator #[rustc_on_unimplemented]s that no longer apply.

Now that `IntoIterator` is implemented for arrays, all the `rustc_on_unimplemented` for arrays of ranges (e.g. `for _ in [1..3] {}`) no longer apply, since they are now valid Rust.

Separated these from rust-lang#85670, because we should discuss a potential new (clippy?) lint for these.

Until Rust 1.52, `for _ in [1..3] {}` produced:

```
error[E0277]: `[std::ops::Range<{integer}>; 1]` is not an iterator
 --> src/main.rs:2:14
  |
2 |     for _ in [1..3] {}
  |              ^^^^^^ if you meant to iterate between two values, remove the square brackets
  |
  = help: the trait `std::iter::Iterator` is not implemented for `[std::ops::Range<{integer}>; 1]`
  = note: `[start..end]` is an array of one `Range`; you might have meant to have a `Range` without the brackets: `start..end`
  = note: required by `std::iter::IntoIterator::into_iter`
```

But in Rust 1.53 and later, it compiles fine. It iterates over the array by value, for one iteration with the element `1..3`.

This is probably a mistake, which is no longer caught. Should we have a lint for it? Should Clippy have a lint for it?

cc ```@estebank``` ```@flip1995```

cc rust-lang#84513
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 5, 2021
Update standard library for IntoIterator implementation of arrays

This PR partially resolves issue rust-lang#84513 of updating the standard library part.

I haven't found any remaining doctest examples which are using iterators over e.g. &i32 instead of just i32 in the standard library. Can anyone point me to them if there's remaining any?

Thanks!

r? `@m-ou-se`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 5, 2021
Update standard library for IntoIterator implementation of arrays

This PR partially resolves issue rust-lang#84513 of updating the standard library part.

I haven't found any remaining doctest examples which are using iterators over e.g. &i32 instead of just i32 in the standard library. Can anyone point me to them if there's remaining any?

Thanks!

r? ``@m-ou-se``
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 6, 2021
Update standard library for IntoIterator implementation of arrays

This PR partially resolves issue rust-lang#84513 of updating the standard library part.

I haven't found any remaining doctest examples which are using iterators over e.g. &i32 instead of just i32 in the standard library. Can anyone point me to them if there's remaining any?

Thanks!

r? ```@m-ou-se```
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 22, 2021
…tsakis

Update array_into_iter lint for 1.53 and edition changes.

This updates the array_into_iter lint for Rust 1.53 and the edition changes.

See rust-lang#84513

r? `@estebank`
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 26, 2021
…akis

Update array_into_iter lint for 1.53 and edition changes.

This updates the array_into_iter lint for Rust 1.53 and the edition changes.

See rust-lang#84513

r? `@estebank`
@joshtriplett joshtriplett removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jul 20, 2022
@joshtriplett
Copy link
Member

The lang portion of this has stabilized and shipped, so un-tagging lang. libs-api may wish to continue tracking this for the proposed documentation changes.

@workingjubilee workingjubilee added the A-array Area: `[T; N]` label Mar 7, 2023
@dtolnay dtolnay added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-array Area: `[T; N]` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-edition-2021 Area: The 2021 edition A-iterators Area: Iterators C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants