Repository navigation
exploration: ignoring arrays in method dispatch #84133
Description
In #65819, we are considering adding an impl of IntoIterator for [T; N] for all N. However, this breaks existing code that relies on method dispatch:
let array = [true; 3];
for x in array.into_iter() {
let _ = *x; // x is a reference
}In this code, array.into_iter() winds up unsizing to a slice and hence resolving to <&[bool] as IntoIterator>::into_iter (and thus executing on refs to booleans). This issue describes a possible modification to older Rust editions that would allow us to add the impl without breaking existing code or compromising our edition guarantees.
Activity
nikomatsakis commentedon Apr 12, 2021
Proposal summary
Allow traits to be annotated with
#[rustc_skip_array_during_method_dispatch]. This is a perma-unstable annotation and it has the effect that we will ignore a trait method match during method dispatch, but only if the receive is a[T; N]type.To use in this case
Voila.
cuviper commentedon Apr 12, 2021
Should that attribute have a parameter for the edition? Or just hard-code that it only applies before 2021?
I also wonder if it makes more sense on the
implfor arrays, rather than the trait itself, but I don't know what that looks like from the perspective of method resolution.Mark-Simulacrum commentedon Apr 12, 2021
Notably we'll also need to decide what to read the edition from - I'd suggest the
.but don't feel strongly.nikomatsakis commentedon Apr 12, 2021
Mentoring instructions
We will modify
rust/compiler/rustc_typeck/src/check/method/probe.rs
Line 1463 in 5e73bd1
so that -- in older editions -- we look for this attribute on the trait. If that attribute is present, then we check the
self_tyvariable and see whether it is a[T; N]array type. If so, we returnProbeResult::NoMatch.As @Mark-Simulacrum notes, we need to decide what span to use to check the edition. I would propose we use the span from the method name. We can get the span from the
method_namefield, since it is anIdentand those contain a span. One can then invoke theeditionmethod -- or justrust_2021.nikomatsakis commentedon Apr 12, 2021
@cuviper take a look at those brief mentoring instructions -- that may explain why I chose to put the attribute on the trait. In short, that's the easiest thing we have available to us.
nikomatsakis commentedon Apr 12, 2021
I nominated this for @rust-lang/lang discussion to see how people react to the idea of an edition-related hack of this magnitude. It's worth noting that we could move this to a future-compatibility warning fairly easily and eventually remove it.
cuviper commentedon Apr 12, 2021
OK -- I am interested in working on this, but should I wait for that discussion?
nikomatsakis commentedon Apr 12, 2021
@cuviper I don't think that's necessary. My guess is that the main question would be whether this is something we eventually phase out and remove, or something we keep forever. Given how small the edits are, I don't see much of an argument for breaking folks immediately when warnings and a transition period are an option.
I can see an argument for not wanting to keep this change permanently, there is a kind of trade-off there. For example, it is a bit confusing that I can write
<[T; N] as IntoIterator>::into_iter(foo)but I can't writefoo.into_iter().cuviper commentedon Apr 12, 2021
@rustbot claim
joshtriplett commentedon Apr 12, 2021
This seems like a great solution to me.
I wouldn't want to keep this indefinitely; in a new edition, I think we do want into_iter to give an iterator over type T.
3 remaining items
nikomatsakis commentedon Apr 13, 2021
@joshtriplett I'm inclined to agree
cramertj commentedon Apr 13, 2021
I think this hack is a good idea, and I think we should do it in this case, but I am worried that we're using our power as language maintainers to sidestep an issue that regularly affects end-users of the language. That is, it is effectively a breaking change to add new methods or trait impls, even though our semantic versioning rules do not consider it to be one. crate maintainers may find that they break downstream users when adding new features that should otherwise not be semver-impacting. I'd love to see steps towards a solution, though admittedly I don't have a great idea what a solution might look like.
cfg(version/accessible)is one good step that helps commonly used utility libraries likefutures-utilanditertoolsadapt to known overlap issues. For the case of preventing unknown overlap issues, I think there are fewer tools available.nikomatsakis commentedon Apr 14, 2021
@cramertj that sounds quite reasonable to me.
Nichts commentedon Apr 22, 2021
@cramertj Issues like this seem like the prime example why python has from __future__ import. Could allow us to opt into the new behavior now and make the transition into the next edition easier.
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
Rollup merge of rust-lang#84590 - est31:array_into_iter, r=nikomatsakis
nikomatsakis commentedon May 4, 2021
The PR is merged, I'm going to close this issue.