Skip to content

Commit

Permalink
Auto merge of #21971 - pnkfelix:fsk-restrict-fixdsz-array-moves, r=ni…
Browse files Browse the repository at this point in the history 
…komatsakis

Revised version of PR #21930.

Restrictions on moves into and out-from fixed-length arrays.

(There was only one use of this "feature" in the compiler source.)

Note 1: the change to the error message in tests/compile-fail/borrowck-use-in-index-lvalue.rs, where we now report that *w is uninitialized (rather than w), was unintended fallout from the implementation strategy used here. The change appears harmless to me, but I welcome advice on how to bring back the old message, which was slightly cleaner (i.e. less unintelligible) since that the syntactic form *w does not actually appear in the source text.

Note 2: the move out-from restriction to only apply to expr[i], and not destructuring bind (e.g. f([a, b, c]: Array) { ... }) since the latter is compatible with nonzeroing drop, AFAICT.

[breaking-change]
  • Loading branch information
@bors
bors committed Feb 7, 2015
2 parents 80627cd + 4583272 commit 8661b3d
Show file tree
Hide file tree
Showing 17 changed files with 276 additions and 108 deletions.
77 changes: 56 additions & 21 deletions src/librustc/middle/mem_categorization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub enum PointerKind {
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub enum InteriorKind {
InteriorField(FieldName),
InteriorElement(ElementKind),
InteriorElement(InteriorOffsetKind, ElementKind),
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
Expand All @@ -137,6 +137,12 @@ pub enum FieldName {
PositionalField(uint)
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub enum InteriorOffsetKind {
Index, // e.g. `array_expr[index_expr]`
Pattern, // e.g. `fn foo([_, a, _, _]: [A; 4]) { ... }`
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub enum ElementKind {
VecElement,
Expand Down Expand Up @@ -196,10 +202,12 @@ pub enum deref_kind {
deref_interior(InteriorKind),
}

type DerefKindContext = Option<InteriorOffsetKind>;

// Categorizes a derefable type. Note that we include vectors and strings as
// derefable (we model an index as the combination of a deref and then a
// pointer adjustment).
pub fn deref_kind(t: Ty) -> McResult<deref_kind> {
fn deref_kind(t: Ty, context: DerefKindContext) -> McResult<deref_kind> {
match t.sty {
ty::ty_uniq(_) => {
Ok(deref_ptr(Unique))
Expand All @@ -220,7 +228,12 @@ pub fn deref_kind(t: Ty) -> McResult<deref_kind> {
}

ty::ty_vec(_, _) | ty::ty_str => {
Ok(deref_interior(InteriorElement(element_kind(t))))
// no deref of indexed content without supplying InteriorOffsetKind
if let Some(context) = context {
Ok(deref_interior(InteriorElement(context, element_kind(t))))
} else {
Err(())
}
}

_ => Err(()),
Expand Down Expand Up @@ -455,7 +468,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
autoderefs,
cmt.repr(self.tcx()));
for deref in 1..autoderefs + 1 {
cmt = try!(self.cat_deref(expr, cmt, deref));
cmt = try!(self.cat_deref(expr, cmt, deref, None));
}
return Ok(cmt);
}
Expand All @@ -467,7 +480,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
match expr.node {
ast::ExprUnary(ast::UnDeref, ref e_base) => {
let base_cmt = try!(self.cat_expr(&**e_base));
self.cat_deref(expr, base_cmt, 0)
self.cat_deref(expr, base_cmt, 0, None)
}

ast::ExprField(ref base, f_name) => {
Expand All @@ -486,6 +499,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {

ast::ExprIndex(ref base, _) => {
let method_call = ty::MethodCall::expr(expr.id());
let context = InteriorOffsetKind::Index;
match self.typer.node_method_ty(method_call) {
Some(method_ty) => {
// If this is an index implemented by a method call, then it
Expand All @@ -507,10 +521,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
// is an rvalue. That is what we will be
// dereferencing.
let base_cmt = self.cat_rvalue_node(expr.id(), expr.span(), ret_ty);
self.cat_deref_common(expr, base_cmt, 1, elem_ty, true)
self.cat_deref_common(expr, base_cmt, 1, elem_ty, Some(context), true)
}
None => {
self.cat_index(expr, try!(self.cat_expr(&**base)))
self.cat_index(expr, try!(self.cat_expr(&**base)), context)
}
}
}
Expand Down Expand Up @@ -854,7 +868,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
fn cat_deref<N:ast_node>(&self,
node: &N,
base_cmt: cmt<'tcx>,
deref_cnt: uint)
deref_cnt: uint,
deref_context: DerefKindContext)
-> McResult<cmt<'tcx>> {
let adjustment = match self.typer.adjustments().borrow().get(&node.id()) {
Some(adj) if ty::adjust_is_object(adj) => ty::AutoObject,
Expand Down Expand Up @@ -882,7 +897,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
};
let base_cmt_ty = base_cmt.ty;
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,
deref_context,
/* implicit: */ false),
None => {
debug!("Explicit deref of non-derefable type: {}",
Expand All @@ -897,10 +914,11 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
base_cmt: cmt<'tcx>,
deref_cnt: uint,
deref_ty: Ty<'tcx>,
deref_context: DerefKindContext,
implicit: bool)
-> McResult<cmt<'tcx>>
{
let (m, cat) = match try!(deref_kind(base_cmt.ty)) {
let (m, cat) = match try!(deref_kind(base_cmt.ty, deref_context)) {
deref_ptr(ptr) => {
let ptr = if implicit {
match ptr {
Expand Down Expand Up @@ -932,7 +950,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {

pub fn cat_index<N:ast_node>(&self,
elt: &N,
mut base_cmt: cmt<'tcx>)
mut base_cmt: cmt<'tcx>,
context: InteriorOffsetKind)
-> McResult<cmt<'tcx>> {
//! Creates a cmt for an indexing operation (`[]`).
//!
Expand Down Expand Up @@ -974,18 +993,21 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
};

let m = base_cmt.mutbl.inherit();
return Ok(interior(elt, base_cmt.clone(), base_cmt.ty, m, element_ty));
return Ok(interior(elt, base_cmt.clone(), base_cmt.ty,
m, context, element_ty));

fn interior<'tcx, N: ast_node>(elt: &N,
of_cmt: cmt<'tcx>,
vec_ty: Ty<'tcx>,
mutbl: MutabilityCategory,
context: InteriorOffsetKind,
element_ty: Ty<'tcx>) -> cmt<'tcx>
{
let interior_elem = InteriorElement(context, element_kind(vec_ty));
Rc::new(cmt_ {
id:elt.id(),
span:elt.span(),
cat:cat_interior(of_cmt, InteriorElement(element_kind(vec_ty))),
cat:cat_interior(of_cmt, interior_elem),
mutbl:mutbl,
ty:element_ty,
note: NoteNone
Expand All @@ -997,10 +1019,11 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
// underlying vec.
fn deref_vec<N:ast_node>(&self,
elt: &N,
base_cmt: cmt<'tcx>)
base_cmt: cmt<'tcx>,
context: InteriorOffsetKind)
-> McResult<cmt<'tcx>>
{
match try!(deref_kind(base_cmt.ty)) {
match try!(deref_kind(base_cmt.ty, Some(context))) {
deref_ptr(ptr) => {
// for unique ptrs, we inherit mutability from the
// owning reference.
Expand Down Expand Up @@ -1041,7 +1064,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
let (slice_mutbl, slice_r) = vec_slice_info(self.tcx(),
slice_pat,
slice_ty);
let cmt_slice = try!(self.cat_index(slice_pat, try!(self.deref_vec(slice_pat, vec_cmt))));
let context = InteriorOffsetKind::Pattern;
let cmt_vec = try!(self.deref_vec(slice_pat, vec_cmt, context));
let cmt_slice = try!(self.cat_index(slice_pat, cmt_vec, context));
return Ok((cmt_slice, slice_mutbl, slice_r));

/// In a pattern like [a, b, ..c], normally `c` has slice type, but if you have [a, b,
Expand Down Expand Up @@ -1253,12 +1278,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
// box p1, &p1, &mut p1. we can ignore the mutability of
// PatRegion since that information is already contained
// in the type.
let subcmt = try!(self.cat_deref(pat, cmt, 0));
let subcmt = try!(self.cat_deref(pat, cmt, 0, None));
try!(self.cat_pattern_(subcmt, &**subpat, op));
}

ast::PatVec(ref before, ref slice, ref after) => {
let elt_cmt = try!(self.cat_index(pat, try!(self.deref_vec(pat, cmt))));
let context = InteriorOffsetKind::Pattern;
let vec_cmt = try!(self.deref_vec(pat, cmt, context));
let elt_cmt = try!(self.cat_index(pat, vec_cmt, context));
for before_pat in before {
try!(self.cat_pattern_(elt_cmt.clone(), &**before_pat, op));
}
Expand Down Expand Up @@ -1455,10 +1482,18 @@ impl<'tcx> cmt_<'tcx> {
cat_interior(_, InteriorField(PositionalField(_))) => {
"anonymous field".to_string()
}
cat_interior(_, InteriorElement(VecElement)) |
cat_interior(_, InteriorElement(OtherElement)) => {
cat_interior(_, InteriorElement(InteriorOffsetKind::Index,
VecElement)) |
cat_interior(_, InteriorElement(InteriorOffsetKind::Index,
OtherElement)) => {
"indexed content".to_string()
}
cat_interior(_, InteriorElement(InteriorOffsetKind::Pattern,
VecElement)) |
cat_interior(_, InteriorElement(InteriorOffsetKind::Pattern,
OtherElement)) => {
"pattern-bound indexed content".to_string()
}
cat_upvar(ref var) => {
var.user_string(tcx)
}
Expand Down Expand Up @@ -1546,7 +1581,7 @@ impl<'tcx> Repr<'tcx> for InteriorKind {
token::get_name(fld).to_string()
}
InteriorField(PositionalField(i)) => format!("#{}", i),
InteriorElement(_) => "[]".to_string(),
InteriorElement(..) => "[]".to_string(),
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/librustc_borrowck/borrowck/check_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use self::UseError::*;

use borrowck::*;
use borrowck::InteriorKind::{InteriorElement, InteriorField};
use rustc::middle::expr_use_visitor as euv;
use rustc::middle::mem_categorization as mc;
use rustc::middle::region;
Expand Down Expand Up @@ -743,15 +744,16 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
self.check_if_assigned_path_is_moved(id, span,
use_kind, lp_base);
}
LpExtend(ref lp_base, _, LpInterior(_)) => {
LpExtend(ref lp_base, _, LpInterior(InteriorField(_))) => {
// assigning to `P.f` is ok if assigning to `P` is ok
self.check_if_assigned_path_is_moved(id, span,
use_kind, lp_base);
}
LpExtend(ref lp_base, _, LpInterior(InteriorElement(..))) |
LpExtend(ref lp_base, _, LpDeref(_)) => {
// assigning to `(*P)` requires that `P` be initialized
self.check_if_path_is_moved(id, span,
use_kind, lp_base);
// assigning to `P[i]` requires `P` is initialized
// assigning to `(*P)` requires `P` is initialized
self.check_if_path_is_moved(id, span, use_kind, lp_base);
}
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/librustc_borrowck/borrowck/fragments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use self::Fragment::*;

use borrowck::InteriorKind::{InteriorField, InteriorElement};
use borrowck::{LoanPath};
use borrowck::LoanPathKind::{LpVar, LpUpvar, LpDowncast, LpExtend};
use borrowck::LoanPathElem::{LpDeref, LpInterior};
Expand Down Expand Up @@ -300,20 +301,24 @@ fn add_fragment_siblings<'tcx>(this: &MoveData<'tcx>,
LpExtend(_, _, LpDeref(mc::Implicit(..))) |
LpExtend(_, _, LpDeref(mc::BorrowedPtr(..))) => {}

// FIXME(pnkfelix): LV[j] should be tracked, at least in the
// FIXME (pnkfelix): LV[j] should be tracked, at least in the
// sense of we will track the remaining drop obligation of the
// rest of the array.
//
// LV[j] is not tracked precisely
LpExtend(_, _, LpInterior(mc::InteriorElement(_))) => {
// Well, either that or LV[j] should be made illegal.
// But even then, we will need to deal with destructuring
// bind.
//
// Anyway, for now: LV[j] is not tracked precisely
LpExtend(_, _, LpInterior(InteriorElement(..))) => {
let mp = this.move_path(tcx, lp.clone());
gathered_fragments.push(AllButOneFrom(mp));
}

// field access LV.x and tuple access LV#k are the cases
// we are interested in
LpExtend(ref loan_parent, mc,
LpInterior(mc::InteriorField(ref field_name))) => {
LpInterior(InteriorField(ref field_name))) => {
let enum_variant_info = match loan_parent.kind {
LpDowncast(ref loan_parent_2, variant_def_id) =>
Some((variant_def_id, loan_parent_2.clone())),
Expand Down Expand Up @@ -452,7 +457,7 @@ fn add_fragment_sibling_core<'tcx>(this: &MoveData<'tcx>,
LpVar(..) | LpUpvar(..) | LpExtend(..) => enum_variant_did,
};

let loan_path_elem = LpInterior(mc::InteriorField(new_field_name));
let loan_path_elem = LpInterior(InteriorField(new_field_name));
let new_lp_type = match new_field_name {
mc::NamedField(ast_name) =>
ty::named_element_ty(tcx, parent.to_type(), ast_name, opt_variant_did),
Expand Down
11 changes: 10 additions & 1 deletion src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use borrowck::gather_loans::move_error::{MoveError, MoveErrorCollector};
use borrowck::move_data::*;
use rustc::middle::expr_use_visitor as euv;
use rustc::middle::mem_categorization as mc;
use rustc::middle::mem_categorization::Typer;
use rustc::middle::mem_categorization::InteriorOffsetKind as Kind;
use rustc::middle::ty;
use rustc::util::ppaux::Repr;
use std::rc::Rc;
Expand Down Expand Up @@ -156,6 +158,7 @@ pub fn gather_assignment<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
mode);
}

// (keep in sync with move_error::report_cannot_move_out_of )
fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
cmt: &mc::cmt<'tcx>)
-> Option<mc::cmt<'tcx>> {
Expand All @@ -174,7 +177,8 @@ fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
}

mc::cat_downcast(ref b, _) |
mc::cat_interior(ref b, _) => {
mc::cat_interior(ref b, mc::InteriorField(_)) |
mc::cat_interior(ref b, mc::InteriorElement(Kind::Pattern, _)) => {
match b.ty.sty {
ty::ty_struct(did, _) | ty::ty_enum(did, _) => {
if ty::has_dtor(bccx.tcx, did) {
Expand All @@ -189,6 +193,11 @@ fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
}
}

mc::cat_interior(_, mc::InteriorElement(Kind::Index, _)) => {
// Forbid move of arr[i] for arr: [T; 3]; see RFC 533.
Some(cmt.clone())
}

mc::cat_deref(ref b, _, mc::Unique) => {
check_and_get_illegal_move_origin(bccx, b)
}
Expand Down
15 changes: 14 additions & 1 deletion src/librustc_borrowck/borrowck/gather_loans/move_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

use borrowck::BorrowckCtxt;
use rustc::middle::mem_categorization as mc;
use rustc::middle::mem_categorization::Typer;
use rustc::middle::mem_categorization::InteriorOffsetKind as Kind;
use rustc::middle::ty;
use rustc::util::ppaux::UserString;
use std::cell::RefCell;
Expand Down Expand Up @@ -110,6 +112,7 @@ fn group_errors_with_same_origin<'tcx>(errors: &Vec<MoveError<'tcx>>)
}
}

// (keep in sync with gather_moves::check_and_get_illegal_move_origin )
fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
move_from: mc::cmt<'tcx>) {
match move_from.cat {
Expand All @@ -121,8 +124,18 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
move_from.descriptive_string(bccx.tcx))[]);
}

mc::cat_interior(ref b, mc::InteriorElement(Kind::Index, _)) => {
let expr = bccx.tcx.map.expect_expr(move_from.id);
if let ast::ExprIndex(..) = expr.node {
bccx.span_err(move_from.span,
&format!("cannot move out of type `{}`, \
a non-copy fixed-size array",
b.ty.user_string(bccx.tcx))[]);
}
}

mc::cat_downcast(ref b, _) |
mc::cat_interior(ref b, _) => {
mc::cat_interior(ref b, mc::InteriorField(_)) => {
match b.ty.sty {
ty::ty_struct(did, _) |
ty::ty_enum(did, _) if ty::has_dtor(bccx.tcx, did) => {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_borrowck/borrowck/gather_loans/restrictions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use rustc::middle::ty;
use rustc::util::ppaux::Repr;
use syntax::codemap::Span;

use borrowck::ToInteriorKind;

use std::rc::Rc;

#[derive(Debug)]
Expand Down Expand Up @@ -96,7 +98,7 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> {
// the memory, so no additional restrictions are
// needed.
let result = self.restrict(cmt_base);
self.extend(result, &cmt, LpInterior(i))
self.extend(result, &cmt, LpInterior(i.cleaned()))
}

mc::cat_static_item(..) => {
Expand Down
Loading

0 comments on commit 8661b3d

Please sign in to comment.