From 9aac5fc0f3754bf8a7ec63119895530616ac7ccc Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 24 Apr 2017 20:27:59 +0300 Subject: [PATCH] refactor away trans::symbol_map --- src/librustc_trans/back/symbol_export.rs | 26 +---- src/librustc_trans/base.rs | 65 +++++++++-- src/librustc_trans/lib.rs | 1 - src/librustc_trans/symbol_map.rs | 131 ----------------------- src/librustc_trans/trans_item.rs | 15 ++- 5 files changed, 70 insertions(+), 168 deletions(-) delete mode 100644 src/librustc_trans/symbol_map.rs diff --git a/src/librustc_trans/back/symbol_export.rs b/src/librustc_trans/back/symbol_export.rs index 467bc6cbfc6..ddd86c46799 100644 --- a/src/librustc_trans/back/symbol_export.rs +++ b/src/librustc_trans/back/symbol_export.rs @@ -10,13 +10,11 @@ use context::SharedCrateContext; use monomorphize::Instance; -use symbol_map::SymbolMap; use util::nodemap::FxHashMap; use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE}; use rustc::session::config; use rustc::ty::TyCtxt; use syntax::attr; -use trans_item::TransItem; /// The SymbolExportLevel of a symbols specifies from which kinds of crates /// the symbol will be exported. `C` symbols will be exported from any @@ -35,16 +33,13 @@ pub struct ExportedSymbols { } impl ExportedSymbols { - pub fn empty() -> ExportedSymbols { ExportedSymbols { exports: FxHashMap(), } } - pub fn compute_from<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, - symbol_map: &SymbolMap<'tcx>) - -> ExportedSymbols { + pub fn compute<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) -> ExportedSymbols { let mut local_crate: Vec<_> = scx .exported_symbols() .iter() @@ -52,7 +47,7 @@ pub fn compute_from<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, scx.tcx().hir.local_def_id(node_id) }) .map(|def_id| { - let name = symbol_for_def_id(scx.tcx(), def_id, symbol_map); + let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id)); let export_level = export_level(scx, def_id); debug!("EXPORTED SYMBOL (local): {} ({:?})", name, export_level); (str::to_owned(&name), export_level) @@ -212,20 +207,3 @@ pub fn is_below_threshold(level: SymbolExportLevel, level == SymbolExportLevel::C } } - -fn symbol_for_def_id<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - def_id: DefId, - symbol_map: &SymbolMap<'tcx>) - -> String { - // Just try to look things up in the symbol map. If nothing's there, we - // recompute. - if let Some(node_id) = tcx.hir.as_local_node_id(def_id) { - if let Some(sym) = symbol_map.get(TransItem::Static(node_id)) { - return sym.to_owned(); - } - } - - let instance = Instance::mono(tcx, def_id); - - str::to_owned(&tcx.symbol_name(instance)) -} diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 12d077a5507..56ff5ebb069 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -65,7 +65,6 @@ use mir; use monomorphize::{self, Instance}; use partitioning::{self, PartitioningStrategy, CodegenUnit}; -use symbol_map::SymbolMap; use symbol_names_test; use trans_item::{TransItem, DefPathBasedNames}; use type_::Type; @@ -803,7 +802,6 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, scx: &SharedCrateContext<'a, 'tcx>, translation_items: &FxHashSet>, llvm_modules: &[ModuleLlvm], - symbol_map: &SymbolMap<'tcx>, exported_symbols: &ExportedSymbols) { let export_threshold = symbol_export::crates_export_threshold(&sess.crate_types.borrow()); @@ -855,7 +853,7 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, let mut linkage_fixed_explicitly = FxHashSet(); for trans_item in translation_items { - let symbol_name = symbol_map.get_or_compute(scx, *trans_item); + let symbol_name = str::to_owned(&trans_item.symbol_name(tcx)); if trans_item.explicit_linkage(tcx).is_some() { linkage_fixed_explicitly.insert(symbol_name.clone()); } @@ -1109,7 +1107,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Run the translation item collector and partition the collected items into // codegen units. - let (translation_items, codegen_units, symbol_map) = + let (translation_items, codegen_units) = collect_and_partition_translation_items(&shared_ccx); let mut all_stats = Stats::default(); @@ -1269,8 +1267,7 @@ fn module_translation<'a, 'tcx>( let sess = shared_ccx.sess(); - let exported_symbols = ExportedSymbols::compute_from(&shared_ccx, - &symbol_map); + let exported_symbols = ExportedSymbols::compute(&shared_ccx); // Get the list of llvm modules we created. We'll do a few wacky // transforms on them now. @@ -1290,7 +1287,6 @@ fn module_translation<'a, 'tcx>( &shared_ccx, &translation_items, &llvm_modules, - &symbol_map, &exported_symbols); }); @@ -1516,10 +1512,57 @@ enum Fields<'a> { } } +#[inline(never)] // give this a place in the profiler +fn assert_symbols_are_distinct<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_items: I) + where I: Iterator> +{ + let mut symbols: Vec<_> = trans_items.map(|trans_item| { + (trans_item, trans_item.symbol_name(tcx)) + }).collect(); + + (&mut symbols[..]).sort_by(|&(_, ref sym1), &(_, ref sym2)|{ + sym1.cmp(sym2) + }); + + for pair in (&symbols[..]).windows(2) { + let sym1 = &pair[0].1; + let sym2 = &pair[1].1; + + if *sym1 == *sym2 { + let trans_item1 = pair[0].0; + let trans_item2 = pair[1].0; + + let span1 = trans_item1.local_span(tcx); + let span2 = trans_item2.local_span(tcx); + + // Deterministically select one of the spans for error reporting + let span = match (span1, span2) { + (Some(span1), Some(span2)) => { + Some(if span1.lo.0 > span2.lo.0 { + span1 + } else { + span2 + }) + } + (Some(span), None) | + (None, Some(span)) => Some(span), + _ => None + }; + + let error_message = format!("symbol `{}` is already defined", sym1); + + if let Some(span) = span { + tcx.sess.span_fatal(span, &error_message) + } else { + tcx.sess.fatal(&error_message) + } + } + } +} + fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) -> (FxHashSet>, - Vec>, - SymbolMap<'tcx>) { + Vec>) { let time_passes = scx.sess().time_passes(); let collection_mode = match scx.sess().opts.debugging_opts.print_trans_items { @@ -1547,7 +1590,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a collector::collect_crate_translation_items(&scx, collection_mode) }); - let symbol_map = SymbolMap::build(scx, items.iter().cloned()); + assert_symbols_are_distinct(scx.tcx(), items.iter()); let strategy = if scx.sess().opts.debugging_opts.incremental.is_some() { PartitioningStrategy::PerModule @@ -1620,5 +1663,5 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a } } - (translation_items, codegen_units, symbol_map) + (translation_items, codegen_units) } diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index faddffb65fa..7bc54e22265 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -125,7 +125,6 @@ pub mod back { mod mir; mod monomorphize; mod partitioning; -mod symbol_map; mod symbol_names_test; mod trans_item; mod tvec; diff --git a/src/librustc_trans/symbol_map.rs b/src/librustc_trans/symbol_map.rs deleted file mode 100644 index 85a8d501753..00000000000 --- a/src/librustc_trans/symbol_map.rs +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright 2016 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. - -use context::SharedCrateContext; -use monomorphize::Instance; -use rustc::ty::TyCtxt; -use std::borrow::Cow; -use syntax::codemap::Span; -use trans_item::TransItem; -use util::nodemap::FxHashMap; - -// In the SymbolMap we collect the symbol names of all translation items of -// the current crate. This map exists as a performance optimization. Symbol -// names of translation items are deterministic and fully defined by the item. -// Thus they could also always be recomputed if needed. - -pub struct SymbolMap<'tcx> { - index: FxHashMap, (usize, usize)>, - arena: String, -} - -impl<'tcx> SymbolMap<'tcx> { - - pub fn build<'a, I>(scx: &SharedCrateContext<'a, 'tcx>, - trans_items: I) - -> SymbolMap<'tcx> - where I: Iterator> - { - // Check for duplicate symbol names - let tcx = scx.tcx(); - let mut symbols: Vec<_> = trans_items.map(|trans_item| { - (trans_item, trans_item.symbol_name(tcx)) - }).collect(); - - (&mut symbols[..]).sort_by(|&(_, ref sym1), &(_, ref sym2)|{ - sym1.cmp(sym2) - }); - - for pair in (&symbols[..]).windows(2) { - let sym1 = &pair[0].1; - let sym2 = &pair[1].1; - - if *sym1 == *sym2 { - let trans_item1 = pair[0].0; - let trans_item2 = pair[1].0; - - let span1 = get_span(scx.tcx(), trans_item1); - let span2 = get_span(scx.tcx(), trans_item2); - - // Deterministically select one of the spans for error reporting - let span = match (span1, span2) { - (Some(span1), Some(span2)) => { - Some(if span1.lo.0 > span2.lo.0 { - span1 - } else { - span2 - }) - } - (Some(span), None) | - (None, Some(span)) => Some(span), - _ => None - }; - - let error_message = format!("symbol `{}` is already defined", sym1); - - if let Some(span) = span { - scx.sess().span_fatal(span, &error_message) - } else { - scx.sess().fatal(&error_message) - } - } - } - - let mut symbol_map = SymbolMap { - index: FxHashMap(), - arena: String::with_capacity(1024), - }; - - for (trans_item, symbol) in symbols { - let start_index = symbol_map.arena.len(); - symbol_map.arena.push_str(&symbol[..]); - let end_index = symbol_map.arena.len(); - let prev_entry = symbol_map.index.insert(trans_item, - (start_index, end_index)); - if prev_entry.is_some() { - bug!("TransItem encountered twice?") - } - } - - fn get_span<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - trans_item: TransItem<'tcx>) -> Option { - match trans_item { - TransItem::Fn(Instance { def, .. }) => { - tcx.hir.as_local_node_id(def.def_id()) - } - TransItem::Static(node_id) | - TransItem::GlobalAsm(node_id) => { - Some(node_id) - } - }.map(|node_id| { - tcx.hir.span(node_id) - }) - } - - symbol_map - } - - pub fn get(&self, trans_item: TransItem<'tcx>) -> Option<&str> { - self.index.get(&trans_item).map(|&(start_index, end_index)| { - &self.arena[start_index .. end_index] - }) - } - - pub fn get_or_compute<'map, 'scx>(&'map self, - scx: &SharedCrateContext<'scx, 'tcx>, - trans_item: TransItem<'tcx>) - -> Cow<'map, str> { - if let Some(sym) = self.get(trans_item) { - Cow::from(sym) - } else { - Cow::from(str::to_owned(&trans_item.symbol_name(scx.tcx()))) - } - } -} diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index f953db94fff..392ee71d52b 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -28,9 +28,10 @@ use rustc::hir::def_id::DefId; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::subst::Substs; -use syntax_pos::symbol::Symbol; use syntax::ast::{self, NodeId}; use syntax::attr; +use syntax_pos::Span; +use syntax_pos::symbol::Symbol; use type_of; use std::fmt::Write; use std::iter; @@ -200,6 +201,18 @@ pub fn symbol_name(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> ty::SymbolName { } } + pub fn local_span(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { + match *self { + TransItem::Fn(Instance { def, .. }) => { + tcx.hir.as_local_node_id(def.def_id()) + } + TransItem::Static(node_id) | + TransItem::GlobalAsm(node_id) => { + Some(node_id) + } + }.map(|node_id| tcx.hir.span(node_id)) + } + pub fn instantiation_mode(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> InstantiationMode { -- 2.44.0