From 03b0d995564b26a2ca9ccf0ded8a6fd375dd9412 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 13 Apr 2017 21:27:35 +0300 Subject: [PATCH 01/17] rustc_typeck: consolidate adjustment composition Fixes #41213. --- src/librustc/ty/adjustment.rs | 2 +- src/librustc_typeck/check/callee.rs | 2 +- src/librustc_typeck/check/coercion.rs | 38 +++++--------- src/librustc_typeck/check/method/confirm.rs | 8 +-- src/librustc_typeck/check/method/mod.rs | 2 +- src/librustc_typeck/check/mod.rs | 55 +++++++++++++++------ src/test/run-pass/issue-41213.rs | 32 ++++++++++++ 7 files changed, 92 insertions(+), 47 deletions(-) create mode 100644 src/test/run-pass/issue-41213.rs diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index d8ca30477205c..1d7100a7a4e44 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -33,7 +33,7 @@ pub enum Adjust<'tcx> { /// Go from a safe fn pointer to an unsafe fn pointer. UnsafeFnPointer, - // Go from a non-capturing closure to an fn pointer. + /// Go from a non-capturing closure to an fn pointer. ClosureFnPointer, /// Go from a mut raw pointer to a const raw pointer. diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index f9bc947a97358..9c5870c12aad4 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -100,7 +100,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // If the callee is a bare function or a closure, then we're all set. match self.structurally_resolved_type(callee_expr.span, adjusted_ty).sty { ty::TyFnDef(..) | ty::TyFnPtr(_) => { - self.write_autoderef_adjustment(callee_expr.id, autoderefs, adjusted_ty); + self.apply_autoderef_adjustment(callee_expr.id, autoderefs, adjusted_ty); return Some(CallStep::Builtin); } diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index a5acd0c7e5300..de7a579ba10ed 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -712,13 +712,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.commit_if_ok(|_| { let ok = coerce.coerce(&[expr], source, target)?; let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - debug!("Success, coerced with {:?}", adjustment); - if self.tables.borrow().adjustments.get(&expr.id).is_some() { - bug!("expr already has an adjustment on it!"); - } - self.write_adjustment(expr.id, adjustment); - } + self.apply_adjustment(expr.id, adjustment); // We should now have added sufficient adjustments etc to // ensure that the type of expression, post-adjustment, is @@ -780,9 +774,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Reify both sides and return the reified fn pointer type. let fn_ptr = self.tcx.mk_fn_ptr(fty); for expr in exprs.iter().map(|e| e.as_coercion_site()).chain(Some(new)) { - // No adjustments can produce a fn item, so this should never trip. - assert!(!self.tables.borrow().adjustments.contains_key(&expr.id)); - self.write_adjustment(expr.id, Adjustment { + // The only adjustment that can produce an fn item is + // `NeverToAny`, so this should always be valid. + self.apply_adjustment(expr.id, Adjustment { kind: Adjust::ReifyFnPointer, target: fn_ptr }); @@ -803,9 +797,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match result { Ok(ok) => { let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - self.write_adjustment(new.id, adjustment); - } + self.apply_adjustment(new.id, adjustment); return Ok(adjustment.target); } Err(e) => first_error = Some(e), @@ -825,7 +817,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }) => { match self.node_ty(expr.id).sty { ty::TyRef(_, mt_orig) => { - // Reborrow that we can safely ignore. + // Reborrow that we can safely ignore, because + // the next adjustment can only be a DerefRef + // which will be merged into it. mutbl_adj == mt_orig.mutbl } _ => false, @@ -858,19 +852,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } Ok(ok) => { let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - let mut tables = self.tables.borrow_mut(); - for expr in exprs { - let expr = expr.as_coercion_site(); - if let Some(&mut Adjustment { - kind: Adjust::NeverToAny, - ref mut target - }) = tables.adjustments.get_mut(&expr.id) { - *target = adjustment.target; - continue; - } - tables.adjustments.insert(expr.id, adjustment); - } + for expr in exprs { + let expr = expr.as_coercion_site(); + self.apply_adjustment(expr.id, adjustment); } Ok(adjustment.target) } diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index 73f6cd76290aa..28ac335cf195a 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -143,7 +143,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { let target = target.adjust_for_autoref(self.tcx, autoref); // Write out the final adjustment. - self.write_adjustment(self.self_expr.id, Adjustment { + self.apply_adjustment(self.self_expr.id, Adjustment { kind: Adjust::DerefRef { autoderefs: pick.autoderefs, autoref: autoref, @@ -433,7 +433,8 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { for (i, &expr) in exprs.iter().rev().enumerate() { debug!("convert_lvalue_derefs_to_mutable: i={} expr={:?}", i, expr); - // Count autoderefs. + // Count autoderefs. We don't need to fix up the autoref - the parent + // expression will fix them up for us. let adjustment = self.tables.borrow().adjustments.get(&expr.id).cloned(); match adjustment { Some(Adjustment { kind: Adjust::DerefRef { autoderefs, .. }, .. }) => { @@ -464,7 +465,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { // expects. This is annoying and horrible. We // ought to recode this routine so it doesn't // (ab)use the normal type checking paths. - let adj = self.tables.borrow().adjustments.get(&base_expr.id).cloned(); + let adj = self.tables.borrow_mut().adjustments.remove(&base_expr.id); let (autoderefs, unsize, adjusted_base_ty) = match adj { Some(Adjustment { kind: Adjust::DerefRef { autoderefs, autoref, unsize }, @@ -537,6 +538,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { // a preference for mut let method_call = ty::MethodCall::expr(expr.id); if self.tables.borrow().method_map.contains_key(&method_call) { + self.tables.borrow_mut().adjustments.remove(&base_expr.id); let method = self.try_overloaded_deref(expr.span, Some(&base_expr), self.node_ty(base_expr.id), diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 4085a171bbef5..9ecf0ffa71ebd 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -299,7 +299,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } }; - self.write_adjustment(self_expr.id, Adjustment { + self.apply_adjustment(self_expr.id, Adjustment { kind: Adjust::DerefRef { autoderefs: autoderefs, autoref: autoref, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c995b7e92843d..dc221a93f1342 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -95,7 +95,7 @@ use rustc::ty::{ParamTy, ParameterEnvironment}; use rustc::ty::{LvaluePreference, NoPreference, PreferMutLvalue}; use rustc::ty::{self, Ty, TyCtxt, Visibility}; use rustc::ty::{MethodCall, MethodCallee}; -use rustc::ty::adjustment; +use rustc::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; use rustc::ty::fold::{BottomUpFolder, TypeFoldable}; use rustc::ty::maps::Providers; use rustc::ty::util::{Representability, IntTypeExt}; @@ -108,6 +108,7 @@ use util::common::{ErrorReported, indenter}; use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap}; use std::cell::{Cell, RefCell}; +use std::collections::hash_map::Entry; use std::cmp; use std::mem::replace; use std::ops::{self, Deref}; @@ -1637,12 +1638,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - pub fn write_autoderef_adjustment(&self, + pub fn apply_autoderef_adjustment(&self, node_id: ast::NodeId, derefs: usize, adjusted_ty: Ty<'tcx>) { - self.write_adjustment(node_id, adjustment::Adjustment { - kind: adjustment::Adjust::DerefRef { + self.apply_adjustment(node_id, Adjustment { + kind: Adjust::DerefRef { autoderefs: derefs, autoref: None, unsize: false @@ -1651,16 +1652,42 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }); } - pub fn write_adjustment(&self, - node_id: ast::NodeId, - adj: adjustment::Adjustment<'tcx>) { - debug!("write_adjustment(node_id={}, adj={:?})", node_id, adj); + pub fn apply_adjustment(&self, node_id: ast::NodeId, adj: Adjustment<'tcx>) { + debug!("apply_adjustment(node_id={}, adj={:?})", node_id, adj); if adj.is_identity() { return; } - self.tables.borrow_mut().adjustments.insert(node_id, adj); + match self.tables.borrow_mut().adjustments.entry(node_id) { + Entry::Vacant(entry) => { entry.insert(adj); }, + Entry::Occupied(mut entry) => { + debug!(" - composing on top of {:?}", entry.get()); + let composed_kind = match (entry.get().kind, adj.kind) { + // Applying any adjustment on top of a NeverToAny + // is a valid NeverToAny adjustment, because it can't + // be reached. + (Adjust::NeverToAny, _) => Adjust::NeverToAny, + (Adjust::DerefRef { + autoderefs: 1, + autoref: Some(AutoBorrow::Ref(..)), + unsize: false + }, Adjust::DerefRef { autoderefs, .. }) if autoderefs > 0 => { + // A reborrow has no effect before a dereference. + adj.kind + } + // FIXME: currently we never try to compose autoderefs + // and ReifyFnPointer/UnsafeFnPointer, but we could. + _ => + bug!("while adjusting {}, can't compose {:?} and {:?}", + node_id, entry.get(), adj) + }; + *entry.get_mut() = Adjustment { + kind: composed_kind, + target: adj.target + }; + } + } } /// Basically whenever we are converting from a type scheme into @@ -2302,7 +2329,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { debug!("try_index_step: success, using built-in indexing"); // If we had `[T; N]`, we should've caught it before unsizing to `[T]`. assert!(!unsize); - self.write_autoderef_adjustment(base_expr.id, autoderefs, adjusted_ty); + self.apply_autoderef_adjustment(base_expr.id, autoderefs, adjusted_ty); return Some((tcx.types.usize, ty)); } _ => {} @@ -2685,8 +2712,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { "expression with never type wound up being adjusted"); let adj_ty = self.next_diverging_ty_var( TypeVariableOrigin::AdjustmentType(expr.span)); - self.write_adjustment(expr.id, adjustment::Adjustment { - kind: adjustment::Adjust::NeverToAny, + self.apply_adjustment(expr.id, Adjustment { + kind: Adjust::NeverToAny, target: adj_ty }); ty = adj_ty; @@ -2917,7 +2944,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let field_ty = self.field_ty(expr.span, field, substs); if self.tcx.vis_is_accessible_from(field.vis, self.body_id) { autoderef.finalize(lvalue_pref, &[base]); - self.write_autoderef_adjustment(base.id, autoderefs, base_t); + self.apply_autoderef_adjustment(base.id, autoderefs, base_t); self.tcx.check_stability(field.did, expr.id, expr.span); @@ -3041,7 +3068,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(field_ty) = field { autoderef.finalize(lvalue_pref, &[base]); - self.write_autoderef_adjustment(base.id, autoderefs, base_t); + self.apply_autoderef_adjustment(base.id, autoderefs, base_t); return field_ty; } } diff --git a/src/test/run-pass/issue-41213.rs b/src/test/run-pass/issue-41213.rs new file mode 100644 index 0000000000000..d4755020fef22 --- /dev/null +++ b/src/test/run-pass/issue-41213.rs @@ -0,0 +1,32 @@ +// Copyright 2017 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. + +enum A { + A1, + A2, + A3, +} + +enum B { + B1(String, String), + B2(String, String), +} + +fn main() { + let a = A::A1; + loop { + let _ctor = match a { + A::A3 => break, + A::A1 => B::B1, + A::A2 => B::B2, + }; + break; + } +} From 537eb45b9d8f547046ae060d4c115e408fd66fdf Mon Sep 17 00:00:00 2001 From: steveklabnik Date: Thu, 13 Apr 2017 14:34:32 -0400 Subject: [PATCH 02/17] Update various bookshelf repositories. The book and the reference have both had changes lately; this integrates them upstream. --- src/doc/book | 2 +- src/doc/reference | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/book b/src/doc/book index a2c56870d4dc5..beea82b9230cd 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit a2c56870d4dc589237102cc5e0fe7b9ebd0d14a1 +Subproject commit beea82b9230cd641dd1ca263cf31025ace4aebb5 diff --git a/src/doc/reference b/src/doc/reference index acedc32cacae8..b060f732145f2 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit acedc32cacae80cf2f4925753a4ce7f7ffd7c86a +Subproject commit b060f732145f2fa16df84c74e511df08a3a47c5d From 15507bcb648964752db0d7fd37c4487780a7a5a4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 11:48:19 -0400 Subject: [PATCH 03/17] remove `metadata_*` from `SharedCrateContext` No good reason for them to be in there. --- src/librustc_trans/base.rs | 37 ++++++++++++++++++++--------------- src/librustc_trans/context.rs | 19 +----------------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index f76e816bcf0c9..fd8beab92f5ef 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -32,7 +32,7 @@ use assert_module_sources; use back::link; use back::linker::LinkerInfo; use back::symbol_export::{self, ExportedSymbols}; -use llvm::{Linkage, ValueRef, Vector, get_param}; +use llvm::{ContextRef, Linkage, ModuleRef, ValueRef, Vector, get_param}; use llvm; use rustc::hir::def_id::LOCAL_CRATE; use middle::lang_items::StartFnLangItem; @@ -56,7 +56,7 @@ use common::CrateContext; use common::{type_is_zero_size, val_ty}; use common; use consts; -use context::{SharedCrateContext, CrateContextList}; +use context::{self, SharedCrateContext, CrateContextList}; use debuginfo; use declare; use machine; @@ -726,9 +726,13 @@ fn contains_null(s: &str) -> bool { fn write_metadata(cx: &SharedCrateContext, exported_symbols: &NodeSet) - -> EncodedMetadata { + -> (ContextRef, ModuleRef, EncodedMetadata) { use flate; + let (metadata_llcx, metadata_llmod) = unsafe { + context::create_context_and_module(cx.sess(), "metadata") + }; + #[derive(PartialEq, Eq, PartialOrd, Ord)] enum MetadataKind { None, @@ -750,10 +754,10 @@ fn write_metadata(cx: &SharedCrateContext, }).max().unwrap(); if kind == MetadataKind::None { - return EncodedMetadata { + return (metadata_llcx, metadata_llmod, EncodedMetadata { raw_data: vec![], hashes: vec![], - }; + }); } let cstore = &cx.tcx().sess.cstore; @@ -761,19 +765,19 @@ fn write_metadata(cx: &SharedCrateContext, cx.link_meta(), exported_symbols); if kind == MetadataKind::Uncompressed { - return metadata; + return (metadata_llcx, metadata_llmod, metadata); } assert!(kind == MetadataKind::Compressed); let mut compressed = cstore.metadata_encoding_version().to_vec(); compressed.extend_from_slice(&flate::deflate_bytes(&metadata.raw_data)); - let llmeta = C_bytes_in_context(cx.metadata_llcx(), &compressed); - let llconst = C_struct_in_context(cx.metadata_llcx(), &[llmeta], false); + let llmeta = C_bytes_in_context(metadata_llcx, &compressed); + let llconst = C_struct_in_context(metadata_llcx, &[llmeta], false); let name = cx.metadata_symbol_name(); let buf = CString::new(name).unwrap(); let llglobal = unsafe { - llvm::LLVMAddGlobal(cx.metadata_llmod(), val_ty(llconst).to_ref(), buf.as_ptr()) + llvm::LLVMAddGlobal(metadata_llmod, val_ty(llconst).to_ref(), buf.as_ptr()) }; unsafe { llvm::LLVMSetInitializer(llglobal, llconst); @@ -787,9 +791,9 @@ fn write_metadata(cx: &SharedCrateContext, // metadata doesn't get loaded into memory. let directive = format!(".section {}", section_name); let directive = CString::new(directive).unwrap(); - llvm::LLVMSetModuleInlineAsm(cx.metadata_llmod(), directive.as_ptr()) + llvm::LLVMSetModuleInlineAsm(metadata_llmod, directive.as_ptr()) } - return metadata; + return (metadata_llcx, metadata_llmod, metadata); } /// Find any symbols that are defined in one compilation unit, but not declared @@ -1070,16 +1074,17 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, exported_symbols, check_overflow); // Translate the metadata. - let metadata = time(tcx.sess.time_passes(), "write metadata", || { - write_metadata(&shared_ccx, shared_ccx.exported_symbols()) - }); + let (metadata_llcx, metadata_llmod, metadata) = + time(tcx.sess.time_passes(), "write metadata", || { + write_metadata(&shared_ccx, shared_ccx.exported_symbols()) + }); let metadata_module = ModuleTranslation { name: link::METADATA_MODULE_NAME.to_string(), symbol_name_hash: 0, // we always rebuild metadata, at least for now source: ModuleSource::Translated(ModuleLlvm { - llcx: shared_ccx.metadata_llcx(), - llmod: shared_ccx.metadata_llmod(), + llcx: metadata_llcx, + llmod: metadata_llmod, }), }; let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 98fbb64fd5540..f080cd3eccfcf 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -65,9 +65,6 @@ pub struct Stats { /// crate, so it must not contain references to any LLVM data structures /// (aside from metadata-related ones). pub struct SharedCrateContext<'a, 'tcx: 'a> { - metadata_llmod: ModuleRef, - metadata_llcx: ContextRef, - exported_symbols: NodeSet, link_meta: LinkMeta, tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -347,7 +344,7 @@ pub fn is_pie_binary(sess: &Session) -> bool { !is_any_library(sess) && get_reloc_model(sess) == llvm::RelocMode::PIC } -unsafe fn create_context_and_module(sess: &Session, mod_name: &str) -> (ContextRef, ModuleRef) { +pub unsafe fn create_context_and_module(sess: &Session, mod_name: &str) -> (ContextRef, ModuleRef) { let llcx = llvm::LLVMContextCreate(); let mod_name = CString::new(mod_name).unwrap(); let llmod = llvm::LLVMModuleCreateWithNameInContext(mod_name.as_ptr(), llcx); @@ -409,10 +406,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { exported_symbols: NodeSet, check_overflow: bool) -> SharedCrateContext<'b, 'tcx> { - let (metadata_llcx, metadata_llmod) = unsafe { - create_context_and_module(&tcx.sess, "metadata") - }; - // An interesting part of Windows which MSVC forces our hand on (and // apparently MinGW didn't) is the usage of `dllimport` and `dllexport` // attributes in LLVM IR as well as native dependencies (in C these @@ -459,8 +452,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { let use_dll_storage_attrs = tcx.sess.target.target.options.is_like_msvc; SharedCrateContext { - metadata_llmod: metadata_llmod, - metadata_llcx: metadata_llcx, exported_symbols: exported_symbols, link_meta: link_meta, empty_param_env: tcx.empty_parameter_environment(), @@ -492,14 +483,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { ty.is_sized(self.tcx, &self.empty_param_env, DUMMY_SP) } - pub fn metadata_llmod(&self) -> ModuleRef { - self.metadata_llmod - } - - pub fn metadata_llcx(&self) -> ContextRef { - self.metadata_llcx - } - pub fn exported_symbols<'a>(&'a self) -> &'a NodeSet { &self.exported_symbols } From b078ecefcde4d090ac2d365c5ee0d1314de7a341 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:07:25 -0400 Subject: [PATCH 04/17] rewrite to pass a ref, not slice + index --- src/librustc_trans/context.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index f080cd3eccfcf..99ffd789286b5 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -266,10 +266,7 @@ impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> { /// pass around (SharedCrateContext, LocalCrateContext) tuples all over trans. pub struct CrateContext<'a, 'tcx: 'a> { shared: &'a SharedCrateContext<'a, 'tcx>, - local_ccxs: &'a [LocalCrateContext<'tcx>], - /// The index of `local` in `local_ccxs`. This is used in - /// `maybe_iter(true)` to identify the original `LocalCrateContext`. - index: usize, + local_ccx: &'a LocalCrateContext<'tcx>, } impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> { @@ -298,8 +295,7 @@ impl<'a, 'tcx> Iterator for CrateContextIterator<'a,'tcx> { let ccx = CrateContext { shared: self.shared, - index: index, - local_ccxs: self.local_ccxs, + local_ccx: &self.local_ccxs[index], }; if @@ -630,8 +626,7 @@ impl<'tcx> LocalCrateContext<'tcx> { assert!(local_ccxs.len() == 1); CrateContext { shared: shared, - index: 0, - local_ccxs: local_ccxs + local_ccx: &local_ccxs[0] } } } @@ -642,7 +637,7 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { } fn local(&self) -> &'b LocalCrateContext<'tcx> { - &self.local_ccxs[self.index] + self.local_ccx } pub fn tcx<'a>(&'a self) -> TyCtxt<'a, 'tcx, 'tcx> { From 7b429242a51fad9cef64f6051ced26eb05fd0903 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:07:45 -0400 Subject: [PATCH 05/17] remove unused `link_meta` --- src/librustc_trans/context.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 99ffd789286b5..c54929383f4de 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -682,10 +682,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.shared.exported_symbols } - pub fn link_meta<'a>(&'a self) -> &'a LinkMeta { - &self.shared.link_meta - } - pub fn needs_unwind_cleanup_cache(&self) -> &RefCell, bool>> { &self.local().needs_unwind_cleanup_cache } From 33875055f0f3c1c9c8c697b374f111377f35d8e2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:07:56 -0400 Subject: [PATCH 06/17] redirect `exported_symbols` through `shared` --- src/librustc_trans/context.rs | 4 ---- src/librustc_trans/debuginfo/utils.rs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index c54929383f4de..657dd56accd91 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -678,10 +678,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { unsafe { llvm::LLVMRustGetModuleDataLayout(self.llmod()) } } - pub fn exported_symbols<'a>(&'a self) -> &'a NodeSet { - &self.shared.exported_symbols - } - pub fn needs_unwind_cleanup_cache(&self) -> &RefCell, bool>> { &self.local().needs_unwind_cleanup_cache } diff --git a/src/librustc_trans/debuginfo/utils.rs b/src/librustc_trans/debuginfo/utils.rs index ceff96a39b2c9..0a873767d9359 100644 --- a/src/librustc_trans/debuginfo/utils.rs +++ b/src/librustc_trans/debuginfo/utils.rs @@ -37,7 +37,7 @@ pub fn is_node_local_to_unit(cx: &CrateContext, node_id: ast::NodeId) -> bool // 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. - !cx.exported_symbols().contains(&node_id) + !cx.shared().exported_symbols().contains(&node_id) } #[allow(non_snake_case)] From fe78b546ed6764cae5d793bf75ff756d94007489 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:38:03 -0400 Subject: [PATCH 07/17] merge the "predeclare" and "declare" phases so we run them per-CGU --- src/librustc_trans/base.rs | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index fd8beab92f5ef..0a08f49d818da 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1142,7 +1142,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, assert_module_sources::assert_module_sources(tcx, &modules); - // Instantiate translation items without filling out definitions yet... for ccx in crate_context_list.iter_need_trans() { let dep_node = ccx.codegen_unit().work_product_dep_node(); tcx.dep_graph.with_task(dep_node, @@ -1150,35 +1149,22 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, AssertDepGraphSafe(symbol_map.clone()), trans_decl_task); + fn trans_decl_task<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>, symbol_map: AssertDepGraphSafe>>) { + // Instantiate translation items without filling out definitions yet... + // FIXME(#40304): Instead of this, the symbol-map should be an // on-demand thing that we compute. let AssertDepGraphSafe(symbol_map) = symbol_map; let cgu = ccx.codegen_unit(); let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map); - for (trans_item, linkage) in trans_items { + for &(trans_item, linkage) in &trans_items { trans_item.predefine(&ccx, linkage); } - } - } - // ... and now that we have everything pre-defined, fill out those definitions. - for ccx in crate_context_list.iter_need_trans() { - let dep_node = ccx.codegen_unit().work_product_dep_node(); - tcx.dep_graph.with_task(dep_node, - ccx, - AssertDepGraphSafe(symbol_map.clone()), - trans_def_task); - - fn trans_def_task<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>, - symbol_map: AssertDepGraphSafe>>) { - // FIXME(#40304): Instead of this, the symbol-map should be an - // on-demand thing that we compute. - let AssertDepGraphSafe(symbol_map) = symbol_map; - let cgu = ccx.codegen_unit(); - let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map); - for (trans_item, _) in trans_items { + // ... and now that we have everything pre-defined, fill out those definitions. + for &(trans_item, _) in &trans_items { trans_item.define(&ccx); } From 6cb516ad7b6e058be547e7ee5dc78762cc9b809b Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:38:27 -0400 Subject: [PATCH 08/17] move `assert_module_sources` call down below --- src/librustc_trans/base.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 0a08f49d818da..ce136a7883cc0 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1140,8 +1140,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) .collect(); - assert_module_sources::assert_module_sources(tcx, &modules); - for ccx in crate_context_list.iter_need_trans() { let dep_node = ccx.codegen_unit().work_product_dep_node(); tcx.dep_graph.with_task(dep_node, @@ -1205,6 +1203,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } + assert_module_sources::assert_module_sources(tcx, &modules); + symbol_names_test::report_symbol_names(&shared_ccx); if shared_ccx.sess().trans_stats() { From bc79f01a581c9340cd869ba36579ee80d9606298 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 12:46:37 -0400 Subject: [PATCH 09/17] create `ModuleTranslation` all in one big loop --- src/librustc_trans/base.rs | 59 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index ce136a7883cc0..c822cc5f4b9a7 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1120,41 +1120,31 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, codegen_units, previous_work_products, symbol_map.clone()); - let modules: Vec<_> = crate_context_list.iter_all() - .map(|ccx| { - let source = match ccx.previous_work_product() { - Some(buf) => ModuleSource::Preexisting(buf.clone()), - None => ModuleSource::Translated(ModuleLlvm { - llcx: ccx.llcx(), - llmod: ccx.llmod(), - }), - }; - ModuleTranslation { - name: String::from(ccx.codegen_unit().name()), - symbol_name_hash: ccx.codegen_unit() - .compute_symbol_name_hash(&shared_ccx, - &symbol_map), - source: source, - } + let modules: Vec = crate_context_list + .iter_all() + .map(|ccx| { + let dep_node = ccx.codegen_unit().work_product_dep_node(); + tcx.dep_graph.with_task(dep_node, + ccx, + AssertDepGraphSafe(symbol_map.clone()), + module_translation) }) .collect(); - for ccx in crate_context_list.iter_need_trans() { - let dep_node = ccx.codegen_unit().work_product_dep_node(); - tcx.dep_graph.with_task(dep_node, - ccx, - AssertDepGraphSafe(symbol_map.clone()), - trans_decl_task); - + fn module_translation<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>, + symbol_map: AssertDepGraphSafe>>) + -> ModuleTranslation { + // FIXME(#40304): Instead of this, the symbol-map should be an + // on-demand thing that we compute. + let AssertDepGraphSafe(symbol_map) = symbol_map; - fn trans_decl_task<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>, - symbol_map: AssertDepGraphSafe>>) { + let source = if let Some(buf) = ccx.previous_work_product() { + // Don't need to translate this module. + ModuleSource::Preexisting(buf.clone()) + } else { // Instantiate translation items without filling out definitions yet... - // FIXME(#40304): Instead of this, the symbol-map should be an - // on-demand thing that we compute. - let AssertDepGraphSafe(symbol_map) = symbol_map; let cgu = ccx.codegen_unit(); let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map); for &(trans_item, linkage) in &trans_items { @@ -1200,6 +1190,19 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, if ccx.sess().opts.debuginfo != NoDebugInfo { debuginfo::finalize(&ccx); } + + ModuleSource::Translated(ModuleLlvm { + llcx: ccx.llcx(), + llmod: ccx.llmod(), + }) + }; + + ModuleTranslation { + name: String::from(ccx.codegen_unit().name()), + symbol_name_hash: ccx.codegen_unit() + .compute_symbol_name_hash(ccx.shared(), + &symbol_map), + source: source, } } From 863927c712d703709c0ee9fa709306ec90e5a88d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 13:14:37 -0400 Subject: [PATCH 10/17] rewrite post-processing routines not to require a `CrateContext` These do some low-level munging on the LLVM data structures. Unclear that they need to operate as a "second pass" but leave it for now. --- src/librustc_trans/base.rs | 41 +++++++++++++++++++++++------------ src/librustc_trans/context.rs | 15 ------------- src/librustc_trans/type_.rs | 10 ++++++++- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index c822cc5f4b9a7..a034974cb13ed 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -799,7 +799,8 @@ 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>(sess: &Session, - ccxs: &CrateContextList<'a, 'tcx>, + scx: &SharedCrateContext<'a, 'tcx>, + llvm_modules: &[ModuleLlvm], symbol_map: &SymbolMap<'tcx>, exported_symbols: &ExportedSymbols) { let export_threshold = @@ -814,7 +815,6 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, .map(|&(ref name, _)| &name[..]) .collect::>(); - let scx = ccxs.shared(); let tcx = scx.tcx(); let incr_comp = sess.opts.debugging_opts.incremental.is_some(); @@ -829,8 +829,8 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, // incremental compilation, we don't need to collect. See below for more // information. if !incr_comp { - for ccx in ccxs.iter_need_trans() { - for val in iter_globals(ccx.llmod()).chain(iter_functions(ccx.llmod())) { + for ll in llvm_modules { + for val in iter_globals(ll.llmod).chain(iter_functions(ll.llmod)) { let linkage = llvm::LLVMRustGetLinkage(val); // We only care about external declarations (not definitions) // and available_externally definitions. @@ -866,8 +866,8 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, // Examine each external definition. If the definition is not used in // any other compilation unit, and is not reachable from other crates, // then give it internal linkage. - for ccx in ccxs.iter_need_trans() { - for val in iter_globals(ccx.llmod()).chain(iter_functions(ccx.llmod())) { + for ll in llvm_modules { + for val in iter_globals(ll.llmod).chain(iter_functions(ll.llmod)) { let linkage = llvm::LLVMRustGetLinkage(val); let is_externally_visible = (linkage == llvm::Linkage::ExternalLinkage) || @@ -926,19 +926,20 @@ fn internalize_symbols<'a, 'tcx>(sess: &Session, // when using MSVC linker. We do this only for data, as linker can fix up // code references on its own. // See #26591, #27438 -fn create_imps(cx: &CrateContextList) { +fn create_imps(sess: &Session, + llvm_modules: &[ModuleLlvm]) { // The x86 ABI seems to require that leading underscores are added to symbol // names, so we need an extra underscore on 32-bit. There's also a leading // '\x01' here which disables LLVM's symbol mangling (e.g. no extra // underscores added in front). - let prefix = if cx.shared().sess().target.target.target_pointer_width == "32" { + let prefix = if sess.target.target.target_pointer_width == "32" { "\x01__imp__" } else { "\x01__imp_" }; unsafe { - for ccx in cx.iter_need_trans() { - let exported: Vec<_> = iter_globals(ccx.llmod()) + for ll in llvm_modules { + let exported: Vec<_> = iter_globals(ll.llmod) .filter(|&val| { llvm::LLVMRustGetLinkage(val) == llvm::Linkage::ExternalLinkage && @@ -946,13 +947,13 @@ fn create_imps(cx: &CrateContextList) { }) .collect(); - let i8p_ty = Type::i8p(&ccx); + let i8p_ty = Type::i8p_llcx(ll.llcx); for val in exported { let name = CStr::from_ptr(llvm::LLVMGetValueName(val)); let mut imp_name = prefix.as_bytes().to_vec(); imp_name.extend(name.to_bytes()); let imp_name = CString::new(imp_name).unwrap(); - let imp = llvm::LLVMAddGlobal(ccx.llmod(), + let imp = llvm::LLVMAddGlobal(ll.llmod, i8p_ty.to_ref(), imp_name.as_ptr() as *const _); let init = llvm::LLVMConstBitCast(val, i8p_ty.to_ref()); @@ -1244,11 +1245,23 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let exported_symbols = ExportedSymbols::compute_from(&shared_ccx, &symbol_map); + // Get the list of llvm modules we created. We'll do a few wacky + // transforms on them now. + + let llvm_modules: Vec<_> = + modules.iter() + .filter_map(|module| match module.source { + ModuleSource::Translated(llvm) => Some(llvm), + _ => None, + }) + .collect(); + // Now that we have all symbols that are exported from the CGUs of this // crate, we can run the `internalize_symbols` pass. time(shared_ccx.sess().time_passes(), "internalize symbols", || { internalize_symbols(sess, - &crate_context_list, + &shared_ccx, + &llvm_modules, &symbol_map, &exported_symbols); }); @@ -1259,7 +1272,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, if sess.target.target.options.is_like_msvc && sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateTypeRlib) { - create_imps(&crate_context_list); + create_imps(sess, &llvm_modules); } let linker_info = LinkerInfo::new(&shared_ccx, &exported_symbols); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 657dd56accd91..e3b1e04c5303f 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -244,21 +244,6 @@ impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> { filter_to_previous_work_product_unavail: false, } } - - /// Iterator over all CCX that need translation (cannot reuse results from - /// previous incr. comp.). - pub fn iter_need_trans<'b>(&'b self) -> CrateContextIterator<'b, 'tcx> { - CrateContextIterator { - shared: self.shared, - index: 0, - local_ccxs: &self.local_ccxs[..], - filter_to_previous_work_product_unavail: true, - } - } - - pub fn shared(&self) -> &'a SharedCrateContext<'a, 'tcx> { - self.shared - } } /// A CrateContext value binds together one LocalCrateContext with the diff --git a/src/librustc_trans/type_.rs b/src/librustc_trans/type_.rs index f68acab911317..d70afc0cce5a3 100644 --- a/src/librustc_trans/type_.rs +++ b/src/librustc_trans/type_.rs @@ -11,7 +11,7 @@ #![allow(non_upper_case_globals)] use llvm; -use llvm::{TypeRef, Bool, False, True, TypeKind}; +use llvm::{ContextRef, TypeRef, Bool, False, True, TypeKind}; use llvm::{Float, Double, X86_FP80, PPC_FP128, FP128}; use context::CrateContext; @@ -82,6 +82,10 @@ impl Type { ty!(llvm::LLVMInt8TypeInContext(ccx.llcx())) } + pub fn i8_llcx(llcx: ContextRef) -> Type { + ty!(llvm::LLVMInt8TypeInContext(llcx)) + } + pub fn i16(ccx: &CrateContext) -> Type { ty!(llvm::LLVMInt16TypeInContext(ccx.llcx())) } @@ -123,6 +127,10 @@ impl Type { Type::i8(ccx).ptr_to() } + pub fn i8p_llcx(llcx: ContextRef) -> Type { + Type::i8_llcx(llcx).ptr_to() + } + pub fn int(ccx: &CrateContext) -> Type { match &ccx.tcx().sess.target.target.target_pointer_width[..] { "16" => Type::i16(ccx), From 3f59079f8a36af1d8b5ff5a218f16a8e4806a542 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 13:57:45 -0400 Subject: [PATCH 11/17] kill `CrateContextList` as a thing --- src/librustc_trans/base.rs | 119 ++++++++++++++-------------------- src/librustc_trans/context.rs | 97 ++++----------------------- 2 files changed, 62 insertions(+), 154 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index a034974cb13ed..309161676218b 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -38,7 +38,7 @@ use rustc::hir::def_id::LOCAL_CRATE; use middle::lang_items::StartFnLangItem; use middle::cstore::EncodedMetadata; use rustc::ty::{self, Ty, TyCtxt}; -use rustc::dep_graph::{AssertDepGraphSafe, DepNode, WorkProduct}; +use rustc::dep_graph::{AssertDepGraphSafe, DepNode}; use rustc::hir::map as hir_map; use rustc::util::common::time; use session::config::{self, NoDebugInfo}; @@ -56,7 +56,7 @@ use common::CrateContext; use common::{type_is_zero_size, val_ty}; use common; use consts; -use context::{self, SharedCrateContext, CrateContextList}; +use context::{self, LocalCrateContext, SharedCrateContext}; use debuginfo; use declare; use machine; @@ -1113,41 +1113,61 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let symbol_map = Rc::new(symbol_map); - let previous_work_products = trans_reuse_previous_work_products(&shared_ccx, - &codegen_units, - &symbol_map); - - let crate_context_list = CrateContextList::new(&shared_ccx, - codegen_units, - previous_work_products, - symbol_map.clone()); - - let modules: Vec = crate_context_list - .iter_all() - .map(|ccx| { - let dep_node = ccx.codegen_unit().work_product_dep_node(); + let modules: Vec = codegen_units + .into_iter() + .map(|cgu| { + let dep_node = cgu.work_product_dep_node(); tcx.dep_graph.with_task(dep_node, - ccx, - AssertDepGraphSafe(symbol_map.clone()), + AssertDepGraphSafe(&shared_ccx), + AssertDepGraphSafe((cgu, symbol_map.clone())), module_translation) }) .collect(); - fn module_translation<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>, - symbol_map: AssertDepGraphSafe>>) - -> ModuleTranslation { - // FIXME(#40304): Instead of this, the symbol-map should be an - // on-demand thing that we compute. - let AssertDepGraphSafe(symbol_map) = symbol_map; + fn module_translation<'a, 'tcx>( + scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>, + args: AssertDepGraphSafe<(CodegenUnit<'tcx>, Rc>)>) + -> 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, symbol_map)) = args; + + let cgu_name = String::from(cgu.name()); + let cgu_id = cgu.work_product_id(); + let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &symbol_map); + + // Check whether there is a previous work-product we can + // re-use. Not only must the file exist, and the inputs not + // be dirty, but the hash of the symbols we will generate must + // be the same. + let previous_work_product = + scx.dep_graph().previous_work_product(&cgu_id).and_then(|work_product| { + if work_product.input_hash == symbol_name_hash { + debug!("trans_reuse_previous_work_products: reusing {:?}", work_product); + Some(work_product) + } else { + if scx.sess().opts.debugging_opts.incremental_info { + println!("incremental: CGU `{}` invalidated because of \ + changed partitioning hash.", + cgu.name()); + } + debug!("trans_reuse_previous_work_products: \ + not reusing {:?} because hash changed to {:?}", + work_product, symbol_name_hash); + None + } + }); - let source = if let Some(buf) = ccx.previous_work_product() { + let source = if let Some(buf) = previous_work_product { // Don't need to translate this module. ModuleSource::Preexisting(buf.clone()) } else { // Instantiate translation items without filling out definitions yet... - - let cgu = ccx.codegen_unit(); - let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map); + let lcx = LocalCrateContext::new(scx, cgu, symbol_map.clone()); + let ccx = CrateContext::new(scx, &lcx); + let trans_items = ccx.codegen_unit() + .items_in_deterministic_order(ccx.tcx(), &symbol_map); for &(trans_item, linkage) in &trans_items { trans_item.predefine(&ccx, linkage); } @@ -1199,11 +1219,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; ModuleTranslation { - name: String::from(ccx.codegen_unit().name()), - symbol_name_hash: ccx.codegen_unit() - .compute_symbol_name_hash(ccx.shared(), - &symbol_map), - source: source, + name: cgu_name, + symbol_name_hash, + source, } } @@ -1487,43 +1505,6 @@ fn gather_type_sizes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { } } -/// For each CGU, identify if we can reuse an existing object file (or -/// maybe other context). -fn trans_reuse_previous_work_products(scx: &SharedCrateContext, - codegen_units: &[CodegenUnit], - symbol_map: &SymbolMap) - -> Vec> { - debug!("trans_reuse_previous_work_products()"); - codegen_units - .iter() - .map(|cgu| { - let id = cgu.work_product_id(); - - let hash = cgu.compute_symbol_name_hash(scx, symbol_map); - - debug!("trans_reuse_previous_work_products: id={:?} hash={}", id, hash); - - if let Some(work_product) = scx.dep_graph().previous_work_product(&id) { - if work_product.input_hash == hash { - debug!("trans_reuse_previous_work_products: reusing {:?}", work_product); - return Some(work_product); - } else { - if scx.sess().opts.debugging_opts.incremental_info { - println!("incremental: CGU `{}` invalidated because of \ - changed partitioning hash.", - cgu.name()); - } - debug!("trans_reuse_previous_work_products: \ - not reusing {:?} because hash changed to {:?}", - work_product, hash); - } - } - - None - }) - .collect() -} - fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) -> (Vec>, SymbolMap<'tcx>) { let time_passes = scx.sess().time_passes(); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index e3b1e04c5303f..80d6f15518f97 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -10,8 +10,7 @@ use llvm; use llvm::{ContextRef, ModuleRef, ValueRef}; -use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap, - DepTrackingMapConfig, WorkProduct}; +use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap, DepTrackingMapConfig}; use middle::cstore::LinkMeta; use rustc::hir; use rustc::hir::def_id::DefId; @@ -86,7 +85,6 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { pub struct LocalCrateContext<'tcx> { llmod: ModuleRef, llcx: ContextRef, - previous_work_product: Option, codegen_unit: CodegenUnit<'tcx>, needs_unwind_cleanup_cache: RefCell, bool>>, /// Cache instances of monomorphic and polymorphic items @@ -211,41 +209,6 @@ impl<'gcx> DepTrackingMapConfig for ProjectionCache<'gcx> { } } -/// This list owns a number of LocalCrateContexts and binds them to their common -/// SharedCrateContext. This type just exists as a convenience, something to -/// pass around all LocalCrateContexts with and get an iterator over them. -pub struct CrateContextList<'a, 'tcx: 'a> { - shared: &'a SharedCrateContext<'a, 'tcx>, - local_ccxs: Vec>, -} - -impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> { - pub fn new(shared_ccx: &'a SharedCrateContext<'a, 'tcx>, - codegen_units: Vec>, - previous_work_products: Vec>, - symbol_map: Rc>) - -> CrateContextList<'a, 'tcx> { - CrateContextList { - shared: shared_ccx, - local_ccxs: codegen_units.into_iter().zip(previous_work_products).map(|(cgu, wp)| { - LocalCrateContext::new(shared_ccx, cgu, wp, symbol_map.clone()) - }).collect() - } - } - - /// Iterate over all crate contexts, whether or not they need - /// translation. That is, whether or not a `.o` file is available - /// for re-use from a previous incr. comp.). - pub fn iter_all<'b>(&'b self) -> CrateContextIterator<'b, 'tcx> { - CrateContextIterator { - shared: self.shared, - index: 0, - local_ccxs: &self.local_ccxs[..], - filter_to_previous_work_product_unavail: false, - } - } -} - /// A CrateContext value binds together one LocalCrateContext with the /// SharedCrateContext. It exists as a convenience wrapper, so we don't have to /// pass around (SharedCrateContext, LocalCrateContext) tuples all over trans. @@ -254,45 +217,15 @@ pub struct CrateContext<'a, 'tcx: 'a> { local_ccx: &'a LocalCrateContext<'tcx>, } -impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> { -} - -pub struct CrateContextIterator<'a, 'tcx: 'a> { - shared: &'a SharedCrateContext<'a, 'tcx>, - local_ccxs: &'a [LocalCrateContext<'tcx>], - index: usize, - - /// if true, only return results where `previous_work_product` is none - filter_to_previous_work_product_unavail: bool, +impl<'a, 'tcx> CrateContext<'a, 'tcx> { + pub fn new(shared: &'a SharedCrateContext<'a, 'tcx>, + local_ccx: &'a LocalCrateContext<'tcx>) + -> Self { + CrateContext { shared, local_ccx } + } } -impl<'a, 'tcx> Iterator for CrateContextIterator<'a,'tcx> { - type Item = CrateContext<'a, 'tcx>; - - fn next(&mut self) -> Option> { - loop { - if self.index >= self.local_ccxs.len() { - return None; - } - - let index = self.index; - self.index += 1; - - let ccx = CrateContext { - shared: self.shared, - local_ccx: &self.local_ccxs[index], - }; - - if - self.filter_to_previous_work_product_unavail && - ccx.previous_work_product().is_some() - { - continue; - } - - return Some(ccx); - } - } +impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> { } pub fn get_reloc_model(sess: &Session) -> llvm::RelocMode { @@ -512,11 +445,10 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { } impl<'tcx> LocalCrateContext<'tcx> { - fn new<'a>(shared: &SharedCrateContext<'a, 'tcx>, - codegen_unit: CodegenUnit<'tcx>, - previous_work_product: Option, - symbol_map: Rc>) - -> LocalCrateContext<'tcx> { + pub fn new<'a>(shared: &SharedCrateContext<'a, 'tcx>, + codegen_unit: CodegenUnit<'tcx>, + symbol_map: Rc>) + -> LocalCrateContext<'tcx> { unsafe { // Append ".rs" to LLVM module identifier. // @@ -542,7 +474,6 @@ impl<'tcx> LocalCrateContext<'tcx> { let local_ccx = LocalCrateContext { llmod: llmod, llcx: llcx, - previous_work_product: previous_work_product, codegen_unit: codegen_unit, needs_unwind_cleanup_cache: RefCell::new(FxHashMap()), instances: RefCell::new(FxHashMap()), @@ -651,10 +582,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.local().llcx } - pub fn previous_work_product(&self) -> Option<&WorkProduct> { - self.local().previous_work_product.as_ref() - } - pub fn codegen_unit(&self) -> &CodegenUnit<'tcx> { &self.local().codegen_unit } From c22fdf9a3ac1e316069d55effbfa5c91cc5c0a12 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 14:58:20 -0400 Subject: [PATCH 12/17] use `tcx.crate_name(LOCAL_CRATE)` rather than `LinkMeta::crate_name` --- src/librustc/middle/cstore.rs | 1 - src/librustc_driver/driver.rs | 2 +- src/librustc_metadata/encoder.rs | 4 ++-- src/librustc_trans/back/link.rs | 6 +----- src/librustc_trans/base.rs | 6 ++++-- src/librustc_trans/context.rs | 4 ++-- src/librustc_trans/debuginfo/metadata.rs | 4 ++-- src/librustc_trans/lib.rs | 2 ++ 8 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index 81cf24e58dda4..cbbfeacadb408 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -53,7 +53,6 @@ pub use self::NativeLibraryKind::*; #[derive(Clone, Debug)] pub struct LinkMeta { - pub crate_name: Symbol, pub crate_hash: Svh, } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 20ae561431b54..6b136d0fa0cbb 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1140,7 +1140,7 @@ pub fn phase_6_link_output(sess: &Session, outputs: &OutputFilenames) { time(sess.time_passes(), "linking", - || link::link_binary(sess, trans, outputs, &trans.link.crate_name.as_str())); + || link::link_binary(sess, trans, outputs, &trans.crate_name.as_str())); } fn escape_dep_filename(filename: &str) -> String { diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index ffe68094c6afc..3bf22f8a6c827 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -14,7 +14,7 @@ use schema::*; use rustc::middle::cstore::{LinkMeta, LinkagePreference, NativeLibrary, EncodedMetadata, EncodedMetadataHash}; -use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefIndex, DefId}; +use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefIndex, DefId, LOCAL_CRATE}; use rustc::hir::map::definitions::DefPathTable; use rustc::middle::dependency_format::Linkage; use rustc::middle::lang_items; @@ -1380,7 +1380,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let link_meta = self.link_meta; let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro); let root = self.lazy(&CrateRoot { - name: link_meta.crate_name, + name: tcx.crate_name(LOCAL_CRATE), triple: tcx.sess.opts.target_triple.clone(), hash: link_meta.crate_hash, disambiguator: tcx.sess.local_crate_disambiguator(), diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 0ffa7a79408e1..7468f4ace1b16 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -47,7 +47,6 @@ use std::str; use flate; use syntax::ast; use syntax::attr; -use syntax::symbol::Symbol; use syntax_pos::Span; /// The LLVM module name containing crate-metadata. This includes a `.` on @@ -136,11 +135,8 @@ pub fn find_crate_name(sess: Option<&Session>, "rust_out".to_string() } -pub fn build_link_meta(incremental_hashes_map: &IncrementalHashesMap, - name: &str) - -> LinkMeta { +pub fn build_link_meta(incremental_hashes_map: &IncrementalHashesMap) -> LinkMeta { let r = LinkMeta { - crate_name: Symbol::intern(name), crate_hash: Svh::new(incremental_hashes_map[&DepNode::Krate].to_smaller_hash()), }; info!("{:?}", r); diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 309161676218b..e8ca2bd5e6c10 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1063,12 +1063,12 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // particular items that will be processed. let krate = tcx.hir.krate(); - let ty::CrateAnalysis { reachable, name, .. } = analysis; + let ty::CrateAnalysis { reachable, .. } = analysis; let exported_symbols = find_exported_symbols(tcx, reachable); let check_overflow = tcx.sess.overflow_checks(); - let link_meta = link::build_link_meta(incremental_hashes_map, &name); + let link_meta = link::build_link_meta(incremental_hashes_map); let shared_ccx = SharedCrateContext::new(tcx, link_meta.clone(), @@ -1096,6 +1096,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let empty_exported_symbols = ExportedSymbols::empty(); let linker_info = LinkerInfo::new(&shared_ccx, &empty_exported_symbols); return CrateTranslation { + crate_name: tcx.crate_name(LOCAL_CRATE), modules: vec![], metadata_module: metadata_module, link: link_meta, @@ -1307,6 +1308,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }); CrateTranslation { + crate_name: tcx.crate_name(LOCAL_CRATE), modules: modules, metadata_module: metadata_module, link: link_meta, diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 80d6f15518f97..0d7e70f6dd1a0 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -13,7 +13,7 @@ use llvm::{ContextRef, ModuleRef, ValueRef}; use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap, DepTrackingMapConfig}; use middle::cstore::LinkMeta; use rustc::hir; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::traits; use debuginfo; use callee; @@ -439,7 +439,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { pub fn metadata_symbol_name(&self) -> String { format!("rust_metadata_{}_{}", - self.link_meta().crate_name, + self.tcx().crate_name(LOCAL_CRATE), self.link_meta().crate_hash) } } diff --git a/src/librustc_trans/debuginfo/metadata.rs b/src/librustc_trans/debuginfo/metadata.rs index 2d1c95114ebd6..1f4756a94ea33 100644 --- a/src/librustc_trans/debuginfo/metadata.rs +++ b/src/librustc_trans/debuginfo/metadata.rs @@ -26,7 +26,7 @@ use llvm::debuginfo::{DIType, DIFile, DIScope, DIDescriptor, DICompositeType, DILexicalBlock, DIFlags}; use rustc::hir::def::CtorKind; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::ty::fold::TypeVisitor; use rustc::ty::subst::Substs; use rustc::ty::util::TypeIdHasher; @@ -810,7 +810,7 @@ pub fn compile_unit_metadata(scc: &SharedCrateContext, }; fn fallback_path(scc: &SharedCrateContext) -> CString { - CString::new(scc.link_meta().crate_name.to_string()).unwrap() + CString::new(scc.tcx().crate_name(LOCAL_CRATE).to_string()).unwrap() } } diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index 628d46f8e7059..abda358bc4f87 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -40,6 +40,7 @@ #![feature(conservative_impl_trait)] use rustc::dep_graph::WorkProduct; +use syntax_pos::symbol::Symbol; extern crate flate; extern crate libc; @@ -165,6 +166,7 @@ unsafe impl Send for ModuleTranslation { } unsafe impl Sync for ModuleTranslation { } pub struct CrateTranslation { + pub crate_name: Symbol, pub modules: Vec, pub metadata_module: ModuleTranslation, pub link: middle::cstore::LinkMeta, From f227187cb8ddd76ef93e630adc56eefc09ae7e59 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 15:55:48 -0400 Subject: [PATCH 13/17] remove `LinkMeta` from `SharedCrateContext` A number of things were using `crate_hash` that really ought to be using `crate_disambiguator` (e.g., to create the plugin symbol names). They have been updated. It is important to remove `LinkMeta` from `SharedCrateContext` since it contains a hash of the entire crate, and hence it will change whenever **anything** changes (which would then require rebuilding **everything**). --- src/librustc/session/mod.rs | 12 +++++------- src/librustc_metadata/creader.rs | 6 +++--- src/librustc_plugin/load.rs | 4 ++-- src/librustc_trans/back/symbol_export.rs | 4 ++-- src/librustc_trans/back/symbol_names.rs | 8 ++++---- src/librustc_trans/base.rs | 7 ++++--- src/librustc_trans/context.rs | 10 +--------- 7 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 039db3d9ee911..adc9aabb8c77a 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -13,7 +13,6 @@ pub use self::code_stats::{SizeKind, TypeSizeInfo, VariantInfo}; use dep_graph::DepGraph; use hir::def_id::{CrateNum, DefIndex}; -use hir::svh::Svh; use lint; use middle::cstore::CrateStore; use middle::dependency_format; @@ -402,15 +401,14 @@ impl Session { /// Returns the symbol name for the registrar function, /// given the crate Svh and the function DefIndex. - pub fn generate_plugin_registrar_symbol(&self, svh: &Svh, index: DefIndex) + pub fn generate_plugin_registrar_symbol(&self, disambiguator: Symbol, index: DefIndex) -> String { - format!("__rustc_plugin_registrar__{}_{}", svh, index.as_usize()) + format!("__rustc_plugin_registrar__{}_{}", disambiguator, index.as_usize()) } - pub fn generate_derive_registrar_symbol(&self, - svh: &Svh, - index: DefIndex) -> String { - format!("__rustc_derive_registrar__{}_{}", svh, index.as_usize()) + pub fn generate_derive_registrar_symbol(&self, disambiguator: Symbol, index: DefIndex) + -> String { + format!("__rustc_derive_registrar__{}_{}", disambiguator, index.as_usize()) } pub fn sysroot<'a>(&'a self) -> &'a Path { diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 04a8b88f8a594..a8ee999505e20 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -600,7 +600,7 @@ impl<'a> CrateLoader<'a> { Err(err) => self.sess.span_fatal(span, &err), }; - let sym = self.sess.generate_derive_registrar_symbol(&root.hash, + let sym = self.sess.generate_derive_registrar_symbol(root.disambiguator, root.macro_derive_registrar.unwrap()); let registrar = unsafe { let sym = match lib.symbol(&sym) { @@ -654,7 +654,7 @@ impl<'a> CrateLoader<'a> { /// Look for a plugin registrar. Returns library path, crate /// SVH and DefIndex of the registrar function. pub fn find_plugin_registrar(&mut self, span: Span, name: &str) - -> Option<(PathBuf, Svh, DefIndex)> { + -> Option<(PathBuf, Symbol, DefIndex)> { let ekrate = self.read_extension_crate(span, &ExternCrateInfo { name: Symbol::intern(name), ident: Symbol::intern(name), @@ -675,7 +675,7 @@ impl<'a> CrateLoader<'a> { let root = ekrate.metadata.get_root(); match (ekrate.dylib.as_ref(), root.plugin_registrar_fn) { (Some(dylib), Some(reg)) => { - Some((dylib.to_path_buf(), root.hash, reg)) + Some((dylib.to_path_buf(), root.disambiguator, reg)) } (None, Some(_)) => { span_err!(self.sess, span, E0457, diff --git a/src/librustc_plugin/load.rs b/src/librustc_plugin/load.rs index e884f3bdbb122..ed49e8a14c8c7 100644 --- a/src/librustc_plugin/load.rs +++ b/src/librustc_plugin/load.rs @@ -100,8 +100,8 @@ impl<'a> PluginLoader<'a> { fn load_plugin(&mut self, span: Span, name: &str, args: Vec) { let registrar = self.reader.find_plugin_registrar(span, name); - if let Some((lib, svh, index)) = registrar { - let symbol = self.sess.generate_plugin_registrar_symbol(&svh, index); + if let Some((lib, disambiguator, index)) = registrar { + let symbol = self.sess.generate_plugin_registrar_symbol(disambiguator, index); let fun = self.dylink_registrar(span, lib, symbol); self.plugins.push(PluginRegistrar { fun: fun, diff --git a/src/librustc_trans/back/symbol_export.rs b/src/librustc_trans/back/symbol_export.rs index 23a67ef5046ee..04617edf4a7a9 100644 --- a/src/librustc_trans/back/symbol_export.rs +++ b/src/librustc_trans/back/symbol_export.rs @@ -64,10 +64,10 @@ impl ExportedSymbols { } if let Some(id) = scx.sess().derive_registrar_fn.get() { - let svh = &scx.link_meta().crate_hash; let def_id = scx.tcx().hir.local_def_id(id); let idx = def_id.index; - let registrar = scx.sess().generate_derive_registrar_symbol(svh, idx); + let disambiguator = scx.sess().local_crate_disambiguator(); + let registrar = scx.sess().generate_derive_registrar_symbol(disambiguator, idx); local_crate.push((registrar, SymbolExportLevel::C)); } diff --git a/src/librustc_trans/back/symbol_names.rs b/src/librustc_trans/back/symbol_names.rs index 3568c1ba8f4c1..8facbd6cc2783 100644 --- a/src/librustc_trans/back/symbol_names.rs +++ b/src/librustc_trans/back/symbol_names.rs @@ -179,14 +179,14 @@ pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>, if let Some(id) = node_id { if scx.sess().plugin_registrar_fn.get() == Some(id) { - let svh = &scx.link_meta().crate_hash; let idx = def_id.index; - return scx.sess().generate_plugin_registrar_symbol(svh, idx); + let disambiguator = scx.sess().local_crate_disambiguator(); + return scx.sess().generate_plugin_registrar_symbol(disambiguator, idx); } if scx.sess().derive_registrar_fn.get() == Some(id) { - let svh = &scx.link_meta().crate_hash; let idx = def_id.index; - return scx.sess().generate_derive_registrar_symbol(svh, idx); + let disambiguator = scx.sess().local_crate_disambiguator(); + return scx.sess().generate_derive_registrar_symbol(disambiguator, idx); } } diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index e8ca2bd5e6c10..cec599e28484b 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -39,6 +39,7 @@ use middle::lang_items::StartFnLangItem; use middle::cstore::EncodedMetadata; use rustc::ty::{self, Ty, TyCtxt}; use rustc::dep_graph::{AssertDepGraphSafe, DepNode}; +use rustc::middle::cstore::LinkMeta; use rustc::hir::map as hir_map; use rustc::util::common::time; use session::config::{self, NoDebugInfo}; @@ -725,6 +726,7 @@ fn contains_null(s: &str) -> bool { } fn write_metadata(cx: &SharedCrateContext, + link_meta: &LinkMeta, exported_symbols: &NodeSet) -> (ContextRef, ModuleRef, EncodedMetadata) { use flate; @@ -762,7 +764,7 @@ fn write_metadata(cx: &SharedCrateContext, let cstore = &cx.tcx().sess.cstore; let metadata = cstore.encode_metadata(cx.tcx(), - cx.link_meta(), + &link_meta, exported_symbols); if kind == MetadataKind::Uncompressed { return (metadata_llcx, metadata_llmod, metadata); @@ -1071,13 +1073,12 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let link_meta = link::build_link_meta(incremental_hashes_map); let shared_ccx = SharedCrateContext::new(tcx, - link_meta.clone(), exported_symbols, check_overflow); // Translate the metadata. let (metadata_llcx, metadata_llmod, metadata) = time(tcx.sess.time_passes(), "write metadata", || { - write_metadata(&shared_ccx, shared_ccx.exported_symbols()) + write_metadata(&shared_ccx, &link_meta, shared_ccx.exported_symbols()) }); let metadata_module = ModuleTranslation { diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 0d7e70f6dd1a0..9d18b4574bcc4 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -11,7 +11,6 @@ use llvm; use llvm::{ContextRef, ModuleRef, ValueRef}; use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap, DepTrackingMapConfig}; -use middle::cstore::LinkMeta; use rustc::hir; use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::traits; @@ -65,7 +64,6 @@ pub struct Stats { /// (aside from metadata-related ones). pub struct SharedCrateContext<'a, 'tcx: 'a> { exported_symbols: NodeSet, - link_meta: LinkMeta, tcx: TyCtxt<'a, 'tcx, 'tcx>, empty_param_env: ty::ParameterEnvironment<'tcx>, stats: Stats, @@ -316,7 +314,6 @@ pub unsafe fn create_context_and_module(sess: &Session, mod_name: &str) -> (Cont impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { pub fn new(tcx: TyCtxt<'b, 'tcx, 'tcx>, - link_meta: LinkMeta, exported_symbols: NodeSet, check_overflow: bool) -> SharedCrateContext<'b, 'tcx> { @@ -367,7 +364,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { SharedCrateContext { exported_symbols: exported_symbols, - link_meta: link_meta, empty_param_env: tcx.empty_parameter_environment(), tcx: tcx, stats: Stats { @@ -409,10 +405,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { &self.project_cache } - pub fn link_meta<'a>(&'a self) -> &'a LinkMeta { - &self.link_meta - } - pub fn tcx<'a>(&'a self) -> TyCtxt<'a, 'tcx, 'tcx> { self.tcx } @@ -440,7 +432,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { pub fn metadata_symbol_name(&self) -> String { format!("rust_metadata_{}_{}", self.tcx().crate_name(LOCAL_CRATE), - self.link_meta().crate_hash) + self.tcx().crate_disambiguator(LOCAL_CRATE)) } } From 8e26983c86c6e6a51c85e1bd5c4621f3b43afe99 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 17:11:54 -0400 Subject: [PATCH 14/17] pull stats out of `SharedCrateContext` shared mutable state is bad --- src/librustc_trans/base.rs | 70 ++++++++++++++++++++--------------- src/librustc_trans/context.rs | 41 +++++++++++--------- 2 files changed, 64 insertions(+), 47 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index cec599e28484b..3fd966eb73ee6 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -57,7 +57,7 @@ use common::CrateContext; use common::{type_is_zero_size, val_ty}; use common; use consts; -use context::{self, LocalCrateContext, SharedCrateContext}; +use context::{self, LocalCrateContext, SharedCrateContext, Stats}; use debuginfo; use declare; use machine; @@ -1115,21 +1115,25 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let symbol_map = Rc::new(symbol_map); + let mut all_stats = Stats::default(); let modules: Vec = codegen_units .into_iter() .map(|cgu| { let dep_node = cgu.work_product_dep_node(); - tcx.dep_graph.with_task(dep_node, - AssertDepGraphSafe(&shared_ccx), - AssertDepGraphSafe((cgu, symbol_map.clone())), - module_translation) + let (stats, module) = + tcx.dep_graph.with_task(dep_node, + AssertDepGraphSafe(&shared_ccx), + AssertDepGraphSafe((cgu, symbol_map.clone())), + module_translation); + all_stats.extend(stats); + module }) .collect(); fn module_translation<'a, 'tcx>( scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>, args: AssertDepGraphSafe<(CodegenUnit<'tcx>, Rc>)>) - -> ModuleTranslation + -> (Stats, ModuleTranslation) { // FIXME(#40304): We ought to be using the id as a key and some queries, I think. let AssertDepGraphSafe(scx) = scx; @@ -1161,12 +1165,19 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } }); - let source = if let Some(buf) = previous_work_product { + if let Some(buf) = previous_work_product { // Don't need to translate this module. - ModuleSource::Preexisting(buf.clone()) - } else { - // Instantiate translation items without filling out definitions yet... - let lcx = LocalCrateContext::new(scx, cgu, symbol_map.clone()); + let module = ModuleTranslation { + name: cgu_name, + symbol_name_hash, + source: ModuleSource::Preexisting(buf.clone()) + }; + return (Stats::default(), module); + } + + // Instantiate translation items without filling out definitions yet... + let lcx = LocalCrateContext::new(scx, cgu, symbol_map.clone()); + let module = { let ccx = CrateContext::new(scx, &lcx); let trans_items = ccx.codegen_unit() .items_in_deterministic_order(ccx.tcx(), &symbol_map); @@ -1214,17 +1225,17 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, debuginfo::finalize(&ccx); } - ModuleSource::Translated(ModuleLlvm { - llcx: ccx.llcx(), - llmod: ccx.llmod(), - }) + ModuleTranslation { + name: cgu_name, + symbol_name_hash, + source: ModuleSource::Translated(ModuleLlvm { + llcx: ccx.llcx(), + llmod: ccx.llmod(), + }) + } }; - ModuleTranslation { - name: cgu_name, - symbol_name_hash, - source, - } + (lcx.into_stats(), module) } assert_module_sources::assert_module_sources(tcx, &modules); @@ -1232,20 +1243,19 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, symbol_names_test::report_symbol_names(&shared_ccx); if shared_ccx.sess().trans_stats() { - let stats = shared_ccx.stats(); println!("--- trans stats ---"); - println!("n_glues_created: {}", stats.n_glues_created.get()); - println!("n_null_glues: {}", stats.n_null_glues.get()); - println!("n_real_glues: {}", stats.n_real_glues.get()); + println!("n_glues_created: {}", all_stats.n_glues_created.get()); + println!("n_null_glues: {}", all_stats.n_null_glues.get()); + println!("n_real_glues: {}", all_stats.n_real_glues.get()); - println!("n_fns: {}", stats.n_fns.get()); - println!("n_inlines: {}", stats.n_inlines.get()); - println!("n_closures: {}", stats.n_closures.get()); + println!("n_fns: {}", all_stats.n_fns.get()); + println!("n_inlines: {}", all_stats.n_inlines.get()); + println!("n_closures: {}", all_stats.n_closures.get()); println!("fn stats:"); - stats.fn_stats.borrow_mut().sort_by(|&(_, insns_a), &(_, insns_b)| { + all_stats.fn_stats.borrow_mut().sort_by(|&(_, insns_a), &(_, insns_b)| { insns_b.cmp(&insns_a) }); - for tuple in stats.fn_stats.borrow().iter() { + for tuple in all_stats.fn_stats.borrow().iter() { match *tuple { (ref name, insns) => { println!("{} insns, {}", insns, *name); @@ -1255,7 +1265,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } if shared_ccx.sess().count_llvm_insns() { - for (k, v) in shared_ccx.stats().llvm_insns.borrow().iter() { + for (k, v) in all_stats.llvm_insns.borrow().iter() { println!("{:7} {}", *v, *k); } } diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 9d18b4574bcc4..7e49dee498d00 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -45,6 +45,7 @@ use syntax::symbol::InternedString; use syntax_pos::DUMMY_SP; use abi::Abi; +#[derive(Clone, Default)] pub struct Stats { pub n_glues_created: Cell, pub n_null_glues: Cell, @@ -58,6 +59,22 @@ pub struct Stats { pub fn_stats: RefCell >, } +impl Stats { + pub fn extend(&mut self, stats: Stats) { + self.n_glues_created.set(self.n_glues_created.get() + stats.n_glues_created.get()); + self.n_null_glues.set(self.n_null_glues.get() + stats.n_null_glues.get()); + self.n_real_glues.set(self.n_real_glues.get() + stats.n_real_glues.get()); + self.n_fns.set(self.n_fns.get() + stats.n_fns.get()); + self.n_inlines.set(self.n_inlines.get() + stats.n_inlines.get()); + self.n_closures.set(self.n_closures.get() + stats.n_closures.get()); + self.n_llvm_insns.set(self.n_llvm_insns.get() + stats.n_llvm_insns.get()); + self.llvm_insns.borrow_mut().extend( + stats.llvm_insns.borrow().iter() + .map(|(key, value)| (key.clone(), value.clone()))); + self.fn_stats.borrow_mut().append(&mut *stats.fn_stats.borrow_mut()); + } +} + /// The shared portion of a `CrateContext`. There is one `SharedCrateContext` /// per crate. The data here is shared between all compilation units of the /// crate, so it must not contain references to any LLVM data structures @@ -66,7 +83,6 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { exported_symbols: NodeSet, tcx: TyCtxt<'a, 'tcx, 'tcx>, empty_param_env: ty::ParameterEnvironment<'tcx>, - stats: Stats, check_overflow: bool, use_dll_storage_attrs: bool, @@ -83,6 +99,7 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { pub struct LocalCrateContext<'tcx> { llmod: ModuleRef, llcx: ContextRef, + stats: Stats, codegen_unit: CodegenUnit<'tcx>, needs_unwind_cleanup_cache: RefCell, bool>>, /// Cache instances of monomorphic and polymorphic items @@ -366,17 +383,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { exported_symbols: exported_symbols, empty_param_env: tcx.empty_parameter_environment(), tcx: tcx, - stats: Stats { - n_glues_created: Cell::new(0), - n_null_glues: Cell::new(0), - n_real_glues: Cell::new(0), - n_fns: Cell::new(0), - n_inlines: Cell::new(0), - n_closures: Cell::new(0), - n_llvm_insns: Cell::new(0), - llvm_insns: RefCell::new(FxHashMap()), - fn_stats: RefCell::new(Vec::new()), - }, check_overflow: check_overflow, use_dll_storage_attrs: use_dll_storage_attrs, translation_items: RefCell::new(FxHashSet()), @@ -417,10 +423,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { &self.tcx.dep_graph } - pub fn stats<'a>(&'a self) -> &'a Stats { - &self.stats - } - pub fn use_dll_storage_attrs(&self) -> bool { self.use_dll_storage_attrs } @@ -466,6 +468,7 @@ impl<'tcx> LocalCrateContext<'tcx> { let local_ccx = LocalCrateContext { llmod: llmod, llcx: llcx, + stats: Stats::default(), codegen_unit: codegen_unit, needs_unwind_cleanup_cache: RefCell::new(FxHashMap()), instances: RefCell::new(FxHashMap()), @@ -537,6 +540,10 @@ impl<'tcx> LocalCrateContext<'tcx> { local_ccx: &local_ccxs[0] } } + + pub fn into_stats(self) -> Stats { + self.stats + } } impl<'b, 'tcx> CrateContext<'b, 'tcx> { @@ -651,7 +658,7 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { } pub fn stats<'a>(&'a self) -> &'a Stats { - &self.shared.stats + &self.local().stats } pub fn int_type(&self) -> Type { From f2487b81523a4f39a5ad27a9d28b198e4cc4dfda Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 18:08:36 -0400 Subject: [PATCH 15/17] refactor `metadata_symbol_name` --- src/librustc_trans/back/symbol_export.rs | 9 ++++++++- src/librustc_trans/base.rs | 2 +- src/librustc_trans/context.rs | 8 +------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_trans/back/symbol_export.rs b/src/librustc_trans/back/symbol_export.rs index 04617edf4a7a9..5d6717272fdf3 100644 --- a/src/librustc_trans/back/symbol_export.rs +++ b/src/librustc_trans/back/symbol_export.rs @@ -15,6 +15,7 @@ use back::symbol_names::symbol_name; 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; @@ -72,7 +73,7 @@ impl ExportedSymbols { } if scx.sess().crate_types.borrow().contains(&config::CrateTypeDylib) { - local_crate.push((scx.metadata_symbol_name(), + local_crate.push((metadata_symbol_name(scx.tcx()), SymbolExportLevel::Rust)); } @@ -173,6 +174,12 @@ impl ExportedSymbols { } } +pub fn metadata_symbol_name(tcx: TyCtxt) -> String { + format!("rust_metadata_{}_{}", + tcx.crate_name(LOCAL_CRATE), + tcx.crate_disambiguator(LOCAL_CRATE)) +} + pub fn crate_export_threshold(crate_type: config::CrateType) -> SymbolExportLevel { match crate_type { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 3fd966eb73ee6..b364814d07e80 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -776,7 +776,7 @@ fn write_metadata(cx: &SharedCrateContext, let llmeta = C_bytes_in_context(metadata_llcx, &compressed); let llconst = C_struct_in_context(metadata_llcx, &[llmeta], false); - let name = cx.metadata_symbol_name(); + let name = symbol_export::metadata_symbol_name(cx.tcx()); let buf = CString::new(name).unwrap(); let llglobal = unsafe { llvm::LLVMAddGlobal(metadata_llmod, val_ty(llconst).to_ref(), buf.as_ptr()) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 7e49dee498d00..c3770470bfd05 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -12,7 +12,7 @@ use llvm; use llvm::{ContextRef, ModuleRef, ValueRef}; use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap, DepTrackingMapConfig}; use rustc::hir; -use rustc::hir::def_id::{DefId, LOCAL_CRATE}; +use rustc::hir::def_id::DefId; use rustc::traits; use debuginfo; use callee; @@ -430,12 +430,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { pub fn translation_items(&self) -> &RefCell>> { &self.translation_items } - - pub fn metadata_symbol_name(&self) -> String { - format!("rust_metadata_{}_{}", - self.tcx().crate_name(LOCAL_CRATE), - self.tcx().crate_disambiguator(LOCAL_CRATE)) - } } impl<'tcx> LocalCrateContext<'tcx> { From 07fb93e65ad2c371a59e8f671650c5b343ef5487 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 13 Apr 2017 18:21:51 -0400 Subject: [PATCH 16/17] make `write_metadata` take `tcx` intead of `SharedCrateContext` --- src/librustc_trans/base.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index b364814d07e80..c770bbdb90f72 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -725,14 +725,14 @@ fn contains_null(s: &str) -> bool { s.bytes().any(|b| b == 0) } -fn write_metadata(cx: &SharedCrateContext, - link_meta: &LinkMeta, - exported_symbols: &NodeSet) - -> (ContextRef, ModuleRef, EncodedMetadata) { +fn write_metadata<'a, 'gcx>(tcx: TyCtxt<'a, 'gcx, 'gcx>, + link_meta: &LinkMeta, + exported_symbols: &NodeSet) + -> (ContextRef, ModuleRef, EncodedMetadata) { use flate; let (metadata_llcx, metadata_llmod) = unsafe { - context::create_context_and_module(cx.sess(), "metadata") + context::create_context_and_module(tcx.sess, "metadata") }; #[derive(PartialEq, Eq, PartialOrd, Ord)] @@ -742,7 +742,7 @@ fn write_metadata(cx: &SharedCrateContext, Compressed } - let kind = cx.sess().crate_types.borrow().iter().map(|ty| { + let kind = tcx.sess.crate_types.borrow().iter().map(|ty| { match *ty { config::CrateTypeExecutable | config::CrateTypeStaticlib | @@ -762,8 +762,8 @@ fn write_metadata(cx: &SharedCrateContext, }); } - let cstore = &cx.tcx().sess.cstore; - let metadata = cstore.encode_metadata(cx.tcx(), + let cstore = &tcx.sess.cstore; + let metadata = cstore.encode_metadata(tcx, &link_meta, exported_symbols); if kind == MetadataKind::Uncompressed { @@ -776,7 +776,7 @@ fn write_metadata(cx: &SharedCrateContext, let llmeta = C_bytes_in_context(metadata_llcx, &compressed); let llconst = C_struct_in_context(metadata_llcx, &[llmeta], false); - let name = symbol_export::metadata_symbol_name(cx.tcx()); + let name = symbol_export::metadata_symbol_name(tcx); let buf = CString::new(name).unwrap(); let llglobal = unsafe { llvm::LLVMAddGlobal(metadata_llmod, val_ty(llconst).to_ref(), buf.as_ptr()) @@ -784,7 +784,7 @@ fn write_metadata(cx: &SharedCrateContext, unsafe { llvm::LLVMSetInitializer(llglobal, llconst); let section_name = - cx.tcx().sess.cstore.metadata_section_name(&cx.sess().target.target); + tcx.sess.cstore.metadata_section_name(&tcx.sess.target.target); let name = CString::new(section_name).unwrap(); llvm::LLVMSetSection(llglobal, name.as_ptr()); @@ -1078,7 +1078,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Translate the metadata. let (metadata_llcx, metadata_llmod, metadata) = time(tcx.sess.time_passes(), "write metadata", || { - write_metadata(&shared_ccx, &link_meta, shared_ccx.exported_symbols()) + write_metadata(tcx, &link_meta, shared_ccx.exported_symbols()) }); let metadata_module = ModuleTranslation { From baeec7b8eb3ee0fd7a40bff409400903e5679ff5 Mon Sep 17 00:00:00 2001 From: est31 Date: Fri, 14 Apr 2017 02:58:54 +0200 Subject: [PATCH 17/17] Avoid to use floating point match Its going to be forbidden, see issue 41255. --- src/librand/distributions/gamma.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/librand/distributions/gamma.rs b/src/librand/distributions/gamma.rs index e024b62adfb15..9a42b82beff67 100644 --- a/src/librand/distributions/gamma.rs +++ b/src/librand/distributions/gamma.rs @@ -103,12 +103,14 @@ impl Gamma { assert!(shape > 0.0, "Gamma::new called with shape <= 0"); assert!(scale > 0.0, "Gamma::new called with scale <= 0"); - let repr = match shape { - 1.0 => One(Exp::new(1.0 / scale)), - 0.0...1.0 => Small(GammaSmallShape::new_raw(shape, scale)), - _ => Large(GammaLargeShape::new_raw(shape, scale)), + let repr = if shape == 1.0 { + One(Exp::new(1.0 / scale)) + } else if 0.0 <= shape && shape < 1.0 { + Small(GammaSmallShape::new_raw(shape, scale)) + } else { + Large(GammaLargeShape::new_raw(shape, scale)) }; - Gamma { repr: repr } + Gamma { repr } } }