From 2bc6547a5a293d61a2485090fd1d1d8a57b6baee Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 14 Jul 2014 19:43:21 +1200 Subject: [PATCH] Borrow checking for overloaded indexing Closes #15525 --- src/librustc/middle/borrowck/check_loans.rs | 3 +- .../borrowck/gather_loans/gather_moves.rs | 1 + .../middle/borrowck/gather_loans/lifetime.rs | 4 +- .../borrowck/gather_loans/move_error.rs | 1 + .../borrowck/gather_loans/restrictions.rs | 7 +- src/librustc/middle/mem_categorization.rs | 66 ++++++++++++++----- src/librustc/middle/typeck/check/regionck.rs | 9 ++- .../borrowck-overloaded-index-2.rs | 26 ++++++++ .../compile-fail/borrowck-overloaded-index.rs | 2 +- 9 files changed, 96 insertions(+), 23 deletions(-) create mode 100644 src/test/compile-fail/borrowck-overloaded-index-2.rs diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index db8ab8c83fbdc..cfec67bf3a395 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -734,7 +734,8 @@ impl<'a> CheckLoanCtxt<'a> { mc::cat_static_item | mc::cat_copied_upvar(..) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | - mc::cat_deref(_, _, mc::BorrowedPtr(..)) => { + mc::cat_deref(_, _, mc::BorrowedPtr(..)) | + mc::cat_deref(_, _, mc::Implicit(..)) => { assert_eq!(cmt.mutbl, mc::McDeclared); return; } diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index de77fa602c9b4..322471f30294f 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -131,6 +131,7 @@ fn check_and_get_illegal_move_origin(bccx: &BorrowckCtxt, cmt: &mc::cmt) -> Option { match cmt.cat { mc::cat_deref(_, _, mc::BorrowedPtr(..)) | + mc::cat_deref(_, _, mc::Implicit(..)) | mc::cat_deref(_, _, mc::GcPtr) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_upvar(..) | mc::cat_static_item | diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index dc8567af9edad..0785538cc76ab 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -72,6 +72,7 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_arg(..) | // L-Local mc::cat_upvar(..) | mc::cat_deref(_, _, mc::BorrowedPtr(..)) | // L-Deref-Borrowed + mc::cat_deref(_, _, mc::Implicit(..)) | mc::cat_deref(_, _, mc::UnsafePtr(..)) => { self.check_scope(self.scope(cmt)) } @@ -180,7 +181,8 @@ impl<'a> GuaranteeLifetimeContext<'a> { mc::cat_deref(_, _, mc::UnsafePtr(..)) => { ty::ReStatic } - mc::cat_deref(_, _, mc::BorrowedPtr(_, r)) => { + mc::cat_deref(_, _, mc::BorrowedPtr(_, r)) | + mc::cat_deref(_, _, mc::Implicit(_, r)) => { r } mc::cat_downcast(ref cmt) | diff --git a/src/librustc/middle/borrowck/gather_loans/move_error.rs b/src/librustc/middle/borrowck/gather_loans/move_error.rs index 9876e12d5ccac..10f051f004f19 100644 --- a/src/librustc/middle/borrowck/gather_loans/move_error.rs +++ b/src/librustc/middle/borrowck/gather_loans/move_error.rs @@ -113,6 +113,7 @@ fn group_errors_with_same_origin(errors: &Vec) fn report_cannot_move_out_of(bccx: &BorrowckCtxt, move_from: mc::cmt) { match move_from.cat { mc::cat_deref(_, _, mc::BorrowedPtr(..)) | + mc::cat_deref(_, _, mc::Implicit(..)) | mc::cat_deref(_, _, mc::GcPtr) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_upvar(..) | mc::cat_static_item | diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index d131b6f7eda29..48399cb0b7e02 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -122,7 +122,9 @@ impl<'a> RestrictionsContext<'a> { } mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::ImmBorrow, lt)) | - mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::UniqueImmBorrow, lt)) => { + mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::UniqueImmBorrow, lt)) | + mc::cat_deref(cmt_base, _, mc::Implicit(ty::ImmBorrow, lt)) | + mc::cat_deref(cmt_base, _, mc::Implicit(ty::UniqueImmBorrow, lt)) => { // R-Deref-Imm-Borrowed if !self.bccx.is_subregion_of(self.loan_region, lt) { self.bccx.report( @@ -137,7 +139,8 @@ impl<'a> RestrictionsContext<'a> { Safe } - mc::cat_deref(cmt_base, _, pk @ mc::BorrowedPtr(ty::MutBorrow, lt)) => { + mc::cat_deref(cmt_base, _, pk @ mc::BorrowedPtr(ty::MutBorrow, lt)) | + mc::cat_deref(cmt_base, _, pk @ mc::Implicit(ty::MutBorrow, lt)) => { // R-Deref-Mut-Borrowed if !self.bccx.is_subregion_of(self.loan_region, lt) { self.bccx.report( diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 33ab2ed363240..e928704b0ccf6 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -106,7 +106,8 @@ pub enum PointerKind { OwnedPtr, GcPtr, BorrowedPtr(ty::BorrowKind, ty::Region), - UnsafePtr(ast::Mutability), + Implicit(ty::BorrowKind, ty::Region), // Implicit deref of a borrowed ptr. + UnsafePtr(ast::Mutability) } // We use the term "interior" to mean "something reachable from the @@ -293,7 +294,7 @@ impl MutabilityCategory { OwnedPtr => { base_mutbl.inherit() } - BorrowedPtr(borrow_kind, _) => { + BorrowedPtr(borrow_kind, _) | Implicit(borrow_kind, _) => { MutabilityCategory::from_borrow_kind(borrow_kind) } GcPtr => { @@ -422,7 +423,7 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { -> McResult { let mut cmt = if_ok!(self.cat_expr_unadjusted(expr)); for deref in range(1u, autoderefs + 1) { - cmt = self.cat_deref(expr, cmt, deref); + cmt = self.cat_deref(expr, cmt, deref, false); } return Ok(cmt); } @@ -434,7 +435,7 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { match expr.node { ast::ExprUnary(ast::UnDeref, ref e_base) => { let base_cmt = if_ok!(self.cat_expr(&**e_base)); - Ok(self.cat_deref(expr, base_cmt, 0)) + Ok(self.cat_deref(expr, base_cmt, 0, false)) } ast::ExprField(ref base, f_name, _) => { @@ -443,8 +444,22 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { } ast::ExprIndex(ref base, _) => { - let base_cmt = if_ok!(self.cat_expr(&**base)); - Ok(self.cat_index(expr, base_cmt, 0)) + let method_call = typeck::MethodCall::expr(expr.id()); + match self.typer.node_method_ty(method_call) { + Some(method_ty) => { + // If this is an index implemented by a method call, then it will + // include an implicit deref of the result. + let ret_ty = ty::ty_fn_ret(method_ty); + Ok(self.cat_deref(expr, + self.cat_rvalue_node(expr.id(), + expr.span(), + ret_ty), 1, true)) + } + None => { + let base_cmt = if_ok!(self.cat_expr(&**base)); + Ok(self.cat_index(expr, base_cmt, 0)) + } + } } ast::ExprPath(_) => { @@ -687,13 +702,14 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { } pub fn cat_deref_obj(&self, node: &N, base_cmt: cmt) -> cmt { - self.cat_deref_common(node, base_cmt, 0, ty::mk_nil()) + self.cat_deref_common(node, base_cmt, 0, ty::mk_nil(), false) } fn cat_deref(&self, node: &N, base_cmt: cmt, - deref_cnt: uint) + deref_cnt: uint, + implicit: bool) -> cmt { let adjustment = match self.typer.adjustments().borrow().find(&node.id()) { Some(&ty::AutoObject(..)) => typeck::AutoObject, @@ -717,7 +733,7 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { None => base_cmt }; match ty::deref(base_cmt.ty, true) { - Some(mt) => self.cat_deref_common(node, base_cmt, deref_cnt, mt.ty), + Some(mt) => self.cat_deref_common(node, base_cmt, deref_cnt, mt.ty, implicit), None => { self.tcx().sess.span_bug( node.span(), @@ -731,10 +747,20 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { node: &N, base_cmt: cmt, deref_cnt: uint, - deref_ty: ty::t) + deref_ty: ty::t, + implicit: bool) -> cmt { let (m, cat) = match deref_kind(self.tcx(), base_cmt.ty) { deref_ptr(ptr) => { + let ptr = if implicit { + match ptr { + BorrowedPtr(bk, r) => Implicit(bk, r), + _ => self.tcx().sess.span_bug(node.span(), + "Implicit deref of non-borrowed pointer") + } + } else { + ptr + }; // for unique ptrs, we inherit mutability from the // owning reference. (MutabilityCategory::from_pointer_kind(base_cmt.mutbl, ptr), @@ -1073,7 +1099,7 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { ast::PatBox(ref subpat) | ast::PatRegion(ref subpat) => { // @p1, ~p1 - let subcmt = self.cat_deref(pat, cmt, 0); + let subcmt = self.cat_deref(pat, cmt, 0, false); if_ok!(self.cat_pattern(subcmt, &**subpat, op)); } @@ -1129,6 +1155,9 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { } _ => { match pk { + Implicit(..) => { + "dereference (dereference is implicit, due to indexing)".to_string() + } OwnedPtr | GcPtr => format!("dereference of `{}`", ptr_sigil(pk)), _ => format!("dereference of `{}`-pointer", ptr_sigil(pk)) } @@ -1188,6 +1217,7 @@ impl cmt_ { cat_deref(_, _, UnsafePtr(..)) | cat_deref(_, _, GcPtr(..)) | cat_deref(_, _, BorrowedPtr(..)) | + cat_deref(_, _, Implicit(..)) | cat_upvar(..) => { Rc::new((*self).clone()) } @@ -1212,7 +1242,9 @@ impl cmt_ { match self.cat { cat_deref(ref b, _, BorrowedPtr(ty::MutBorrow, _)) | + cat_deref(ref b, _, Implicit(ty::MutBorrow, _)) | cat_deref(ref b, _, BorrowedPtr(ty::UniqueImmBorrow, _)) | + cat_deref(ref b, _, Implicit(ty::UniqueImmBorrow, _)) | cat_downcast(ref b) | cat_deref(ref b, _, OwnedPtr) | cat_interior(ref b, _) | @@ -1252,7 +1284,8 @@ impl cmt_ { Some(AliasableManaged) } - cat_deref(_, _, BorrowedPtr(ty::ImmBorrow, _)) => { + cat_deref(_, _, BorrowedPtr(ty::ImmBorrow, _)) | + cat_deref(_, _, Implicit(ty::ImmBorrow, _)) => { Some(AliasableBorrowed) } } @@ -1300,9 +1333,12 @@ pub fn ptr_sigil(ptr: PointerKind) -> &'static str { match ptr { OwnedPtr => "Box", GcPtr => "Gc", - BorrowedPtr(ty::ImmBorrow, _) => "&", - BorrowedPtr(ty::MutBorrow, _) => "&mut", - BorrowedPtr(ty::UniqueImmBorrow, _) => "&unique", + BorrowedPtr(ty::ImmBorrow, _) | + Implicit(ty::ImmBorrow, _) => "&", + BorrowedPtr(ty::MutBorrow, _) | + Implicit(ty::MutBorrow, _) => "&mut", + BorrowedPtr(ty::UniqueImmBorrow, _) | + Implicit(ty::UniqueImmBorrow, _) => "&unique", UnsafePtr(_) => "*" } } diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index 924934e4bcdc5..180dac5382805 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -1219,7 +1219,8 @@ fn link_region(rcx: &Rcx, kind.repr(rcx.tcx()), cmt_borrowed.repr(rcx.tcx())); match cmt_borrowed.cat.clone() { - mc::cat_deref(base, _, mc::BorrowedPtr(_, r_borrowed)) => { + mc::cat_deref(base, _, mc::BorrowedPtr(_, r_borrowed)) | + mc::cat_deref(base, _, mc::Implicit(_, r_borrowed)) => { // References to an upvar `x` are translated to // `*x`, since that is what happens in the // underlying machine. We detect such references @@ -1340,7 +1341,8 @@ fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx, continue; } - mc::cat_deref(base, _, mc::BorrowedPtr(..)) => { + mc::cat_deref(base, _, mc::BorrowedPtr(..)) | + mc::cat_deref(base, _, mc::Implicit(..)) => { match base.cat { mc::cat_upvar(ref upvar_id, _) => { // if this is an implicit deref of an @@ -1394,7 +1396,8 @@ fn adjust_upvar_borrow_kind_for_unique(rcx: &Rcx, cmt: mc::cmt) { continue; } - mc::cat_deref(base, _, mc::BorrowedPtr(..)) => { + mc::cat_deref(base, _, mc::BorrowedPtr(..)) | + mc::cat_deref(base, _, mc::Implicit(..)) => { match base.cat { mc::cat_upvar(ref upvar_id, _) => { // if this is an implicit deref of an diff --git a/src/test/compile-fail/borrowck-overloaded-index-2.rs b/src/test/compile-fail/borrowck-overloaded-index-2.rs new file mode 100644 index 0000000000000..1e3144a931fbf --- /dev/null +++ b/src/test/compile-fail/borrowck-overloaded-index-2.rs @@ -0,0 +1,26 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct MyVec { + data: Vec, +} + +impl Index for MyVec { + fn index<'a>(&'a self, &i: &uint) -> &'a T { + self.data.get(i) + } +} + +fn main() { + let v = MyVec { data: vec!(box 1i, box 2, box 3) }; + let good = &v[0]; // Shouldn't fail here + let bad = v[0]; + //~^ ERROR cannot move out of dereference (dereference is implicit, due to indexing) +} diff --git a/src/test/compile-fail/borrowck-overloaded-index.rs b/src/test/compile-fail/borrowck-overloaded-index.rs index d34aa1cd9cbd3..0422f6381dc3d 100644 --- a/src/test/compile-fail/borrowck-overloaded-index.rs +++ b/src/test/compile-fail/borrowck-overloaded-index.rs @@ -58,7 +58,7 @@ fn main() { x: 1, }; s[2] = 20; - //~^ ERROR cannot assign to immutable indexed content + //~^ ERROR cannot assign to immutable dereference (dereference is implicit, due to indexing) }