From ceeb158e0a04c3a73a4c5014609020ee628b4c06 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 22 Jul 2016 10:39:30 -0400 Subject: [PATCH] Address mw nits --- src/librustc/dep_graph/graph.rs | 10 ++++---- src/librustc/util/fs.rs | 14 +++++++++++ src/librustc_driver/driver.rs | 24 +++++++++---------- src/librustc_incremental/persist/load.rs | 11 +++++---- .../persist/work_product.rs | 6 ++--- src/librustc_trans/back/write.rs | 12 +--------- src/librustc_trans/base.rs | 13 ++++++++-- src/librustc_trans/partitioning.rs | 3 +-- src/librustc_trans/trans_item.rs | 8 ++----- src/test/incremental/spike-neg1.rs | 5 ---- src/test/incremental/spike-neg2.rs | 5 ---- 11 files changed, 54 insertions(+), 57 deletions(-) diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index ab7013df33f..8691894b325 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -25,17 +25,17 @@ pub struct DepGraph { } struct DepGraphData { - /// we send messages to the thread to let it build up the dep-graph - /// from the current run + /// We send messages to the thread to let it build up the dep-graph + /// from the current run. thread: DepGraphThreadData, - /// when we load, there may be `.o` files, cached mir, or other such + /// When we load, there may be `.o` files, cached mir, or other such /// things available to us. If we find that they are not dirty, we /// load the path to the file storing those work-products here into /// this map. We can later look for and extract that data. previous_work_products: RefCell, WorkProduct>>, - /// work-products that we generate in this run + /// Work-products that we generate in this run. work_products: RefCell, WorkProduct>>, } @@ -132,7 +132,7 @@ pub fn work_products(&self) -> Ref, WorkProduct>> /// Each work product is associated with a dep-node, representing the /// process that produced the work-product. If that dep-node is found /// to be dirty when we load up, then we will delete the work-product -/// at load time. If the work-product is found to be clean, the we +/// at load time. If the work-product is found to be clean, then we /// will keep a record in the `previous_work_products` list. /// /// In addition, work products have an associated hash. This hash is diff --git a/src/librustc/util/fs.rs b/src/librustc/util/fs.rs index 4936e049ef2..f4e1c06090e 100644 --- a/src/librustc/util/fs.rs +++ b/src/librustc/util/fs.rs @@ -10,6 +10,8 @@ use std::path::{self, Path, PathBuf}; use std::ffi::OsString; +use std::fs; +use std::io; // Unfortunately, on windows, it looks like msvcrt.dll is silently translating // verbatim paths under the hood to non-verbatim paths! This manifests itself as @@ -53,3 +55,15 @@ pub fn fix_windows_verbatim_for_gcc(p: &Path) -> PathBuf { _ => p.to_path_buf(), } } + +/// Copy `p` into `q`, preferring to use hard-linking if possible. If +/// `q` already exists, it is removed first. +pub fn link_or_copy, Q: AsRef>(p: P, q: Q) -> io::Result<()> { + let p = p.as_ref(); + let q = q.as_ref(); + if q.exists() { + try!(fs::remove_file(&q)); + } + fs::hard_link(p, q) + .or_else(|_| fs::copy(p, q).map(|_| ())) +} diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index a48ff253348..f172f38b809 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -88,7 +88,7 @@ macro_rules! controller_entry_point { // We need nested scopes here, because the intermediate results can keep // large chunks of memory alive and we want to free them as soon as // possible to keep the peak memory usage low - let (outputs, trans, id) = { + let (outputs, trans, crate_name) = { let krate = match phase_1_parse_input(sess, cfg, input) { Ok(krate) => krate, Err(mut parse_error) => { @@ -113,13 +113,13 @@ macro_rules! controller_entry_point { }; let outputs = build_output_filenames(input, outdir, output, &krate.attrs, sess); - let id = link::find_crate_name(Some(sess), &krate.attrs, input); + let crate_name = link::find_crate_name(Some(sess), &krate.attrs, input); let ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = { phase_2_configure_and_expand( - sess, &cstore, krate, &id, addl_plugins, control.make_glob_map, + sess, &cstore, krate, &crate_name, addl_plugins, control.make_glob_map, |expanded_crate| { let mut state = CompileState::state_after_expand( - input, sess, outdir, output, &cstore, expanded_crate, &id, + input, sess, outdir, output, &cstore, expanded_crate, &crate_name, ); controller_entry_point!(after_expand, sess, state, Ok(())); Ok(()) @@ -127,7 +127,7 @@ macro_rules! controller_entry_point { )? }; - write_out_deps(sess, &outputs, &id); + write_out_deps(sess, &outputs, &crate_name); let arenas = ty::CtxtArenas::new(); @@ -151,7 +151,7 @@ macro_rules! controller_entry_point { &resolutions, &expanded_crate, &hir_map.krate(), - &id), + &crate_name), Ok(())); } @@ -171,7 +171,7 @@ macro_rules! controller_entry_point { analysis, resolutions, &arenas, - &id, + &crate_name, |tcx, mir_map, analysis, result| { { // Eventually, we will want to track plugins. @@ -186,7 +186,7 @@ macro_rules! controller_entry_point { &analysis, mir_map.as_ref(), tcx, - &id); + &crate_name); (control.after_analysis.callback)(&mut state); if control.after_analysis.stop == Compilation::Stop { @@ -212,11 +212,11 @@ macro_rules! controller_entry_point { // Discard interned strings as they are no longer required. token::clear_ident_interner(); - Ok((outputs, trans, id.clone())) + Ok((outputs, trans, crate_name.clone())) })?? }; - let phase5_result = phase_5_run_llvm_passes(sess, &id, &trans, &outputs); + let phase5_result = phase_5_run_llvm_passes(sess, &crate_name, &trans, &outputs); controller_entry_point!(after_llvm, sess, @@ -1069,14 +1069,14 @@ fn escape_dep_filename(filename: &str) -> String { filename.replace(" ", "\\ ") } -fn write_out_deps(sess: &Session, outputs: &OutputFilenames, id: &str) { +fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) { let mut out_filenames = Vec::new(); for output_type in sess.opts.output_types.keys() { let file = outputs.path(*output_type); match *output_type { OutputType::Exe => { for output in sess.crate_types.borrow().iter() { - let p = link::filename_for_input(sess, *output, id, outputs); + let p = link::filename_for_input(sess, *output, crate_name, outputs); out_filenames.push(p); } } diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index 9fef2285aa7..6b856459aab 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/src/librustc_incremental/persist/load.rs @@ -63,10 +63,13 @@ fn load_dep_graph_if_exists<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { match decode_dep_graph(tcx, &dep_graph_data, &work_products_data) { Ok(()) => return, - Err(err) => bug!("decoding error in dep-graph from `{}` and `{}`: {}", + Err(err) => { + tcx.sess.warn( + &format!("decoding error in dep-graph from `{}` and `{}`: {}", dep_graph_path.display(), work_products_path.display(), - err), + err)); + } } } @@ -94,9 +97,7 @@ fn load_data(sess: &Session, path: &Path) -> Option> { } /// Decode the dep graph and load the edges/nodes that are still clean -/// into `tcx.dep_graph`. On success, returns a hashset containing all -/// the paths of work-products from clean nodes (any work-products not -/// in this set can be deleted). +/// into `tcx.dep_graph`. pub fn decode_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, dep_graph_data: &[u8], work_products_data: &[u8]) diff --git a/src/librustc_incremental/persist/work_product.rs b/src/librustc_incremental/persist/work_product.rs index 01ac3f6c391..7695291bf62 100644 --- a/src/librustc_incremental/persist/work_product.rs +++ b/src/librustc_incremental/persist/work_product.rs @@ -13,6 +13,7 @@ use persist::util::*; use rustc::dep_graph::{WorkProduct, WorkProductId}; use rustc::session::Session; +use rustc::util::fs::link_or_copy; use std::fs; use std::path::Path; use std::sync::Arc; @@ -39,10 +40,7 @@ pub fn save_trans_partition(sess: &Session, let _ = fs::remove_file(&path_in_incr_dir); } - match - fs::hard_link(path_to_obj_file, &path_in_incr_dir) - .or_else(|_| fs::copy(path_to_obj_file, &path_in_incr_dir).map(|_| ())) - { + match link_or_copy(path_to_obj_file, &path_in_incr_dir) { Ok(_) => { let work_product = WorkProduct { input_hash: partition_hash, diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index 70925218781..08d7b531c2f 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -20,6 +20,7 @@ use {CrateTranslation, ModuleLlvm, ModuleSource, ModuleTranslation}; use util::common::time; use util::common::path2cstr; +use util::fs::link_or_copy; use errors::{self, Handler, Level, DiagnosticBuilder}; use errors::emitter::Emitter; use syntax_pos::MultiSpan; @@ -27,7 +28,6 @@ use std::collections::HashMap; use std::ffi::{CStr, CString}; use std::fs; -use std::io; use std::path::{Path, PathBuf}; use std::str; use std::sync::{Arc, Mutex}; @@ -929,16 +929,6 @@ fn build_work_item(sess: &Session, } } -fn link_or_copy, Q: AsRef>(p: P, q: Q) -> io::Result<()> { - let p = p.as_ref(); - let q = q.as_ref(); - if q.exists() { - try!(fs::remove_file(&q)); - } - fs::hard_link(p, q) - .or_else(|_| fs::copy(p, q).map(|_| ())) -} - fn execute_work_item(cgcx: &CodegenContext, work_item: WorkItem) { unsafe { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 69a88443135..e73c4f41c94 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2262,12 +2262,20 @@ fn write_metadata(cx: &SharedCrateContext, /// Find any symbols that are defined in one compilation unit, but not declared /// in any other compilation unit. Give these symbols internal linkage. -fn internalize_symbols<'a, 'tcx>(ccxs: &CrateContextList<'a, 'tcx>, +fn internalize_symbols<'a, 'tcx>(sess: &Session, + ccxs: &CrateContextList<'a, 'tcx>, symbol_map: &SymbolMap<'tcx>, reachable: &FnvHashSet<&str>) { let scx = ccxs.shared(); let tcx = scx.tcx(); + // In incr. comp. mode, we can't necessarily see all refs since we + // don't generate LLVM IR for reused modules, so skip this + // step. Later we should get smarter. + if sess.opts.debugging_opts.incremental.is_some() { + return; + } + // 'unsafe' because we are holding on to CStr's from the LLVM module within // this block. unsafe { @@ -2682,7 +2690,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } time(shared_ccx.sess().time_passes(), "internalize symbols", || { - internalize_symbols(&crate_context_list, + internalize_symbols(sess, + &crate_context_list, &symbol_map, &reachable_symbols.iter() .map(|s| &s[..]) diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 12df8bd8370..32bcbf9f756 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -362,8 +362,7 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, if codegen_units.is_empty() { let codegen_unit_name = InternedString::new(FALLBACK_CODEGEN_UNIT); codegen_units.entry(codegen_unit_name.clone()) - .or_insert_with(|| CodegenUnit::new(codegen_unit_name.clone(), - FnvHashMap())); + .or_insert_with(|| CodegenUnit::empty(codegen_unit_name.clone())); } PreInliningPartitioning { diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 3afedb49090..fc2758e50f2 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -104,12 +104,8 @@ pub fn define(&self, ccx: &CrateContext<'a, 'tcx>) { } } TransItem::Fn(instance) => { - let _task; - - if instance.def.is_local() { - _task = ccx.tcx().dep_graph.in_task( - DepNode::TransCrateItem(instance.def)); // (*) - } + let _task = ccx.tcx().dep_graph.in_task( + DepNode::TransCrateItem(instance.def)); // (*) base::trans_instance(&ccx, instance); } diff --git a/src/test/incremental/spike-neg1.rs b/src/test/incremental/spike-neg1.rs index e84906d12d0..b00c68a184e 100644 --- a/src/test/incremental/spike-neg1.rs +++ b/src/test/incremental/spike-neg1.rs @@ -39,14 +39,10 @@ fn make() -> X { X { x: 11, y: 11 } } - #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] - #[rustc_clean(label="ItemSignature", cfg="rpass2")] pub fn new() -> X { make() } - #[rustc_clean(label="TypeckItemBody", cfg="rpass2")] - #[rustc_clean(label="ItemSignature", cfg="rpass2")] pub fn sum(x: &X) -> u32 { x.x + x.y } @@ -55,7 +51,6 @@ pub fn sum(x: &X) -> u32 { mod y { use x; - #[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn assert_sum() -> bool { let x = x::new(); x::sum(&x) == 22 diff --git a/src/test/incremental/spike-neg2.rs b/src/test/incremental/spike-neg2.rs index 40f4b4f0c44..472d11d7f90 100644 --- a/src/test/incremental/spike-neg2.rs +++ b/src/test/incremental/spike-neg2.rs @@ -39,14 +39,10 @@ fn make() -> X { X { x: 11, y: 11 } } - #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] - #[rustc_clean(label="ItemSignature", cfg="rpass2")] pub fn new() -> X { make() } - #[rustc_clean(label="TypeckItemBody", cfg="rpass2")] - #[rustc_clean(label="ItemSignature", cfg="rpass2")] pub fn sum(x: &X) -> u32 { x.x + x.y } @@ -55,7 +51,6 @@ pub fn sum(x: &X) -> u32 { mod y { use x; - #[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn assert_sum() -> bool { let x = x::new(); x::sum(&x) == 22 -- 2.44.0