rustc: Remove another global map from trans

This commit removes the `crate_trans_items` field from the `CrateContext` of
trans. This field, a big map, was calculated during partioning and was a set of
all translation items. This isn't quite incremental-friendly because the map may
change a lot but not have much effect on downstream consumers.

Instead a new query was added for the one location this map was needed, along
with a new comment explaining what the location is doing!

Author: alexcrichton

src/librustc/dep_graph/dep_node.rs

@@ -579,6 +579,7 @@ define_dep_nodes!( <'tcx>
     [] CollectAndPartitionTranslationItems,
     [] ExportName(DefId),
     [] ContainsExternIndicator(DefId),
+    [] IsTranslatedFunction(DefId),
 );
 
 trait DepNodeParams<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> : fmt::Debug {

src/librustc/ty/maps.rs

@@ -24,7 +24,7 @@ use middle::resolve_lifetime::{Region, ObjectLifetimeDefault};
 use middle::stability::{self, DeprecationEntry};
 use middle::lang_items::{LanguageItems, LangItem};
 use middle::exported_symbols::SymbolExportLevel;
-use middle::trans::{TransItem, CodegenUnit};
+use middle::trans::CodegenUnit;
 use mir;
 use mir::transform::{MirSuite, MirPassIndex};
 use session::CompileResult;
@@ -1391,9 +1391,10 @@ define_maps! { <'tcx>
         -> Arc<Vec<(String, DefId, SymbolExportLevel)>>,
     [] fn collect_and_partition_translation_items:
         collect_and_partition_translation_items_node(CrateNum)
-        -> (Arc<FxHashSet<TransItem<'tcx>>>, Vec<Arc<CodegenUnit<'tcx>>>),
+        -> (Arc<DefIdSet>, Arc<Vec<Arc<CodegenUnit<'tcx>>>>),
     [] fn export_name: ExportName(DefId) -> Option<Symbol>,
     [] fn contains_extern_indicator: ContainsExternIndicator(DefId) -> bool,
+    [] fn is_translated_function: IsTranslatedFunction(DefId) -> bool,
 }
 
 fn type_param_predicates<'tcx>((item_id, param_id): (DefId, DefId)) -> DepConstructor<'tcx> {

src/librustc_trans/base.rs

@@ -35,7 +35,7 @@ use back::write::{self, OngoingCrateTranslation};
 use llvm::{ContextRef, ModuleRef, ValueRef, Vector, get_param};
 use llvm;
 use metadata;
-use rustc::hir::def_id::{CrateNum, LOCAL_CRATE};
+use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
 use rustc::middle::lang_items::StartFnLangItem;
 use rustc::middle::trans::{Linkage, Visibility};
 use rustc::middle::cstore::{EncodedMetadata, EncodedMetadataHashes};
@@ -75,7 +75,7 @@ use trans_item::{TransItem, TransItemExt, DefPathBasedNames};
 use type_::Type;
 use type_of;
 use value::Value;
-use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet};
+use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet, DefIdSet};
 use CrateInfo;
 
 use libc::c_uint;
@@ -990,8 +990,9 @@ 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) =
-        shared_ccx.tcx().collect_and_partition_translation_items(LOCAL_CRATE);
+    let codegen_units =
+        shared_ccx.tcx().collect_and_partition_translation_items(LOCAL_CRATE).1;
+    let codegen_units = (*codegen_units).clone();
 
     assert!(codegen_units.len() <= 1 || !tcx.sess.lto());
 
@@ -1076,8 +1077,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             let ((stats, module), _) =
                 tcx.dep_graph.with_task(dep_node,
                                         AssertDepGraphSafe(&shared_ccx),
-                                        AssertDepGraphSafe((cgu,
-                                                            translation_items.clone())),
+                                        AssertDepGraphSafe(cgu),
                                         module_translation);
             all_stats.extend(stats);
 
