From 79d8d087a3d911ddcd881e452a7e72a8a7121435 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 2 Jul 2018 16:21:34 +0200 Subject: [PATCH 1/2] incr.comp.: Take names of children into account when computing the ICH of a module's HIR. --- src/librustc/ich/impls_hir.rs | 29 +++++++++++++++++++------- src/librustc_codegen_llvm/mono_item.rs | 5 ++--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 0c7baea85ad8f..8917051bfd8ae 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -756,13 +756,28 @@ impl_stable_hash_for!(enum hir::ImplPolarity { Negative }); -impl_stable_hash_for!(struct hir::Mod { - inner, - // We are not hashing the IDs of the items contained in the module. - // This is harmless and matches the current behavior but it's not - // actually correct. See issue #40876. - item_ids -> _, -}); +impl<'a> HashStable> for hir::Mod { + fn hash_stable(&self, + hcx: &mut StableHashingContext<'a>, + hasher: &mut StableHasher) { + let hir::Mod { + inner: ref inner_span, + ref item_ids, + } = *self; + + inner_span.hash_stable(hcx, hasher); + + let mut item_ids: Vec = item_ids.iter().map(|id| { + let (def_path_hash, local_id) = id.id.to_stable_hash_key(hcx); + debug_assert_eq!(local_id, hir::ItemLocalId(0)); + def_path_hash + }).collect(); + + item_ids.sort_unstable(); + + item_ids.hash_stable(hcx, hasher); + } +} impl_stable_hash_for!(struct hir::ForeignMod { abi, diff --git a/src/librustc_codegen_llvm/mono_item.rs b/src/librustc_codegen_llvm/mono_item.rs index 6ba3582f0143b..c4a23ac653ca0 100644 --- a/src/librustc_codegen_llvm/mono_item.rs +++ b/src/librustc_codegen_llvm/mono_item.rs @@ -25,11 +25,10 @@ use monomorphize::Instance; use type_of::LayoutLlvmExt; use rustc::hir; use rustc::hir::def::Def; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::mir::mono::{Linkage, Visibility}; use rustc::ty::TypeFoldable; use rustc::ty::layout::LayoutOf; -use syntax::attr; use std::fmt; pub use rustc::mir::mono::MonoItem; @@ -173,7 +172,7 @@ fn predefine_fn<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, // visibility as we're going to link this object all over the place but // don't want the symbols to get exported. if linkage != Linkage::Internal && linkage != Linkage::Private && - attr::contains_name(cx.tcx.hir.krate_attrs(), "compiler_builtins") { + cx.tcx.is_compiler_builtins(LOCAL_CRATE) { unsafe { llvm::LLVMRustSetVisibility(lldecl, llvm::Visibility::Hidden); } From 447f1f3f5cca20c15d2ed023310b07218aabe7fa Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 3 Jul 2018 11:16:38 +0200 Subject: [PATCH 2/2] Avoid sorting the item_ids array the StableHash impl of hir::Mod. --- src/librustc/hir/map/definitions.rs | 8 ++++++++ src/librustc/ich/fingerprint.rs | 12 ++++++++++++ src/librustc/ich/impls_hir.rs | 26 ++++++++++++++++---------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index fa01a1ccca59b..328cb8225478b 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -23,6 +23,7 @@ use rustc_data_structures::indexed_vec::{IndexVec}; use rustc_data_structures::stable_hasher::StableHasher; use serialize::{Encodable, Decodable, Encoder, Decoder}; use session::CrateDisambiguator; +use std::borrow::Borrow; use std::fmt::Write; use std::hash::Hash; use syntax::ast; @@ -389,6 +390,13 @@ pub struct DefPathHash(pub Fingerprint); impl_stable_hash_for!(tuple_struct DefPathHash { fingerprint }); +impl Borrow for DefPathHash { + #[inline] + fn borrow(&self) -> &Fingerprint { + &self.0 + } +} + impl Definitions { /// Create new empty definition map. pub fn new() -> Definitions { diff --git a/src/librustc/ich/fingerprint.rs b/src/librustc/ich/fingerprint.rs index 2a3b1ce6a36a5..a6e35d78dcb5a 100644 --- a/src/librustc/ich/fingerprint.rs +++ b/src/librustc/ich/fingerprint.rs @@ -45,6 +45,18 @@ impl Fingerprint { ) } + // Combines two hashes in an order independent way. Make sure this is what + // you want. + #[inline] + pub fn combine_commutative(self, other: Fingerprint) -> Fingerprint { + let a = (self.1 as u128) << 64 | self.0 as u128; + let b = (other.1 as u128) << 64 | other.0 as u128; + + let c = a.wrapping_add(b); + + Fingerprint((c >> 64) as u64, c as u64) + } + pub fn to_hex(&self) -> String { format!("{:x}{:x}", self.0, self.1) } diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 8917051bfd8ae..70905bf9612f2 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -14,7 +14,7 @@ use hir; use hir::map::DefPathHash; use hir::def_id::{DefId, LocalDefId, CrateNum, CRATE_DEF_INDEX}; -use ich::{StableHashingContext, NodeIdHashingMode}; +use ich::{StableHashingContext, NodeIdHashingMode, Fingerprint}; use rustc_data_structures::stable_hasher::{HashStable, ToStableHashKey, StableHasher, StableHasherResult}; use std::mem; @@ -767,15 +767,21 @@ impl<'a> HashStable> for hir::Mod { inner_span.hash_stable(hcx, hasher); - let mut item_ids: Vec = item_ids.iter().map(|id| { - let (def_path_hash, local_id) = id.id.to_stable_hash_key(hcx); - debug_assert_eq!(local_id, hir::ItemLocalId(0)); - def_path_hash - }).collect(); - - item_ids.sort_unstable(); - - item_ids.hash_stable(hcx, hasher); + // Combining the DefPathHashes directly is faster than feeding them + // into the hasher. Because we use a commutative combine, we also don't + // have to sort the array. + let item_ids_hash = item_ids + .iter() + .map(|id| { + let (def_path_hash, local_id) = id.id.to_stable_hash_key(hcx); + debug_assert_eq!(local_id, hir::ItemLocalId(0)); + def_path_hash.0 + }).fold(Fingerprint::ZERO, |a, b| { + a.combine_commutative(b) + }); + + item_ids.len().hash_stable(hcx, hasher); + item_ids_hash.hash_stable(hcx, hasher); } }