From 566aa54b494524e4af48fd0d72cada6ebf138487 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 6 May 2016 00:47:28 -0400 Subject: [PATCH] trans: Make common::fulfill_obligation only depend on SharedCrateContext. Plus make it produce a nicer dependency graph via DepTrackingMap::memoize(). --- src/librustc_trans/base.rs | 2 +- src/librustc_trans/callee.rs | 2 +- src/librustc_trans/collector.rs | 6 +- src/librustc_trans/common.rs | 112 +++++++++++++++----------------- src/librustc_trans/context.rs | 15 ++--- src/librustc_trans/glue.rs | 2 +- src/librustc_trans/meth.rs | 2 +- 7 files changed, 67 insertions(+), 74 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index bb5b1e5c7752..098b7d163c19 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -678,7 +678,7 @@ pub fn custom_coerce_unsize_info<'ccx, 'tcx>(ccx: &CrateContext<'ccx, 'tcx>, substs: ccx.tcx().mk_substs(trait_substs) }); - match fulfill_obligation(ccx, DUMMY_SP, trait_ref) { + match fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref) { traits::VtableImpl(traits::VtableImplData { impl_def_id, .. }) => { ccx.tcx().custom_coerce_unsized_kind(impl_def_id) } diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index f5288220b2ea..db605e4dc5d6 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -156,7 +156,7 @@ impl<'tcx> Callee<'tcx> { let trait_id = method_item.container().id(); let trait_ref = ty::Binder(substs.to_trait_ref(tcx, trait_id)); let trait_ref = infer::normalize_associated_type(tcx, &trait_ref); - match common::fulfill_obligation(ccx, DUMMY_SP, trait_ref) { + match common::fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref) { traits::VtableImpl(vtable_impl) => { let impl_did = vtable_impl.impl_def_id; let mname = tcx.item_name(def_id); diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 2fb8c9737fa2..db0e04c34763 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -701,7 +701,7 @@ fn find_drop_glue_neighbors<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, substs: self_type_substs, }.to_poly_trait_ref(); - let substs = match fulfill_obligation(ccx, DUMMY_SP, trait_ref) { + let substs = match fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref) { traits::VtableImpl(data) => data.substs, _ => bug!() }; @@ -844,7 +844,7 @@ fn do_static_trait_method_dispatch<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, callee_substs); let trait_ref = ty::Binder(rcvr_substs.to_trait_ref(tcx, trait_id)); - let vtbl = fulfill_obligation(ccx, DUMMY_SP, trait_ref); + let vtbl = fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref); // Now that we know which impl is being used, we can dispatch to // the actual function: @@ -999,7 +999,7 @@ fn create_trans_items_for_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Walk all methods of the trait, including those of its supertraits for trait_ref in traits::supertraits(ccx.tcx(), poly_trait_ref) { - let vtable = fulfill_obligation(ccx, DUMMY_SP, trait_ref); + let vtable = fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref); match vtable { traits::VtableImpl( traits::VtableImplData { diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index 0684355cb106..65d307cec688 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -20,6 +20,7 @@ use rustc::cfg; use rustc::hir::def::Def; use rustc::hir::def_id::DefId; use rustc::infer; +use rustc::util::common::MemoizationMap; use middle::lang_items::LangItem; use rustc::ty::subst::Substs; use abi::{Abi, FnType}; @@ -54,7 +55,7 @@ use syntax::codemap::{DUMMY_SP, Span}; use syntax::parse::token::InternedString; use syntax::parse::token; -pub use context::CrateContext; +pub use context::{CrateContext, SharedCrateContext}; /// Is the type's representation size known at compile time? pub fn type_is_sized<'tcx>(tcx: &TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { @@ -1051,7 +1052,7 @@ pub fn expr_ty_adjusted<'blk, 'tcx>(bcx: &BlockS<'blk, 'tcx>, ex: &hir::Expr) -> /// Attempts to resolve an obligation. The result is a shallow vtable resolution -- meaning that we /// do not (necessarily) resolve all nested obligations on the impl. Note that type check should /// guarantee to us that all nested obligations *could be* resolved if we wanted to. -pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, +pub fn fulfill_obligation<'a, 'tcx>(ccx: &SharedCrateContext<'a, 'tcx>, span: Span, trait_ref: ty::PolyTraitRef<'tcx>) -> traits::Vtable<'tcx, ()> @@ -1061,68 +1062,63 @@ pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Remove any references to regions; this helps improve caching. let trait_ref = tcx.erase_regions(&trait_ref); - // First check the cache. - match ccx.trait_cache().borrow().get(&trait_ref) { - Some(vtable) => { - info!("Cache hit: {:?}", trait_ref); - return (*vtable).clone(); - } - None => { } - } + ccx.trait_cache().memoize(trait_ref, || { + debug!("trans fulfill_obligation: trait_ref={:?} def_id={:?}", + trait_ref, trait_ref.def_id()); - debug!("trans fulfill_obligation: trait_ref={:?} def_id={:?}", - trait_ref, trait_ref.def_id()); + // Do the initial selection for the obligation. This yields the + // shallow result we are looking for -- that is, what specific impl. + let infcx = infer::normalizing_infer_ctxt(tcx, + &tcx.tables, + ProjectionMode::Any); + let mut selcx = SelectionContext::new(&infcx); + let obligation_cause = traits::ObligationCause::misc(span, + ast::DUMMY_NODE_ID); + let obligation = traits::Obligation::new(obligation_cause, + trait_ref.to_poly_trait_predicate()); - // Do the initial selection for the obligation. This yields the - // shallow result we are looking for -- that is, what specific impl. - let infcx = infer::normalizing_infer_ctxt(tcx, &tcx.tables, ProjectionMode::Any); - let mut selcx = SelectionContext::new(&infcx); + let selection = match selcx.select(&obligation) { + Ok(Some(selection)) => selection, + Ok(None) => { + // Ambiguity can happen when monomorphizing during trans + // expands to some humongo type that never occurred + // statically -- this humongo type can then overflow, + // leading to an ambiguous result. So report this as an + // overflow bug, since I believe this is the only case + // where ambiguity can result. + debug!("Encountered ambiguity selecting `{:?}` during trans, \ + presuming due to overflow", + trait_ref); + ccx.sess().span_fatal( + span, + "reached the recursion limit during monomorphization \ + (selection ambiguity)"); + } + Err(e) => { + span_bug!( + span, + "Encountered error `{:?}` selecting `{:?}` during trans", + e, + trait_ref) + } + }; - let obligation = - traits::Obligation::new(traits::ObligationCause::misc(span, ast::DUMMY_NODE_ID), - trait_ref.to_poly_trait_predicate()); - let selection = match selcx.select(&obligation) { - Ok(Some(selection)) => selection, - Ok(None) => { - // Ambiguity can happen when monomorphizing during trans - // expands to some humongo type that never occurred - // statically -- this humongo type can then overflow, - // leading to an ambiguous result. So report this as an - // overflow bug, since I believe this is the only case - // where ambiguity can result. - debug!("Encountered ambiguity selecting `{:?}` during trans, \ - presuming due to overflow", - trait_ref); - ccx.sess().span_fatal( - span, - "reached the recursion limit during monomorphization (selection ambiguity)"); - } - Err(e) => { - span_bug!( - span, - "Encountered error `{:?}` selecting `{:?}` during trans", - e, - trait_ref) - } - }; + // Currently, we use a fulfillment context to completely resolve + // all nested obligations. This is because they can inform the + // inference of the impl's type parameters. + let mut fulfill_cx = traits::FulfillmentContext::new(); + let vtable = selection.map(|predicate| { + fulfill_cx.register_predicate_obligation(&infcx, predicate); + }); + let vtable = infer::drain_fulfillment_cx_or_panic( + span, &infcx, &mut fulfill_cx, &vtable + ); - // Currently, we use a fulfillment context to completely resolve - // all nested obligations. This is because they can inform the - // inference of the impl's type parameters. - let mut fulfill_cx = traits::FulfillmentContext::new(); - let vtable = selection.map(|predicate| { - fulfill_cx.register_predicate_obligation(&infcx, predicate); - }); - let vtable = infer::drain_fulfillment_cx_or_panic( - span, &infcx, &mut fulfill_cx, &vtable - ); + info!("Cache miss: {:?} => {:?}", trait_ref, vtable); - info!("Cache miss: {:?} => {:?}", trait_ref, vtable); - - ccx.trait_cache().borrow_mut().insert(trait_ref, vtable.clone()); - - vtable + vtable + }) } /// Normalizes the predicates and checks whether they hold. If this diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 7db42bb00ee1..d7ec448a7f0b 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -84,6 +84,7 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { use_dll_storage_attrs: bool, translation_items: RefCell, TransItemState>>, + trait_cache: RefCell>>, } /// The local portion of a `CrateContext`. There is one `LocalCrateContext` @@ -169,8 +170,6 @@ pub struct LocalCrateContext<'tcx> { /// Depth of the current type-of computation - used to bail out type_of_depth: Cell, - - trait_cache: RefCell>>, } // Implement DepTrackingMapConfig for `trait_cache` @@ -423,6 +422,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { available_drop_glues: RefCell::new(FnvHashMap()), use_dll_storage_attrs: use_dll_storage_attrs, translation_items: RefCell::new(FnvHashMap()), + trait_cache: RefCell::new(DepTrackingMap::new(tcx.dep_graph.clone())), } } @@ -446,6 +446,10 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { &self.item_symbols } + pub fn trait_cache(&self) -> &RefCell>> { + &self.trait_cache + } + pub fn link_meta<'a>(&'a self) -> &'a LinkMeta { &self.link_meta } @@ -516,9 +520,6 @@ impl<'tcx> LocalCrateContext<'tcx> { intrinsics: RefCell::new(FnvHashMap()), n_llvm_insns: Cell::new(0), type_of_depth: Cell::new(0), - trait_cache: RefCell::new(DepTrackingMap::new(shared.tcx - .dep_graph - .clone())), }; let (int_type, opaque_vec_type, str_slice_ty, mut local_ccx) = { @@ -805,10 +806,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.local().n_llvm_insns.set(self.local().n_llvm_insns.get() + 1); } - pub fn trait_cache(&self) -> &RefCell>> { - &self.local().trait_cache - } - pub fn obj_size_bound(&self) -> u64 { self.tcx().data_layout.obj_size_bound() } diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 06761a7016ad..06f9000c4ba9 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -364,7 +364,7 @@ fn trans_struct_drop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, def_id: tcx.lang_items.drop_trait().unwrap(), substs: tcx.mk_substs(Substs::empty().with_self_ty(t)) }); - let vtbl = match fulfill_obligation(bcx.ccx(), DUMMY_SP, trait_ref) { + let vtbl = match fulfill_obligation(bcx.ccx().shared(), DUMMY_SP, trait_ref) { traits::VtableImpl(data) => data, _ => bug!("dtor for {:?} is not an impl???", t) }; diff --git a/src/librustc_trans/meth.rs b/src/librustc_trans/meth.rs index d08c26783e98..648a232ef694 100644 --- a/src/librustc_trans/meth.rs +++ b/src/librustc_trans/meth.rs @@ -144,7 +144,7 @@ pub fn get_vtable<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Not in the cache. Build it. let methods = traits::supertraits(tcx, trait_ref.clone()).flat_map(|trait_ref| { - let vtable = fulfill_obligation(ccx, DUMMY_SP, trait_ref.clone()); + let vtable = fulfill_obligation(ccx.shared(), DUMMY_SP, trait_ref.clone()); match vtable { // Should default trait error here? traits::VtableDefaultImpl(_) |