Skip to content

Commit f62f774

Browse files
committed
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.
2 parents 27ede55 + 6132806 commit f62f774

File tree

4 files changed

+104
-12
lines changed

4 files changed

+104
-12
lines changed

src/librustc_typeck/check/mod.rs

-12
Original file line numberDiff line numberDiff line change
@@ -2217,18 +2217,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
22172217
adjusted_ty,
22182218
index_ty);
22192219

2220-
// First, try built-in indexing.
2221-
match (adjusted_ty.builtin_index(), &index_ty.sty) {
2222-
(Some(ty), &ty::TyUint(ast::UintTy::Usize)) |
2223-
(Some(ty), &ty::TyInfer(ty::IntVar(_))) => {
2224-
debug!("try_index_step: success, using built-in indexing");
2225-
let adjustments = autoderef.adjust_steps(lvalue_pref);
2226-
self.apply_adjustments(base_expr, adjustments);
2227-
return Some((self.tcx.types.usize, ty));
2228-
}
2229-
_ => {}
2230-
}
2231-
22322220
for &unsize in &[false, true] {
22332221
let mut self_ty = adjusted_ty;
22342222
if unsize {

src/librustc_typeck/check/writeback.rs

+46
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use rustc::hir::def_id::{DefId, DefIndex};
1818
use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
1919
use rustc::infer::InferCtxt;
2020
use rustc::ty::{self, Ty, TyCtxt};
21+
use rustc::ty::adjustment::{Adjust, Adjustment};
2122
use rustc::ty::fold::{TypeFoldable, TypeFolder};
2223
use rustc::util::nodemap::DefIdSet;
2324
use syntax::ast;
@@ -159,8 +160,52 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
159160
_ => {}
160161
}
161162
}
163+
164+
// Similar to operators, indexing is always assumed to be overloaded
165+
// Here, correct cases where an indexing expression can be simplified
166+
// to use builtin indexing because the index type is known to be
167+
// usize-ish
168+
fn fix_index_builtin_expr(&mut self, e: &hir::Expr) {
169+
if let hir::ExprIndex(ref base, ref index) = e.node {
170+
let mut tables = self.fcx.tables.borrow_mut();
171+
172+
match tables.expr_ty_adjusted(&base).sty {
173+
// All valid indexing looks like this
174+
ty::TyRef(_, ty::TypeAndMut { ty: ref base_ty, .. }) => {
175+
let index_ty = tables.expr_ty_adjusted(&index);
176+
let index_ty = self.fcx.resolve_type_vars_if_possible(&index_ty);
177+
178+
if base_ty.builtin_index().is_some()
179+
&& index_ty == self.fcx.tcx.types.usize {
180+
// Remove the method call record
181+
tables.type_dependent_defs_mut().remove(e.hir_id);
182+
tables.node_substs_mut().remove(e.hir_id);
183+
184+
tables.adjustments_mut().get_mut(base.hir_id).map(|a| {
185+
// Discard the need for a mutable borrow
186+
match a.pop() {
187+
// Extra adjustment made when indexing causes a drop
188+
// of size information - we need to get rid of it
189+
// Since this is "after" the other adjustment to be
190+
// discarded, we do an extra `pop()`
191+
Some(Adjustment { kind: Adjust::Unsize, .. }) => {
192+
// So the borrow discard actually happens here
193+
a.pop();
194+
},
195+
_ => {}
196+
}
197+
});
198+
}
199+
},
200+
// Might encounter non-valid indexes at this point, so there
201+
// has to be a fall-through
202+
_ => {},
203+
}
204+
}
205+
}
162206
}
163207

208+
164209
///////////////////////////////////////////////////////////////////////////
165210
// Impl of Visitor for Resolver
166211
//
@@ -176,6 +221,7 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> {
176221

177222
fn visit_expr(&mut self, e: &'gcx hir::Expr) {
178223
self.fix_scalar_builtin_expr(e);
224+
self.fix_index_builtin_expr(e);
179225

180226
self.visit_node_id(e.span, e.hir_id);
181227

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

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

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

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
struct A;
12+
13+
impl A {
14+
fn take_mutably(&mut self) {}
15+
}
16+
17+
fn identity<T>(t: T) -> T {
18+
t
19+
}
20+
21+
// Issue 46095
22+
// Built-in indexing should be used even when the index is not
23+
// trivially an integer
24+
// Overloaded indexing would cause wrapped to be borrowed mutably
25+
26+
fn main() {
27+
let mut a1 = A;
28+
let mut a2 = A;
29+
30+
let wrapped = [&mut a1, &mut a2];
31+
32+
{
33+
wrapped[0 + 1 - 1].take_mutably();
34+
}
35+
36+
{
37+
wrapped[identity(0)].take_mutably();
38+
}
39+
}

0 commit comments

Comments
 (0)