exploration: ignoring arrays in method dispatch

[nikomatsakis]

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.

[nikomatsakis]

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

#[rustc_skip_array_during_method_dispatch]
trait IntoIterator { ... }

Voila.

[cuviper]

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 impl for arrays, rather than the trait itself, but I don't know what that looks like from the perspective of method resolution.

[Mark-Simulacrum]

Notably we'll also need to decide what to read the edition from - I'd suggest the . but don't feel strongly.

[nikomatsakis]

Mentoring instructions

We will modify

rust/compiler/rustc_typeck/src/check/method/probe.rs

Line 1463 in 5e73bd1

TraitCandidate(trait_ref) => {

so that -- in older editions -- we look for this attribute on the trait. If that attribute is present, then we check the self_ty variable and see whether it is a [T; N] array type. If so, we return ProbeResult::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_name field, since it is an Ident and those contain a span. One can then invoke the edition method -- or just rust_2021.

[nikomatsakis]

@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]

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]

OK -- I am interested in working on this, but should I wait for that discussion?

[nikomatsakis]

@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 write foo.into_iter().

[cuviper]

@rustbot claim

[joshtriplett]

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.

[nikomatsakis]

@joshtriplett note that Rust 2021 would work immediately. The question is only whether Rust 2018 eventually makes foo.into_iter() consider [T; N] as a possible self type or not.

[joshtriplett]

@nikomatsakis Ah, I see. I don't think there's enough value in making that transition in previous editions, and making people deal with the breakage. I think it'll be easier if we just make it a 2021 edition change.

[nikomatsakis]

@joshtriplett I'm inclined to agree

[cramertj]

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 like futures-util and itertools adapt to known overlap issues. For the case of preventing unknown overlap issues, I think there are fewer tools available.

[nikomatsakis]

@cramertj that sounds quite reasonable to me.

[Nichts]

@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.

[nikomatsakis]

The PR is merged, I'm going to close this issue.