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

update array missing IntoIterator msg #82626

Merged
merged 2 commits into from
Mar 27, 2021

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Feb 28, 2021

fixes #82602

r? @estebank do you know whether we can use the expr span in rustc_on_unimplemented? The label isn't too great rn

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Feb 28, 2021
@estebank
Copy link
Contributor

estebank commented Mar 1, 2021

I believe https://doc.bccnsoft.com/docs/rust-1.36.0-docs-html/unstable-book/language-features/on-unimplemented.html is still up-to-date. I'm not sure I follow what you are asking for, but it's likely not available (unless you modify on_unimplemented, which is easy enough, but they are tricky/unusable until the changes land in beta.

@@ -81,8 +81,8 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
),
on(
_Self = "[]",
label = "borrow the array with `&` or call `.iter()` on it to iterate over it",
note = "arrays are not iterators, but slices like the following are: `&[1, 2, 3]`"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to suggest the &?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, i wasn't sure how to explain that in a concise way, as adding & changes the element type.

Do you have an idea for a second label here?

Comment on lines 23 to 32
error[E0277]: `[RangeFrom<{integer}>; 1]` is not an iterator
--> $DIR/array-of-ranges.rs:6:14
|
LL | for _ in [0..] {}
| ^^^^^ borrow the array with `&` or call `.iter()` on it to iterate over it
| ^^^^^ arrays do not yet implement `IntoIterator`; try using `std::array::IntoIter::new(arr)`
|
= help: the trait `Iterator` is not implemented for `[RangeFrom<{integer}>; 1]`
= note: arrays are not iterators, but slices like the following are: `&[1, 2, 3]`
= note: see <https://github.com/rust-lang/rust/pull/65819> for more details
= note: required because of the requirements on the impl of `IntoIterator` for `[RangeFrom<{integer}>; 1]`
= note: required by `into_iter`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the array of ranges cases, it might make sense for us to suggest that maybe they didn't want the outer [], just the range. That seems to be the most likely intent behind the code as written.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we already had a check for those cases https://github.com/rust-lang/rust/pull/82626/files#diff-c552b2bf18fb3212fe93dd601ce487badf5e50faf9aeed4db7b0f6c22571ef01L28-L33

Maybe at some point inference broke and instead of Idx we got {integer}?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AH! It broke not because of that, but because of 07e7823 and there being a single RangeInclusive but multiple Range structs in the std. rustc_on_unimplemented has a bug, it should always be using the full path for its filtering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, fixed that

@lcnr lcnr force-pushed the encode_with_shorthandb branch from 1156478 to 77bfa69 Compare March 12, 2021 11:23
@bors
Copy link
Contributor

bors commented Mar 24, 2021

☔ The latest upstream changes (presumably #75384) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank
Copy link
Contributor

r=me after fixing the merge conflict (sorry for not approving earlier)

@lcnr lcnr force-pushed the encode_with_shorthandb branch from 77bfa69 to 5ac917d Compare March 26, 2021 20:22
@lcnr
Copy link
Contributor Author

lcnr commented Mar 27, 2021

@bors r=estebank

@bors
Copy link
Contributor

bors commented Mar 27, 2021

📌 Commit 5ac917d has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
…bank

update array missing `IntoIterator` msg

fixes rust-lang#82602

r? `@estebank` do you know whether we can use the expr span in `rustc_on_unimplemented`? The label isn't too great rn
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
…bank

update array missing `IntoIterator` msg

fixes rust-lang#82602

r? ``@estebank`` do you know whether we can use the expr span in `rustc_on_unimplemented`? The label isn't too great rn
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#81351 (combine: stop eagerly evaluating consts)
 - rust-lang#82525 (make unaligned_references future-incompat lint warn-by-default)
 - rust-lang#82626 (update array missing `IntoIterator` msg)
 - rust-lang#82917 (Add function core::iter::zip)
 - rust-lang#82993 (rustdoc: Use diagnostics for error when including sources)
 - rust-lang#83522 (Improve fs error open_from unix)
 - rust-lang#83548 (Always preserve `None`-delimited groups in a captured `TokenStream`)
 - rust-lang#83555 (Add #[inline] to io::Error methods)

Failed merges:

 - rust-lang#83130 (escape_ascii take 2)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ebea9d9 into rust-lang:master Mar 27, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diagnostics: suggest using array::IntoIter::new to iterate over arrays
5 participants