Auto merge of #47167 - ivanbakel:builtin_indexing, r=nikomatsakis

Fix built-in indexing not being used where index type wasn't "obviously" usize

Fixes #33903
Fixes #46095

This PR was made possible thanks to the generous help of @EddyB

Following the example of binary operators, builtin checking for indexing has been moved from the typecheck stage to a writeback stage, after type constraints have been resolved.

src/librustc_typeck/check/mod.rs

@@ -2217,18 +2217,6 @@ fn try_index_step(&self,
                adjusted_ty,
                index_ty);
 
-        // First, try built-in indexing.
-        match (adjusted_ty.builtin_index(), &index_ty.sty) {
-            (Some(ty), &ty::TyUint(ast::UintTy::Usize)) |
-            (Some(ty), &ty::TyInfer(ty::IntVar(_))) => {
-                debug!("try_index_step: success, using built-in indexing");
-                let adjustments = autoderef.adjust_steps(lvalue_pref);
-                self.apply_adjustments(base_expr, adjustments);
-                return Some((self.tcx.types.usize, ty));
-            }
-            _ => {}
-        }
-
         for &unsize in &[false, true] {
             let mut self_ty = adjusted_ty;
             if unsize {

src/librustc_typeck/check/writeback.rs

@@ -18,6 +18,7 @@
 use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc::infer::InferCtxt;
 use rustc::ty::{self, Ty, TyCtxt};
+use rustc::ty::adjustment::{Adjust, Adjustment};
 use rustc::ty::fold::{TypeFoldable, TypeFolder};
 use rustc::util::nodemap::DefIdSet;
 use syntax::ast;
@@ -159,8 +160,52 @@ fn fix_scalar_builtin_expr(&mut self, e: &hir::Expr) {
             _ => {}
         }
     }
+
+    // Similar to operators, indexing is always assumed to be overloaded
+    // Here, correct cases where an indexing expression can be simplified
+    // to use builtin indexing because the index type is known to be
+    // usize-ish
+    fn fix_index_builtin_expr(&mut self, e: &hir::Expr) {
+        if let hir::ExprIndex(ref base, ref index) = e.node {
+            let mut tables = self.fcx.tables.borrow_mut();
+
+            match tables.expr_ty_adjusted(&base).sty {
+                // All valid indexing looks like this
+                ty::TyRef(_, ty::TypeAndMut { ty: ref base_ty, .. }) => {
+                    let index_ty = tables.expr_ty_adjusted(&index);
+                    let index_ty = self.fcx.resolve_type_vars_if_possible(&index_ty);
+
+                    if base_ty.builtin_index().is_some()
+                        && index_ty == self.fcx.tcx.types.usize {
+                        // Remove the method call record
+                        tables.type_dependent_defs_mut().remove(e.hir_id);
+                        tables.node_substs_mut().remove(e.hir_id);
+
+                        tables.adjustments_mut().get_mut(base.hir_id).map(|a| {
+                            // Discard the need for a mutable borrow
+                            match a.pop() {
+                                // Extra adjustment made when indexing causes a drop
+                                // of size information - we need to get rid of it
+                                // Since this is "after" the other adjustment to be
+                                // discarded, we do an extra `pop()`
+                                Some(Adjustment { kind: Adjust::Unsize, .. }) => {
+                                    // So the borrow discard actually happens here
+                                    a.pop();
+                                },
+                                _ => {}
+                            }
+                        });
+                    }
+                },
+                // Might encounter non-valid indexes at this point, so there
+                // has to be a fall-through
+                _ => {},
+            }
+        }
+    }
 }
 
+
 ///////////////////////////////////////////////////////////////////////////
 // Impl of Visitor for Resolver
 //
@@ -176,6 +221,7 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'gcx> {
 
     fn visit_expr(&mut self, e: &'gcx hir::Expr) {
         self.fix_scalar_builtin_expr(e);
+        self.fix_index_builtin_expr(e);
 
         self.visit_node_id(e.span, e.hir_id);
 

src/test/run-pass/issue-33903.rs

+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Issue 33903:
+// Built-in indexing should be used even when the index is not
+// trivially an integer
+// Only built-in indexing can be used in constant expresssions
+
+const FOO: i32 = [12, 34][0 + 1];
+
+fn main() {}
+

src/test/run-pass/issue-46095.rs

+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct A;
+
+impl A {
+    fn take_mutably(&mut self) {}
+}
+
+fn identity<T>(t: T) -> T {
+    t
+}
+
+// Issue 46095
+// Built-in indexing should be used even when the index is not
+// trivially an integer
+// Overloaded indexing would cause wrapped to be borrowed mutably
+
+fn main() {
+    let mut a1 = A;
+    let mut a2 = A;
+
+    let wrapped = [&mut a1, &mut a2];
+
+    {
+        wrapped[0 + 1 - 1].take_mutably();
+    }
+
+    {
+        wrapped[identity(0)].take_mutably();
+    }
+}