@@ -1118,13 +1118,12 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     fn module_translation<'a, 'tcx>(
         scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>,
-        args: AssertDepGraphSafe<(Arc<CodegenUnit<'tcx>>,
-                                  Arc<FxHashSet<TransItem<'tcx>>>)>)
+        args: AssertDepGraphSafe<Arc<CodegenUnit<'tcx>>>)
         -> (Stats, ModuleTranslation)
     {
         // FIXME(#40304): We ought to be using the id as a key and some queries, I think.
         let AssertDepGraphSafe(scx) = scx;
-        let AssertDepGraphSafe((cgu, crate_trans_items)) = args;
+        let AssertDepGraphSafe(cgu) = args;
 
         let cgu_name = cgu.name().to_string();
         let cgu_id = cgu.work_product_id();
@@ -1164,7 +1163,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         }
 
         // Instantiate translation items without filling out definitions yet...
-        let lcx = LocalCrateContext::new(scx, cgu, crate_trans_items);
+        let lcx = LocalCrateContext::new(scx, cgu);
         let module = {
             let ccx = CrateContext::new(scx, &lcx);
             let trans_items = ccx.codegen_unit()
@@ -1353,7 +1352,7 @@ fn assert_symbols_are_distinct<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_i
 fn collect_and_partition_translation_items<'a, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     cnum: CrateNum,
-) -> (Arc<FxHashSet<TransItem<'tcx>>>, Vec<Arc<CodegenUnit<'tcx>>>)
+) -> (Arc<DefIdSet>, Arc<Vec<Arc<CodegenUnit<'tcx>>>>)
 {
     assert_eq!(cnum, LOCAL_CRATE);
     let time_passes = tcx.sess.time_passes();
@@ -1404,7 +1403,12 @@ fn collect_and_partition_translation_items<'a, 'tcx>(
     assert!(tcx.sess.opts.cg.codegen_units == codegen_units.len() ||
             tcx.sess.opts.debugging_opts.incremental.is_some());
 
-    let translation_items: FxHashSet<TransItem<'tcx>> = items.iter().cloned().collect();
+    let translation_items: DefIdSet = items.iter().filter_map(|trans_item| {
+        match *trans_item {
+            TransItem::Fn(ref instance) => Some(instance.def_id()),
+            _ => None,
+        }
+    }).collect();
 
     if tcx.sess.opts.debugging_opts.print_trans_items.is_some() {
         let mut item_to_cgus = FxHashMap();
@@ -1459,7 +1463,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(
         }
     }
 
-    (Arc::new(translation_items), codegen_units)
+    (Arc::new(translation_items), Arc::new(codegen_units))
 }
 
 impl CrateInfo {
@@ -1505,9 +1509,25 @@ impl CrateInfo {
     }
 }
 
-pub fn provide(providers: &mut Providers) {
+fn is_translated_function(tcx: TyCtxt, id: DefId) -> bool {
+    // FIXME(#42293) needs red/green tracking to avoid failing a bunch of
+    // existing tests
+    tcx.dep_graph.with_ignore(|| {
+        let (all_trans_items, _) =
+            tcx.collect_and_partition_translation_items(LOCAL_CRATE);
+        all_trans_items.contains(&id)
+    })
+}
+
+pub fn provide_local(providers: &mut Providers) {
     providers.collect_and_partition_translation_items =
         collect_and_partition_translation_items;
+
+    providers.is_translated_function = is_translated_function;
+}
+
+pub fn provide_extern(providers: &mut Providers) {
+    providers.is_translated_function = is_translated_function;
 }
 
 pub fn linkage_to_llvm(linkage: Linkage) -> llvm::Linkage {

src/librustc_trans/callee.rs

@@ -23,7 +23,6 @@ use monomorphize::{self, Instance};
 use rustc::hir::def_id::DefId;
 use rustc::ty::TypeFoldable;
 use rustc::ty::subst::Substs;
-use trans_item::TransItem;
 use type_of;
 
 /// Translates a reference to a fn/method item, monomorphizing and
@@ -109,10 +108,43 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
             attributes::unwind(llfn, true);
         }
 
+        // Apply an appropriate linkage/visibility value to our item that we
+        // just declared.
+        //
+        // This is sort of subtle. Inside our codegen unit we started off
+        // compilation by predefining all our own `TransItem` instances. That
+        // is, everything we're translating ourselves is already defined. That
+        // means that anything we're actually translating ourselves will have
+        // hit the above branch in `get_declared_value`. As a result, we're
+        // guaranteed here that we're declaring a symbol that won't get defined,
+        // or in other words we're referencing a foreign value.
+        //
+        // So because this is a foreign value we blanket apply an external
+        // linkage directive because it's coming from a different object file.
+        // The visibility here is where it gets tricky. This symbol could be
+        // referencing some foreign crate or foreign library (an `extern`
+        // block) in which case we want to leave the default visibility. We may
+        // also, though, have multiple codegen units.
+        //
+        // In the situation of multiple codegen units this function may be
+        // referencing a function from another codegen unit. If we're
+        // indeed referencing a symbol in another codegen unit then we're in one
+        // of two cases:
+        //
+        //  * This is a symbol defined in a foreign crate and we're just
+        //    monomorphizing in another codegen unit. In this case this symbols
+        //    is for sure not exported, so both codegen units will be using
+        //    hidden visibility. Hence, we apply a hidden visibility here.
+        //
+        //  * This is a symbol defined in our local crate. If the symbol in the
+        //    other codegen unit is also not exported then like with the foreign
+        //    case we apply a hidden visibility. If the symbol is exported from
+        //    the foreign object file, however, then we leave this at the
+        //    default visibility as we'll just import it naturally.
         unsafe {
             llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage);
 
-            if ccx.crate_trans_items().contains(&TransItem::Fn(instance)) {
+            if ccx.tcx().is_translated_function(instance_def_id) {
                 if instance_def_id.is_local() {
                     if !ccx.tcx().is_exported_symbol(instance_def_id) {
                         llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden);

src/librustc_trans/context.rs

@@ -22,14 +22,13 @@ use declare;
 use monomorphize::Instance;
 
 use partitioning::CodegenUnit;
-use trans_item::TransItem;
 use type_::Type;
 use rustc_data_structures::base_n;
 use rustc::session::config::{self, NoDebugInfo, OutputFilenames};
 use rustc::session::Session;
 use rustc::ty::{self, Ty, TyCtxt};
 use rustc::ty::layout::{LayoutCx, LayoutError, LayoutTyper, TyLayout};
-use rustc::util::nodemap::{FxHashMap, FxHashSet};
+use rustc::util::nodemap::FxHashMap;
 
 use std::ffi::{CStr, CString};
 use std::cell::{Cell, RefCell};
@@ -94,9 +93,6 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> {
     stats: Stats,
     codegen_unit: Arc<CodegenUnit<'tcx>>,
 
-    /// The translation items of the whole crate.
-    crate_trans_items: Arc<FxHashSet<TransItem<'tcx>>>,
-
     /// Cache instances of monomorphic and polymorphic items
     instances: RefCell<FxHashMap<Instance<'tcx>, ValueRef>>,
     /// Cache generated vtables
@@ -349,8 +345,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> {
 
 impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> {
     pub fn new(shared: &SharedCrateContext<'a, 'tcx>,
-               codegen_unit: Arc<CodegenUnit<'tcx>>,
-               crate_trans_items: Arc<FxHashSet<TransItem<'tcx>>>)
+               codegen_unit: Arc<CodegenUnit<'tcx>>)
                -> LocalCrateContext<'a, 'tcx> {
         unsafe {
             // Append ".rs" to LLVM module identifier.
@@ -382,7 +377,6 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> {
                 llcx,
                 stats: Stats::default(),
                 codegen_unit,
-                crate_trans_items,
                 instances: RefCell::new(FxHashMap()),
                 vtables: RefCell::new(FxHashMap()),
                 const_cstr_cache: RefCell::new(FxHashMap()),
@@ -489,10 +483,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         &self.local().codegen_unit
     }
 
-    pub fn crate_trans_items(&self) -> &FxHashSet<TransItem<'tcx>> {
-        &self.local().crate_trans_items
-    }
-
     pub fn td(&self) -> llvm::TargetDataRef {
         unsafe { llvm::LLVMRustGetModuleDataLayout(self.llmod()) }
     }

src/librustc_trans/lib.rs

@@ -251,10 +251,11 @@ __build_diagnostic_array! { librustc_trans, DIAGNOSTICS }
 pub fn provide_local(providers: &mut Providers) {
     back::symbol_names::provide(providers);
     back::symbol_export::provide_local(providers);
-    base::provide(providers);
+    base::provide_local(providers);
 }
 
 pub fn provide_extern(providers: &mut Providers) {
     back::symbol_names::provide(providers);
     back::symbol_export::provide_extern(providers);
+    base::provide_extern(providers);
 }