diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index 9f45e66f0d937..f7a6dee8307d3 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -10,6 +10,10 @@ use hir::def_id::DefId; use rustc_data_structures::fx::FxHashMap; + +#[cfg(debug_assertions)] +use rustc_data_structures::fx::FxHashSet; + use std::cell::RefCell; use std::collections::hash_map::Entry; use std::ops::Index; @@ -26,6 +30,8 @@ pub struct DepTrackingMap { phantom: PhantomData, graph: DepGraph, map: FxHashMap, + #[cfg(debug_assertions)] + tombstones: RefCell>, } pub trait DepTrackingMapConfig { @@ -35,11 +41,22 @@ pub trait DepTrackingMapConfig { } impl DepTrackingMap { + #[cfg(debug_assertions)] + pub fn new(graph: DepGraph) -> DepTrackingMap { + DepTrackingMap { + phantom: PhantomData, + graph: graph, + map: FxHashMap(), + tombstones: RefCell::new(FxHashSet()), + } + } + + #[cfg(not(debug_assertions))] pub fn new(graph: DepGraph) -> DepTrackingMap { DepTrackingMap { phantom: PhantomData, graph: graph, - map: FxHashMap() + map: FxHashMap(), } } @@ -59,13 +76,31 @@ impl DepTrackingMap { pub fn get(&self, k: &M::Key) -> Option<&M::Value> { self.read(k); - self.map.get(k) + let result = self.map.get(k); + + #[cfg(debug_assertions)] + { + if result.is_none() { + self.tombstones.borrow_mut().insert(k.clone()); + } + } + + result } pub fn insert(&mut self, k: M::Key, v: M::Value) { self.write(&k); + + // If we ever read a `None` value for this key, we do not want + // to permit a later write to it. The only valid use case for + // this is the memoization pattern: for that, use `memoize()` + // below + #[cfg(debug_assertions)] + assert!(!self.tombstones.borrow().contains(&k), + "inserting to `{0:?}` after reading from `{0:?}`", + M::to_dep_node(&k)); let old_value = self.map.insert(k, v); - assert!(old_value.is_none()); + assert!(old_value.is_none(), "inserted value twice"); } pub fn entry(&mut self, k: M::Key) -> Entry { @@ -75,7 +110,13 @@ impl DepTrackingMap { pub fn contains_key(&self, k: &M::Key) -> bool { self.read(k); - self.map.contains_key(k) + if self.map.contains_key(k) { + true + } else { + #[cfg(debug_assertions)] + self.tombstones.borrow_mut().insert(k.clone()); + false + } } pub fn keys(&self) -> Vec { @@ -125,7 +166,7 @@ impl MemoizationMap for RefCell> { /// let item_def_id = ccx.tcx.hir.local_def_id(it.id); /// ccx.tcx.item_types.memoized(item_def_id, || { /// ccx.tcx.dep_graph.read(DepNode::Hir(item_def_id)); // (*) - /// compute_type_of_item(ccx, item) + /// Some(compute_type_of_item(ccx, item)) /// }); /// } /// ``` @@ -134,7 +175,7 @@ impl MemoizationMap for RefCell> { /// accesses the body of the item `item`, so we register a read /// from `Hir(item_def_id)`. fn memoize(&self, key: M::Key, op: OP) -> M::Value - where OP: FnOnce() -> M::Value + where OP: FnOnce() -> Option { let graph; { @@ -147,9 +188,18 @@ impl MemoizationMap for RefCell> { } let _task = graph.in_task(M::to_dep_node(&key)); - let result = op(); - self.borrow_mut().map.insert(key, result.clone()); - result + if let Some(result) = op() { + let old_value = self.borrow_mut().map.insert(key, result.clone()); + assert!(old_value.is_none()); + result + } else { + self.borrow().map + .get(&key) + .unwrap_or_else(|| bug!( + "memoize closure failed to write to {:?}", M::to_dep_node(&key) + )) + .clone() + } } } diff --git a/src/librustc/ty/contents.rs b/src/librustc/ty/contents.rs index e14295982916f..877cca842cf07 100644 --- a/src/librustc/ty/contents.rs +++ b/src/librustc/ty/contents.rs @@ -125,7 +125,7 @@ impl fmt::Debug for TypeContents { impl<'a, 'tcx> ty::TyS<'tcx> { pub fn type_contents(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> TypeContents { - return tcx.tc_cache.memoize(self, || tc_ty(tcx, self, &mut FxHashMap())); + return tcx.tc_cache.memoize(self, || Some(tc_ty(tcx, self, &mut FxHashMap()))); fn tc_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, ty: Ty<'tcx>, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index d7efee06db2b5..39f20b3860843 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1633,30 +1633,24 @@ impl<'a, 'gcx, 'tcx> AdtDef { /// check should catch this case. fn calculate_sized_constraint_inner(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, - stack: &mut Vec) + stack: &mut Vec<(DefId, bool)>) -> Ty<'tcx> { - if let Some(ty) = tcx.maps.adt_sized_constraint.borrow().get(&self.did) { - return ty; - } - - // Follow the memoization pattern: push the computation of - // DepNode::SizedConstraint as our current task. - let _task = tcx.dep_graph.in_task(DepNode::SizedConstraint(self.did)); + if let Some(index) = stack.iter().position(|pair| pair.0 == self.did) { + stack[index].1 = true; - if stack.contains(&self.did) { debug!("calculate_sized_constraint: {:?} is recursive", self); // This should be reported as an error by `check_representable`. // // Consider the type as Sized in the meanwhile to avoid // further errors. - tcx.maps.adt_sized_constraint.borrow_mut().insert(self.did, tcx.types.err); return tcx.types.err; } - stack.push(self.did); + tcx.maps.adt_sized_constraint.memoize(self.did, || { + stack.push((self.did, false)); - let tys : Vec<_> = + let tys: Vec<_> = self.variants.iter().flat_map(|v| { v.fields.last() }).flat_map(|f| { @@ -1664,34 +1658,26 @@ impl<'a, 'gcx, 'tcx> AdtDef { self.sized_constraint_for_ty(tcx, stack, ty) }).collect(); - let self_ = stack.pop().unwrap(); - assert_eq!(self_, self.did); + let (self_, cycle) = stack.pop().unwrap(); + assert_eq!(self_, self.did); - let ty = match tys.len() { - _ if tys.references_error() => tcx.types.err, - 0 => tcx.types.bool, - 1 => tys[0], - _ => tcx.intern_tup(&tys[..], false) - }; + let ty = if cycle || tys.references_error() { + tcx.types.err + } else { + match tys.len() { + 0 => tcx.types.bool, + 1 => tys[0], + _ => tcx.intern_tup(&tys[..], false) + } + }; - let old = tcx.maps.adt_sized_constraint.borrow().get(&self.did).cloned(); - match old { - Some(old_ty) => { - debug!("calculate_sized_constraint: {:?} recurred", self); - assert_eq!(old_ty, tcx.types.err); - old_ty - } - None => { - debug!("calculate_sized_constraint: {:?} => {:?}", self, ty); - tcx.maps.adt_sized_constraint.borrow_mut().insert(self.did, ty); - ty - } - } + Some(ty) + }) } fn sized_constraint_for_ty(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, - stack: &mut Vec, + stack: &mut Vec<(DefId, bool)>, ty: Ty<'tcx>) -> Vec> { let result = match ty.sty { @@ -2094,9 +2080,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } - // memoize wants us to return something, so return - // the one we generated for this def-id - *self.maps.associated_item.borrow().get(&def_id).unwrap() + None }) } @@ -2176,7 +2160,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } _ => span_bug!(item.span, "associated_item_def_ids: not impl or trait") }; - Rc::new(vec) + Some(Rc::new(vec)) }) } diff --git a/src/librustc/util/common.rs b/src/librustc/util/common.rs index 4ddccbfd4c597..17aae1ac48dc9 100644 --- a/src/librustc/util/common.rs +++ b/src/librustc/util/common.rs @@ -208,7 +208,7 @@ pub trait MemoizationMap { /// added into the dep graph. See the `DepTrackingMap` impl for /// more details! fn memoize(&self, key: Self::Key, op: OP) -> Self::Value - where OP: FnOnce() -> Self::Value; + where OP: FnOnce() -> Option; } impl MemoizationMap for RefCell> @@ -218,15 +218,18 @@ impl MemoizationMap for RefCell> type Value = V; fn memoize(&self, key: K, op: OP) -> V - where OP: FnOnce() -> V + where OP: FnOnce() -> Option { let result = self.borrow().get(&key).cloned(); match result { Some(result) => result, None => { - let result = op(); - self.borrow_mut().insert(key, result.clone()); - result + if let Some(result) = op() { + self.borrow_mut().insert(key, result.clone()); + result + } else { + self.borrow().get(&key).unwrap().clone() + } } } } diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index a509587f80fd0..1d4436ad586b4 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -488,7 +488,7 @@ pub fn fulfill_obligation<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, let vtable = infcx.drain_fulfillment_cx_or_panic(span, &mut fulfill_cx, &vtable); info!("Cache miss: {:?} => {:?}", trait_ref, vtable); - vtable + Some(vtable) }) }) } diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index 4b31d5b7f88de..50075e96e3f40 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -121,7 +121,7 @@ impl<'a, 'b, 'gcx> TypeFolder<'gcx, 'gcx> for AssociatedTypeNormalizer<'a, 'b, ' } else { self.shared.project_cache().memoize(ty, || { debug!("AssociatedTypeNormalizer: ty={:?}", ty); - self.shared.tcx().normalize_associated_type(&ty) + Some(self.shared.tcx().normalize_associated_type(&ty)) }) } } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index db7cf3c000ba4..92430210fd521 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -617,7 +617,7 @@ fn convert_enum_variant_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, prev_discr = Some(if let Some(e) = variant.node.disr_expr { let expr_did = tcx.hir.local_def_id(e.node_id); let result = tcx.maps.monomorphic_const_eval.memoize(expr_did, || { - evaluate_disr_expr(tcx, e) + Some(evaluate_disr_expr(tcx, e)) }); match result {