From eeb37b76afd33be8cb13bc02ce472fa46945c80f Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Sun, 9 Mar 2014 13:42:22 +0200 Subject: [PATCH] De-@ reachable. --- src/librustc/driver/driver.rs | 19 ++-- src/librustc/middle/reachable.rs | 132 ++++++++----------------- src/librustc/middle/trans/base.rs | 57 ++++------- src/librustc/middle/trans/context.rs | 4 +- src/librustc/middle/trans/debuginfo.rs | 5 +- 5 files changed, 69 insertions(+), 148 deletions(-) diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 0c1b9d5732c..6c1c38aa897 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -276,7 +276,7 @@ pub struct CrateAnalysis { public_items: middle::privacy::PublicItems, ty_cx: ty::ctxt, maps: astencode::Maps, - reachable: @RefCell, + reachable: NodeSet, } /// Run the resolution, typechecking, region checking and other @@ -377,16 +377,13 @@ pub fn phase_3_run_analysis_passes(sess: Session, time(time_passes, "reachability checking", (), |_| reachable::find_reachable(&ty_cx, method_map, &exported_items)); - { - let reachable_map = reachable_map.borrow(); - time(time_passes, "death checking", (), |_| { - middle::dead::check_crate(&ty_cx, - method_map, - &exported_items, - reachable_map.get(), - krate) - }); - } + time(time_passes, "death checking", (), |_| { + middle::dead::check_crate(&ty_cx, + method_map, + &exported_items, + &reachable_map, + krate) + }); time(time_passes, "lint checking", (), |_| lint::check_crate(&ty_cx, method_map, &exported_items, krate)); diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index 11beadd40f7..44a0fb61b24 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -20,7 +20,6 @@ use middle::typeck; use middle::privacy; use util::nodemap::NodeSet; -use std::cell::RefCell; use std::vec_ng::Vec; use collections::HashSet; use syntax::ast; @@ -90,27 +89,19 @@ struct ReachableContext<'a> { // methods they've been resolved to. method_map: typeck::MethodMap, // The set of items which must be exported in the linkage sense. - reachable_symbols: @RefCell, + reachable_symbols: NodeSet, // A worklist of item IDs. Each item ID in this worklist will be inlined // and will be scanned for further references. - worklist: @RefCell >, + worklist: Vec, } -struct MarkSymbolVisitor<'a> { - worklist: @RefCell>, - method_map: typeck::MethodMap, - tcx: &'a ty::ctxt, - reachable_symbols: @RefCell, -} - -impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { +impl<'a> Visitor<()> for ReachableContext<'a> { fn visit_expr(&mut self, expr: &ast::Expr, _: ()) { match expr.node { ast::ExprPath(_) => { - let def_map = self.tcx.def_map.borrow(); - let def = match def_map.get().find(&expr.id) { + let def = match self.tcx.def_map.borrow().get().find(&expr.id) { Some(&def) => def, None => { self.tcx.sess.span_bug(expr.span, @@ -120,12 +111,8 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { let def_id = def_id_of_def(def); if is_local(def_id) { - if ReachableContext:: - def_id_represents_local_inlined_item(self.tcx, def_id) { - { - let mut worklist = self.worklist.borrow_mut(); - worklist.get().push(def_id.node) - } + if self.def_id_represents_local_inlined_item(def_id) { + self.worklist.push(def_id.node) } else { match def { // If this path leads to a static, then we may have @@ -133,16 +120,13 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { // is indeed reachable (address_insignificant // statics are *never* reachable). ast::DefStatic(..) => { - let mut worklist = self.worklist.borrow_mut(); - worklist.get().push(def_id.node); + self.worklist.push(def_id.node); } // If this wasn't a static, then this destination is // surely reachable. _ => { - let mut reachable_symbols = - self.reachable_symbols.borrow_mut(); - reachable_symbols.get().insert(def_id.node); + self.reachable_symbols.insert(def_id.node); } } } @@ -153,13 +137,10 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { match self.method_map.borrow().get().get(&method_call).origin { typeck::MethodStatic(def_id) => { if is_local(def_id) { - if ReachableContext:: - def_id_represents_local_inlined_item( - self.tcx, - def_id) { - self.worklist.borrow_mut().get().push(def_id.node) + if self.def_id_represents_local_inlined_item(def_id) { + self.worklist.push(def_id.node) } - self.reachable_symbols.borrow_mut().get().insert(def_id.node); + self.reachable_symbols.insert(def_id.node); } } _ => {} @@ -183,21 +164,20 @@ impl<'a> ReachableContext<'a> { ReachableContext { tcx: tcx, method_map: method_map, - reachable_symbols: @RefCell::new(NodeSet::new()), - worklist: @RefCell::new(Vec::new()), + reachable_symbols: NodeSet::new(), + worklist: Vec::new(), } } // Returns true if the given def ID represents a local item that is // eligible for inlining and false otherwise. - fn def_id_represents_local_inlined_item(tcx: &ty::ctxt, def_id: ast::DefId) - -> bool { + fn def_id_represents_local_inlined_item(&self, def_id: ast::DefId) -> bool { if def_id.krate != ast::LOCAL_CRATE { return false } let node_id = def_id.node; - match tcx.map.find(node_id) { + match self.tcx.map.find(node_id) { Some(ast_map::NodeItem(item)) => { match item.node { ast::ItemFn(..) => item_might_be_inlined(item), @@ -215,11 +195,11 @@ impl<'a> ReachableContext<'a> { attributes_specify_inlining(method.attrs.as_slice()) { true } else { - let impl_did = tcx.map.get_parent_did(node_id); + let impl_did = self.tcx.map.get_parent_did(node_id); // Check the impl. If the generics on the self type of the // impl require inlining, this method does too. assert!(impl_did.krate == ast::LOCAL_CRATE); - match tcx.map.expect_item(impl_did.node).node { + match self.tcx.map.expect_item(impl_did.node).node { ast::ItemImpl(ref generics, _, _, _) => { generics_require_inlining(generics) } @@ -232,40 +212,21 @@ impl<'a> ReachableContext<'a> { } } - // Helper function to set up a visitor for `propagate()` below. - fn init_visitor(&self) -> MarkSymbolVisitor<'a> { - let (worklist, method_map) = (self.worklist, self.method_map); - let (tcx, reachable_symbols) = (self.tcx, self.reachable_symbols); - - MarkSymbolVisitor { - worklist: worklist, - method_map: method_map, - tcx: tcx, - reachable_symbols: reachable_symbols, - } - } - // Step 2: Mark all symbols that the symbols on the worklist touch. - fn propagate(&self) { - let mut visitor = self.init_visitor(); + fn propagate(&mut self) { let mut scanned = HashSet::new(); loop { - let search_item = { - let mut worklist = self.worklist.borrow_mut(); - if worklist.get().len() == 0 { - break - } - let search_item = worklist.get().pop().unwrap(); - if scanned.contains(&search_item) { - continue - } - search_item - }; + if self.worklist.len() == 0 { + break + } + let search_item = self.worklist.pop().unwrap(); + if scanned.contains(&search_item) { + continue + } scanned.insert(search_item); match self.tcx.map.find(search_item) { - Some(ref item) => self.propagate_node(item, search_item, - &mut visitor), + Some(ref item) => self.propagate_node(item, search_item), None if search_item == ast::CRATE_NODE_ID => {} None => { self.tcx.sess.bug(format!("found unmapped ID in worklist: \ @@ -276,9 +237,8 @@ impl<'a> ReachableContext<'a> { } } - fn propagate_node(&self, node: &ast_map::Node, - search_item: ast::NodeId, - visitor: &mut MarkSymbolVisitor) { + fn propagate_node(&mut self, node: &ast_map::Node, + search_item: ast::NodeId) { if !self.tcx.sess.building_library.get() { // If we are building an executable, then there's no need to flag // anything as external except for `extern fn` types. These @@ -289,9 +249,7 @@ impl<'a> ReachableContext<'a> { ast_map::NodeItem(item) => { match item.node { ast::ItemFn(_, ast::ExternFn, _, _, _) => { - let mut reachable_symbols = - self.reachable_symbols.borrow_mut(); - reachable_symbols.get().insert(search_item); + self.reachable_symbols.insert(search_item); } _ => {} } @@ -303,8 +261,7 @@ impl<'a> ReachableContext<'a> { // continue to participate in linkage after this product is // produced. In this case, we traverse the ast node, recursing on // all reachable nodes from this one. - let mut reachable_symbols = self.reachable_symbols.borrow_mut(); - reachable_symbols.get().insert(search_item); + self.reachable_symbols.insert(search_item); } match *node { @@ -312,7 +269,7 @@ impl<'a> ReachableContext<'a> { match item.node { ast::ItemFn(_, _, _, _, search_block) => { if item_might_be_inlined(item) { - visit::walk_block(visitor, search_block, ()) + visit::walk_block(self, search_block, ()) } } @@ -321,9 +278,7 @@ impl<'a> ReachableContext<'a> { ast::ItemStatic(..) => { if attr::contains_name(item.attrs.as_slice(), "address_insignificant") { - let mut reachable_symbols = - self.reachable_symbols.borrow_mut(); - reachable_symbols.get().remove(&search_item); + self.reachable_symbols.remove(&search_item); } } @@ -348,14 +303,14 @@ impl<'a> ReachableContext<'a> { // Keep going, nothing to get exported } ast::Provided(ref method) => { - visit::walk_block(visitor, method.body, ()) + visit::walk_block(self, method.body, ()) } } } ast_map::NodeMethod(method) => { let did = self.tcx.map.get_parent_did(search_item); if method_might_be_inlined(self.tcx, method, did) { - visit::walk_block(visitor, method.body, ()) + visit::walk_block(self, method.body, ()) } } // Nothing to recurse on for these @@ -375,13 +330,10 @@ impl<'a> ReachableContext<'a> { // FIXME(pcwalton): This is a conservative overapproximation, but fixing // this properly would result in the necessity of computing *type* // reachability, which might result in a compile time loss. - fn mark_destructors_reachable(&self) { - let destructor_for_type = self.tcx.destructor_for_type.borrow(); - for (_, destructor_def_id) in destructor_for_type.get().iter() { + fn mark_destructors_reachable(&mut self) { + for (_, destructor_def_id) in self.tcx.destructor_for_type.borrow().get().iter() { if destructor_def_id.krate == ast::LOCAL_CRATE { - let mut reachable_symbols = self.reachable_symbols - .borrow_mut(); - reachable_symbols.get().insert(destructor_def_id.node); + self.reachable_symbols.insert(destructor_def_id.node); } } } @@ -390,27 +342,25 @@ impl<'a> ReachableContext<'a> { pub fn find_reachable(tcx: &ty::ctxt, method_map: typeck::MethodMap, exported_items: &privacy::ExportedItems) - -> @RefCell { - let reachable_context = ReachableContext::new(tcx, method_map); + -> NodeSet { + let mut reachable_context = ReachableContext::new(tcx, method_map); // Step 1: Seed the worklist with all nodes which were found to be public as // a result of the privacy pass along with all local lang items. If // other crates link to us, they're going to expect to be able to // use the lang items, so we need to be sure to mark them as // exported. - let mut worklist = reachable_context.worklist.borrow_mut(); for &id in exported_items.iter() { - worklist.get().push(id); + reachable_context.worklist.push(id); } for (_, item) in tcx.lang_items.items() { match *item { Some(did) if is_local(did) => { - worklist.get().push(did.node); + reachable_context.worklist.push(did.node); } _ => {} } } - drop(worklist); // Step 2: Mark all symbols that the symbols on the worklist touch. reachable_context.propagate(); diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index b151799e504..6dff780cc3c 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1785,16 +1785,10 @@ pub fn trans_mod(ccx: &CrateContext, m: &ast::Mod) { fn finish_register_fn(ccx: &CrateContext, sp: Span, sym: ~str, node_id: ast::NodeId, llfn: ValueRef) { - { - let mut item_symbols = ccx.item_symbols.borrow_mut(); - item_symbols.get().insert(node_id, sym); - } + ccx.item_symbols.borrow_mut().get().insert(node_id, sym); - { - let reachable = ccx.reachable.borrow(); - if !reachable.get().contains(&node_id) { - lib::llvm::SetLinkage(llfn, lib::llvm::InternalLinkage); - } + if !ccx.reachable.contains(&node_id) { + lib::llvm::SetLinkage(llfn, lib::llvm::InternalLinkage); } if is_entry_fn(ccx.sess(), node_id) && !ccx.sess().building_library.get() { @@ -1995,27 +1989,17 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef { llvm::LLVMAddGlobal(ccx.llmod, llty, buf) }); - { - let reachable = ccx.reachable.borrow(); - if !reachable.get().contains(&id) { - lib::llvm::SetLinkage( - g, - lib::llvm::InternalLinkage); - } + if !ccx.reachable.contains(&id) { + lib::llvm::SetLinkage(g, lib::llvm::InternalLinkage); } // Apply the `unnamed_addr` attribute if // requested if attr::contains_name(i.attrs.as_slice(), - "address_insignificant"){ - { - let reachable = - ccx.reachable.borrow(); - if reachable.get().contains(&id) { - ccx.sess().span_bug(i.span, - "insignificant static is \ - reachable"); - } + "address_insignificant") { + if ccx.reachable.contains(&id) { + ccx.sess().span_bug(i.span, + "insignificant static is reachable"); } lib::llvm::SetUnnamedAddr(g, true); @@ -2180,11 +2164,8 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef { // foreign items (extern fns and extern statics) don't have internal // linkage b/c that doesn't quite make sense. Otherwise items can // have internal linkage if they're not reachable. - { - let reachable = ccx.reachable.borrow(); - if !foreign && !reachable.get().contains(&id) { - lib::llvm::SetLinkage(val, lib::llvm::InternalLinkage); - } + if !foreign && !ccx.reachable.contains(&id) { + lib::llvm::SetLinkage(val, lib::llvm::InternalLinkage); } let mut item_vals = ccx.item_vals.borrow_mut(); @@ -2557,7 +2538,7 @@ pub fn trans_crate(krate: ast::Crate, analysis.maps, Sha256::new(), link_meta, - analysis.reachable); + &analysis.reachable); { let _icx = push_ctxt("text"); trans_mod(ccx, &krate.module); @@ -2627,13 +2608,9 @@ pub fn trans_crate(krate: ast::Crate, let link_meta = ccx.link_meta.clone(); let llmod = ccx.llmod; - let mut reachable: Vec<~str> = { - let reachable_map = ccx.reachable.borrow(); - reachable_map.get().iter().filter_map(|id| { - let item_symbols = ccx.item_symbols.borrow(); - item_symbols.get().find(id).map(|s| s.to_owned()) - }).collect() - }; + let mut reachable: Vec<~str> = ccx.reachable.iter().filter_map(|id| { + ccx.item_symbols.borrow().get().find(id).map(|s| s.to_owned()) + }).collect(); // Make sure that some other crucial symbols are not eliminated from the // module. This includes the main function, the crate map (used for debug @@ -2647,12 +2624,12 @@ pub fn trans_crate(krate: ast::Crate, reachable.push(~"rust_eh_personality"); // referenced from .eh_frame section on some platforms reachable.push(~"rust_eh_personality_catch"); // referenced from rt/rust_try.ll - return CrateTranslation { + CrateTranslation { context: llcx, module: llmod, link: link_meta, metadata_module: ccx.metadata_llmod, metadata: metadata, reachable: reachable, - }; + } } diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index b135569eb43..1d257cfefcc 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -48,7 +48,7 @@ pub struct CrateContext<'a> { intrinsics: HashMap<&'static str, ValueRef>, item_vals: RefCell>, exp_map2: resolve::ExportMap2, - reachable: @RefCell, + reachable: &'a NodeSet, item_symbols: RefCell>, link_meta: LinkMeta, drop_glues: RefCell>, @@ -123,7 +123,7 @@ impl<'a> CrateContext<'a> { maps: astencode::Maps, symbol_hasher: Sha256, link_meta: LinkMeta, - reachable: @RefCell) + reachable: &'a NodeSet) -> CrateContext<'a> { unsafe { let llcx = llvm::LLVMContextCreate(); diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 5ad5c813339..a917cdd7df7 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -652,10 +652,7 @@ pub fn create_function_debug_context(cx: &CrateContext, // (by being externally visible or by being inlined into something externally visible). It might // better to use the `exported_items` set from `driver::CrateAnalysis` in the future, but (atm) // this set is not available in the translation pass. - let is_local_to_unit = { - let reachable = cx.reachable.borrow(); - !reachable.get().contains(&fn_ast_id) - }; + let is_local_to_unit = !cx.reachable.contains(&fn_ast_id); let fn_metadata = function_name.with_c_str(|function_name| { linkage_name.with_c_str(|linkage_name| {