Can't overload indexing for arrays

[tarsa]

Following code compiled in Rust 1.24.1, but fails to compile under Rust 1.25.0:

use std::ops;

struct MyIndex(bool);

impl ops::Index<MyIndex> for [String; 2] {
    type Output = String;

    fn index(&self, index: MyIndex) -> &String {
        match index.0 {
            false => &self[0],
            true => &self[1],
        }
    }
}

fn main() {
  let a = [String::from("ala"), String::from("ma kota")];
  println!("{}", a[MyIndex(false)]);
}

Terminal session:

$ rustup toolchain list
stable-x86_64-unknown-linux-gnu
1.24.1-x86_64-unknown-linux-gnu (default)
$ rustup default stable-x86_64-unknown-linux-gnu
info: using existing install for 'stable-x86_64-unknown-linux-gnu'
info: default toolchain set to 'stable-x86_64-unknown-linux-gnu'

  stable-x86_64-unknown-linux-gnu unchanged - rustc 1.25.0 (84203cac6 2018-03-25)

$ rustc aaa.rs
error[E0308]: mismatched types
  --> aaa.rs:10:28
   |
10 |             false => &self[0],
   |                            ^ expected struct `MyIndex`, found integral variable
   |
   = note: expected type `MyIndex`
              found type `{integer}`

error[E0308]: mismatched types
  --> aaa.rs:11:27
   |
11 |             true => &self[1],
   |                           ^ expected struct `MyIndex`, found integral variable
   |
   = note: expected type `MyIndex`
              found type `{integer}`

error: aborting due to 2 previous errors

$ rustup default 1.24.1-x86_64-unknown-linux-gnu
info: using existing install for '1.24.1-x86_64-unknown-linux-gnu'
info: default toolchain set to '1.24.1-x86_64-unknown-linux-gnu'

  1.24.1-x86_64-unknown-linux-gnu unchanged - rustc 1.24.1 (d3ae9a9e0 2018-02-27)

$ rustc aaa.rs
$ ./aaa 
ala

[nikomatsakis]

Can we get a bisection? I remember there being some PRs that changed indexing of built-in types to work with impls though, so I suspect that is the cause.

cc @rust-lang/release

[nikomatsakis]

triage: P-high

Regression, perhaps legit, but we need to see why.

[pietroalbini]

Bisected this, the regression is in #47167. cc @ivanbakel @nikomatsakis

[nikomatsakis]

@pietroalbini thanks! That's the PR I was thinking of, I believe.

[nikomatsakis]

Sigh. This was missing a T-compiler tag, so it was overlooked.

[nikomatsakis]

Ok, so, I tracked down what's going on. The answer is that we have no impls of Index for [T; N] types, only for [T]. Ordinarily, when you do &self[0], what happens is that we "autoderef" self down to the type [String; 2] and then we unsize that to [String]. At the point, the indexing impls defined for [String] kick in. But now, with this new impl, we are failing to proceed that far, and instead stopping at the impl for [String; 2] (which is defined for Index<MyIndex>).

As a workaround, the code can be fixed by doing the unsize to [String] by hand:

impl ops::Index<MyIndex> for [String; 2] {
    type Output = String;

    fn index(&self, index: MyIndex) -> &String {
        let this: &[String] = self;
        match index.0 {
            false => &this[0],
            true => &this[1],
        }
    }
}

The question is, is this the desired behavior? It seems like when we search for matching impls we are not taking the type of the index into account. Looking at the code, I can't quite see why that is. It seems like we have the info we need, we're just choosing to ignore it. I'm experimenting. For reference, this is the function that checks for whether [] should stop at a given autoderef:

rust/src/librustc_typeck/check/mod.rs

Lines 2352 to 2358 in 90463a6

	fn try_index_step(&self,
	expr: &hir::Expr,
	base_expr: &hir::Expr,
	autoderef: &Autoderef<'a, 'gcx, 'tcx>,
	needs: Needs,
	index_ty: Ty<'tcx>)
	-> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)>

The index_ty argument appears to be the type of index input (in this instance, it is an unresolved integer variable). But when we actually check to see if the trait is implemented, we choose to ignore that information:

rust/src/librustc_typeck/check/mod.rs

Lines 2382 to 2384 in 90463a6

	let input_ty = self.next_ty_var(TypeVariableOrigin::AutoDeref(base_expr.span));
	let method = self.try_overloaded_place_op(
	expr.span, self_ty, &[input_ty], needs, PlaceOp::Index);

Perhaps this is meant to be analogous to foo.bar(baz), where the type of baz is not used?

[nikomatsakis]

Still, this diff makes the test pass:

diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index db859e4205..ef5599f82b 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -2381,9 +2381,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             // If some lookup succeeds, write callee into table and extract index/element
             // type from the method signature.
             // If some lookup succeeded, install method in table
-            let input_ty = self.next_ty_var(TypeVariableOrigin::AutoDeref(base_expr.span));
             let method = self.try_overloaded_place_op(
-                expr.span, self_ty, &[input_ty], needs, PlaceOp::Index);
+                expr.span, self_ty, &[index_ty], needs, PlaceOp::Index);

             let result = method.map(|ok| {
                 debug!("try_index_step: success, using overloaded indexing");
@@ -2418,7 +2417,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 self.apply_adjustments(base_expr, adjustments);

                 self.write_method_call(expr.hir_id, method);
-                (input_ty, self.make_overloaded_place_return_type(method).ty)
+                (index_ty, self.make_overloaded_place_return_type(method).ty)
             });
             if result.is_some() {
                 return result;

[nikomatsakis]

Now we just have to decide if we want to do that. I'll also want to check the other implications. I'm going to make a branch and start a ./x.py test to see if things break (I can't imagine what would, but you never know).

[nikomatsakis]

The main complication seems to be that we regress some of the helpful error messages around indexing.