diff --git a/src/librustc_typeck/check/dropck.rs b/src/librustc_typeck/check/dropck.rs index 8545e73c4f932..fd90d662bd141 100644 --- a/src/librustc_typeck/check/dropck.rs +++ b/src/librustc_typeck/check/dropck.rs @@ -396,19 +396,24 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( } }; - let opt_type_did = match typ.sty { - ty::ty_struct(struct_did, _) => Some(struct_did), - ty::ty_enum(enum_did, _) => Some(enum_did), - _ => None, + let dtor_kind = match typ.sty { + ty::ty_enum(def_id, _) | + ty::ty_struct(def_id, _) => { + match destructor_for_type.get(&def_id) { + Some(def_id) => DtorKind::KnownDropMethod(*def_id), + None => DtorKind::PureRecur, + } + } + ty::ty_trait(ref ty_trait) => { + DtorKind::Unknown(ty_trait.bounds.clone()) + } + _ => DtorKind::PureRecur, }; - let opt_dtor = - opt_type_did.and_then(|did| destructor_for_type.get(&did)); - debug!("iterate_over_potentially_unsafe_regions_in_type \ - {}typ: {} scope: {:?} opt_dtor: {:?} xref: {}", + {}typ: {} scope: {:?} xref: {}", (0..depth).map(|_| ' ').collect::(), - typ.repr(rcx.tcx()), scope, opt_dtor, xref_depth); + typ.repr(rcx.tcx()), scope, xref_depth); // If `typ` has a destructor, then we must ensure that all // borrowed data reachable via `typ` must outlive the parent @@ -439,102 +444,7 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( // simply skip the `type_must_outlive` call entirely (but // resume the recursive checking of the type-substructure). - let has_dtor_of_interest; - - if let Some(&dtor_method_did) = opt_dtor { - let impl_did = ty::impl_of_method(rcx.tcx(), dtor_method_did) - .unwrap_or_else(|| { - rcx.tcx().sess.span_bug( - span, "no Drop impl found for drop method") - }); - - let dtor_typescheme = ty::lookup_item_type(rcx.tcx(), impl_did); - let dtor_generics = dtor_typescheme.generics; - - let mut has_pred_of_interest = false; - - let mut seen_items = Vec::new(); - let mut items_to_inspect = vec![impl_did]; - 'items: while let Some(item_def_id) = items_to_inspect.pop() { - if seen_items.contains(&item_def_id) { - continue; - } - - for pred in ty::lookup_predicates(rcx.tcx(), item_def_id).predicates { - let result = match pred { - ty::Predicate::Equate(..) | - ty::Predicate::RegionOutlives(..) | - ty::Predicate::TypeOutlives(..) | - ty::Predicate::Projection(..) => { - // For now, assume all these where-clauses - // may give drop implementation capabilty - // to access borrowed data. - true - } - - ty::Predicate::Trait(ty::Binder(ref t_pred)) => { - let def_id = t_pred.trait_ref.def_id; - if ty::trait_items(rcx.tcx(), def_id).len() != 0 { - // If trait has items, assume it adds - // capability to access borrowed data. - true - } else { - // Trait without items is itself - // uninteresting from POV of dropck. - // - // However, may have parent w/ items; - // so schedule checking of predicates, - items_to_inspect.push(def_id); - // and say "no capability found" for now. - false - } - } - }; - - if result { - has_pred_of_interest = true; - debug!("typ: {} has interesting dtor due to generic preds, e.g. {}", - typ.repr(rcx.tcx()), pred.repr(rcx.tcx())); - break 'items; - } - } - - seen_items.push(item_def_id); - } - - // In `impl<'a> Drop ...`, we automatically assume - // `'a` is meaningful and thus represents a bound - // through which we could reach borrowed data. - // - // FIXME (pnkfelix): In the future it would be good to - // extend the language to allow the user to express, - // in the impl signature, that a lifetime is not - // actually used (something like `where 'a: ?Live`). - let has_region_param_of_interest = - dtor_generics.has_region_params(subst::TypeSpace); - - has_dtor_of_interest = - has_region_param_of_interest || - has_pred_of_interest; - - if has_dtor_of_interest { - debug!("typ: {} has interesting dtor, due to \ - region params: {} or pred: {}", - typ.repr(rcx.tcx()), - has_region_param_of_interest, - has_pred_of_interest); - } else { - debug!("typ: {} has dtor, but it is uninteresting", - typ.repr(rcx.tcx())); - } - - } else { - debug!("typ: {} has no dtor, and thus is uninteresting", - typ.repr(rcx.tcx())); - has_dtor_of_interest = false; - } - - if has_dtor_of_interest { + if has_dtor_of_interest(rcx.tcx(), dtor_kind, typ, span) { // If `typ` has a destructor, then we must ensure that all // borrowed data reachable via `typ` must outlive the // parent of `scope`. (It does not suffice for it to @@ -620,7 +530,7 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( ty::ty_rptr(..) | ty::ty_ptr(_) | ty::ty_bare_fn(..) => { // Don't recurse, since references, pointers, - // boxes, and bare functions don't own instances + // and bare functions don't own instances // of the types appearing within them. walker.skip_current_subtree(); } @@ -639,3 +549,140 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( return Ok(()); } + +enum DtorKind<'tcx> { + // Type has an associated drop method with this def id + KnownDropMethod(ast::DefId), + + // Type has no destructor (or its dtor is known to be pure + // with respect to lifetimes), though its *substructure* + // may carry a destructor. + PureRecur, + + // Type may have impure destructor that is unknown; + // e.g. `Box` + Unknown(ty::ExistentialBounds<'tcx>), +} + +fn has_dtor_of_interest<'tcx>(tcx: &ty::ctxt<'tcx>, + dtor_kind: DtorKind, + typ: ty::Ty<'tcx>, + span: Span) -> bool { + let has_dtor_of_interest: bool; + + match dtor_kind { + DtorKind::PureRecur => { + has_dtor_of_interest = false; + debug!("typ: {} has no dtor, and thus is uninteresting", + typ.repr(tcx)); + } + DtorKind::Unknown(bounds) => { + match bounds.region_bound { + ty::ReStatic => { + debug!("trait: {} has 'static bound, and thus is uninteresting", + typ.repr(tcx)); + has_dtor_of_interest = false; + } + ty::ReEmpty => { + debug!("trait: {} has empty region bound, and thus is uninteresting", + typ.repr(tcx)); + has_dtor_of_interest = false; + } + r => { + debug!("trait: {} has non-static bound: {}; assumed interesting", + typ.repr(tcx), r.repr(tcx)); + has_dtor_of_interest = true; + } + } + } + DtorKind::KnownDropMethod(dtor_method_did) => { + let impl_did = ty::impl_of_method(tcx, dtor_method_did) + .unwrap_or_else(|| { + tcx.sess.span_bug( + span, "no Drop impl found for drop method") + }); + + let dtor_typescheme = ty::lookup_item_type(tcx, impl_did); + let dtor_generics = dtor_typescheme.generics; + + let mut has_pred_of_interest = false; + + let mut seen_items = Vec::new(); + let mut items_to_inspect = vec![impl_did]; + 'items: while let Some(item_def_id) = items_to_inspect.pop() { + if seen_items.contains(&item_def_id) { + continue; + } + + for pred in ty::lookup_predicates(tcx, item_def_id).predicates { + let result = match pred { + ty::Predicate::Equate(..) | + ty::Predicate::RegionOutlives(..) | + ty::Predicate::TypeOutlives(..) | + ty::Predicate::Projection(..) => { + // For now, assume all these where-clauses + // may give drop implementation capabilty + // to access borrowed data. + true + } + + ty::Predicate::Trait(ty::Binder(ref t_pred)) => { + let def_id = t_pred.trait_ref.def_id; + if ty::trait_items(tcx, def_id).len() != 0 { + // If trait has items, assume it adds + // capability to access borrowed data. + true + } else { + // Trait without items is itself + // uninteresting from POV of dropck. + // + // However, may have parent w/ items; + // so schedule checking of predicates, + items_to_inspect.push(def_id); + // and say "no capability found" for now. + false + } + } + }; + + if result { + has_pred_of_interest = true; + debug!("typ: {} has interesting dtor due to generic preds, e.g. {}", + typ.repr(tcx), pred.repr(tcx)); + break 'items; + } + } + + seen_items.push(item_def_id); + } + + // In `impl<'a> Drop ...`, we automatically assume + // `'a` is meaningful and thus represents a bound + // through which we could reach borrowed data. + // + // FIXME (pnkfelix): In the future it would be good to + // extend the language to allow the user to express, + // in the impl signature, that a lifetime is not + // actually used (something like `where 'a: ?Live`). + let has_region_param_of_interest = + dtor_generics.has_region_params(subst::TypeSpace); + + has_dtor_of_interest = + has_region_param_of_interest || + has_pred_of_interest; + + if has_dtor_of_interest { + debug!("typ: {} has interesting dtor, due to \ + region params: {} or pred: {}", + typ.repr(tcx), + has_region_param_of_interest, + has_pred_of_interest); + } else { + debug!("typ: {} has dtor, but it is uninteresting", + typ.repr(tcx)); + } + } + } + + return has_dtor_of_interest; +} diff --git a/src/libstd/net/parser.rs b/src/libstd/net/parser.rs index 88cfc5a7b2d8d..69f40d7e7befe 100644 --- a/src/libstd/net/parser.rs +++ b/src/libstd/net/parser.rs @@ -61,7 +61,7 @@ impl<'a> Parser<'a> { } // Return result of first successful parser - fn read_or(&mut self, parsers: &mut [Box Option>]) + fn read_or(&mut self, parsers: &mut [Box Option + 'static>]) -> Option { for pf in parsers.iter_mut() { match self.read_atomically(|p: &mut Parser| pf(p)) { diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 4ea2d4e5c686c..c3f1b9748155f 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -986,9 +986,10 @@ fn expand_pat(p: P, fld: &mut MacroExpander) -> P { let fm = fresh_mark(); let marked_before = mark_tts(&tts[..], fm); let mac_span = fld.cx.original_span(); - let expanded = match expander.expand(fld.cx, - mac_span, - &marked_before[..]).make_pat() { + let pat = expander.expand(fld.cx, + mac_span, + &marked_before[..]).make_pat(); + let expanded = match pat { Some(e) => e, None => { fld.cx.span_err( diff --git a/src/libsyntax/util/parser_testing.rs b/src/libsyntax/util/parser_testing.rs index d016eb39239f6..929f2a6abd6b8 100644 --- a/src/libsyntax/util/parser_testing.rs +++ b/src/libsyntax/util/parser_testing.rs @@ -73,7 +73,11 @@ pub fn string_to_stmt(source_str : String) -> P { /// Parse a string, return a pat. Uses "irrefutable"... which doesn't /// (currently) affect parsing. pub fn string_to_pat(source_str: String) -> P { - string_to_parser(&new_parse_sess(), source_str).parse_pat() + // Binding `sess` and `parser` works around dropck-injected + // region-inference issues; see #25212, #22323, #22321. + let sess = new_parse_sess(); + let mut parser = string_to_parser(&sess, source_str); + parser.parse_pat() } /// Convert a vector of strings to a vector of ast::Ident's diff --git a/src/test/compile-fail/dropck_trait_cycle_checked.rs b/src/test/compile-fail/dropck_trait_cycle_checked.rs new file mode 100644 index 0000000000000..6e543d017f260 --- /dev/null +++ b/src/test/compile-fail/dropck_trait_cycle_checked.rs @@ -0,0 +1,131 @@ +// Copyright 2015 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. + +// Reject mixing cyclic structure and Drop when using trait +// objects to hide the the cross-references. +// +// (Compare against compile-fail/dropck_vec_cycle_checked.rs) + +use std::cell::Cell; +use id::Id; + +mod s { + use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; + + static S_COUNT: AtomicUsize = ATOMIC_USIZE_INIT; + + pub fn next_count() -> usize { + S_COUNT.fetch_add(1, Ordering::SeqCst) + 1 + } +} + +mod id { + use s; + #[derive(Debug)] + pub struct Id { + orig_count: usize, + count: usize, + } + + impl Id { + pub fn new() -> Id { + let c = s::next_count(); + println!("building Id {}", c); + Id { orig_count: c, count: c } + } + pub fn count(&self) -> usize { + println!("Id::count on {} returns {}", self.orig_count, self.count); + self.count + } + } + + impl Drop for Id { + fn drop(&mut self) { + println!("dropping Id {}", self.count); + self.count = 0; + } + } +} + +trait HasId { + fn count(&self) -> usize; +} + +#[derive(Debug)] +struct CheckId { + v: T +} + +#[allow(non_snake_case)] +fn CheckId(t: T) -> CheckId { CheckId{ v: t } } + +impl Drop for CheckId { + fn drop(&mut self) { + assert!(self.v.count() > 0); + } +} + +trait Obj<'a> : HasId { + fn set0(&self, b: &'a Box>); + fn set1(&self, b: &'a Box>); +} + +struct O<'a> { + id: Id, + obj0: CheckId>>>>, + obj1: CheckId>>>>, +} + +impl<'a> HasId for O<'a> { + fn count(&self) -> usize { self.id.count() } +} + +impl<'a> O<'a> { + fn new() -> Box> { + Box::new(O { + id: Id::new(), + obj0: CheckId(Cell::new(None)), + obj1: CheckId(Cell::new(None)), + }) + } +} + +impl<'a> HasId for Cell>>> { + fn count(&self) -> usize { + match self.get() { + None => 1, + Some(c) => c.count(), + } + } +} + +impl<'a> Obj<'a> for O<'a> { + fn set0(&self, b: &'a Box>) { + self.obj0.v.set(Some(b)) + } + fn set1(&self, b: &'a Box>) { + self.obj1.v.set(Some(b)) + } +} + + +fn f() { + let (o1, o2, o3): (Box, Box, Box) = (O::new(), O::new(), O::new()); + o1.set0(&o2); //~ ERROR `o2` does not live long enough + o1.set1(&o3); //~ ERROR `o3` does not live long enough + o2.set0(&o2); //~ ERROR `o2` does not live long enough + o2.set1(&o3); //~ ERROR `o3` does not live long enough + o3.set0(&o1); //~ ERROR `o1` does not live long enough + o3.set1(&o2); //~ ERROR `o2` does not live long enough +} + +fn main() { + f(); +} diff --git a/src/test/compile-fail/issue-25199.rs b/src/test/compile-fail/issue-25199.rs new file mode 100644 index 0000000000000..74ea1ca2947f2 --- /dev/null +++ b/src/test/compile-fail/issue-25199.rs @@ -0,0 +1,83 @@ +// Copyright 2015 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. + +// Regression test for Issue 25199: Check that one cannot hide a +// destructor's access to borrowed data behind a boxed trait object. +// +// Prior to fixing Issue 25199, this example was able to be compiled +// with rustc, and thus when you ran it, you would see the `Drop` impl +// for `Test` accessing state that had already been dropping (which is +// marked explicitly here with checking code within the `Drop` impl +// for `VecHolder`, but in the general case could just do unsound +// things like accessing memory that has been freed). +// +// Note that I would have liked to encode my go-to example of cyclic +// structure that accesses its neighbors in drop (and thus is +// fundamentally unsound) via this trick, but the closest I was able +// to come was dropck_trait_cycle_checked.rs, which is not quite as +// "good" as this regression test because the encoding of that example +// was forced to attach a lifetime to the trait definition itself +// (`trait Obj<'a>`) while *this* example is solely + +use std::cell::RefCell; + +trait Obj { } + +struct VecHolder { + v: Vec<(bool, &'static str)>, +} + +impl Drop for VecHolder { + fn drop(&mut self) { + println!("Dropping Vec"); + self.v[30].0 = false; + self.v[30].1 = "invalid access: VecHolder dropped already"; + } +} + +struct Container<'a> { + v: VecHolder, + d: RefCell>>, +} + +impl<'a> Container<'a> { + fn new() -> Container<'a> { + Container { + d: RefCell::new(Vec::new()), + v: VecHolder { + v: vec![(true, "valid"); 100] + } + } + } + + fn store(&'a self, val: T) { + self.d.borrow_mut().push(Box::new(val)); + } +} + +struct Test<'a> { + test: &'a Container<'a>, +} + +impl<'a> Obj for Test<'a> { } +impl<'a> Drop for Test<'a> { + fn drop(&mut self) { + for e in &self.test.v.v { + assert!(e.0, e.1); + } + } +} + +fn main() { + let container = Container::new(); + let test = Test{test: &container}; //~ ERROR `container` does not live long enough + println!("container.v[30]: {:?}", container.v.v[30]); + container.store(test); //~ ERROR `container` does not live long enough +} diff --git a/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs index 7398e6f1089bc..f73b06653012a 100644 --- a/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs +++ b/src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs @@ -22,12 +22,24 @@ fn a() { let mut factorial: Option u32>> = None; let f = |x: u32| -> u32 { + //~^ ERROR `factorial` does not live long enough + let g = factorial.as_ref().unwrap(); + if x == 0 {1} else {x * g(x-1)} + }; + + factorial = Some(Box::new(f)); +} + +fn b() { + let mut factorial: Option u32 + 'static>> = None; + + let f = |x: u32| -> u32 { + //~^ ERROR closure may outlive the current function, but it borrows `factorial` let g = factorial.as_ref().unwrap(); if x == 0 {1} else {x * g(x-1)} }; factorial = Some(Box::new(f)); - //~^ ERROR cannot assign to `factorial` because it is borrowed } fn main() { } diff --git a/src/test/run-pass/issue-11205.rs b/src/test/run-pass/issue-11205.rs index 41b54727b662f..679d494a47302 100644 --- a/src/test/run-pass/issue-11205.rs +++ b/src/test/run-pass/issue-11205.rs @@ -21,7 +21,7 @@ fn foos(_: &[&Foo]) {} fn foog(_: &[T], _: &[T]) {} fn bar(_: [Box; 2]) {} -fn bars(_: &[Box]) {} +fn bars(_: &[Box]) {} fn main() { let x: [&Foo; 2] = [&1, &2]; @@ -45,11 +45,11 @@ fn main() { bar(x); bar([Box::new(1), Box::new(2)]); - let x: &[Box] = &[Box::new(1), Box::new(2)]; + let x: &[Box] = &[Box::new(1), Box::new(2)]; bars(x); bars(&[Box::new(1), Box::new(2)]); - let x: &[Box] = &[Box::new(1), Box::new(2)]; + let x: &[Box] = &[Box::new(1), Box::new(2)]; foog(x, &[Box::new(1)]); struct T<'a> {