From 3e62c129bf69ea784c98d276b07c7f3ab5f2f512 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 7 Sep 2021 13:05:45 +0200 Subject: [PATCH 1/2] fix non_blanket_impls iteration order --- compiler/rustc_middle/src/ich/hcx.rs | 17 +++++++---------- .../src/traits/specialization_graph.rs | 6 +++--- compiler/rustc_middle/src/ty/fast_reject.rs | 9 ++++++++- compiler/rustc_middle/src/ty/trait_def.rs | 19 +++++++++++-------- .../traits/specialize/specialization_graph.rs | 15 ++++++++------- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_middle/src/ich/hcx.rs b/compiler/rustc_middle/src/ich/hcx.rs index 32ccdafaeb48c..e09caf7ca4e80 100644 --- a/compiler/rustc_middle/src/ich/hcx.rs +++ b/compiler/rustc_middle/src/ich/hcx.rs @@ -3,7 +3,7 @@ use crate::middle::cstore::CrateStore; use crate::ty::{fast_reject, TyCtxt}; use rustc_ast as ast; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::Lrc; use rustc_hir as hir; @@ -15,7 +15,7 @@ use rustc_span::symbol::Symbol; use rustc_span::{BytePos, CachingSourceMapView, SourceFile, SpanData}; use smallvec::SmallVec; -use std::cmp::Ord; +use std::collections::BTreeMap; fn compute_ignored_attr_names() -> FxHashSet { debug_assert!(!ich::IGNORED_ATTRIBUTES.is_empty()); @@ -243,7 +243,7 @@ pub fn hash_stable_trait_impls<'a>( hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher, blanket_impls: &[DefId], - non_blanket_impls: &FxHashMap>, + non_blanket_impls: &BTreeMap>, ) { { let mut blanket_impls: SmallVec<[_; 8]> = @@ -257,14 +257,11 @@ pub fn hash_stable_trait_impls<'a>( } { - let mut keys: SmallVec<[_; 8]> = - non_blanket_impls.keys().map(|k| (k, k.map_def(|d| hcx.def_path_hash(d)))).collect(); - keys.sort_unstable_by(|&(_, ref k1), &(_, ref k2)| k1.cmp(k2)); - keys.len().hash_stable(hcx, hasher); - for (key, ref stable_key) in keys { - stable_key.hash_stable(hcx, hasher); + non_blanket_impls.len().hash_stable(hcx, hasher); + for (key, impls) in non_blanket_impls.iter() { + key.hash_stable(hcx, hasher); let mut impls: SmallVec<[_; 8]> = - non_blanket_impls[key].iter().map(|&impl_id| hcx.def_path_hash(impl_id)).collect(); + impls.iter().map(|&impl_id| hcx.def_path_hash(impl_id)).collect(); if impls.len() > 1 { impls.sort_unstable(); diff --git a/compiler/rustc_middle/src/traits/specialization_graph.rs b/compiler/rustc_middle/src/traits/specialization_graph.rs index cb60bfa4c5408..c2ec12c0a75b3 100644 --- a/compiler/rustc_middle/src/traits/specialization_graph.rs +++ b/compiler/rustc_middle/src/traits/specialization_graph.rs @@ -1,12 +1,12 @@ use crate::ich::{self, StableHashingContext}; -use crate::ty::fast_reject::SimplifiedType; +use crate::ty::fast_reject::StableSimplifiedType; use crate::ty::fold::TypeFoldable; use crate::ty::{self, TyCtxt}; -use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_errors::ErrorReported; use rustc_hir::def_id::{DefId, DefIdMap}; use rustc_span::symbol::Ident; +use std::collections::BTreeMap; /// A per-trait graph of impls in specialization order. At the moment, this /// graph forms a tree rooted with the trait itself, with all other nodes @@ -62,7 +62,7 @@ pub struct Children { // together *all* the impls for a trait, and are populated prior to building // the specialization graph. /// Impls of the trait. - pub nonblanket_impls: FxHashMap>, + pub nonblanket_impls: BTreeMap>, /// Blanket impls associated with the trait. pub blanket_impls: Vec, diff --git a/compiler/rustc_middle/src/ty/fast_reject.rs b/compiler/rustc_middle/src/ty/fast_reject.rs index 94d75a469d3d7..e1c281d8cbd9f 100644 --- a/compiler/rustc_middle/src/ty/fast_reject.rs +++ b/compiler/rustc_middle/src/ty/fast_reject.rs @@ -1,7 +1,7 @@ use crate::ich::StableHashingContext; use crate::ty::{self, Ty, TyCtxt}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{DefId, DefPathHash}; use std::fmt::Debug; use std::hash::Hash; use std::mem; @@ -9,6 +9,7 @@ use std::mem; use self::SimplifiedTypeGen::*; pub type SimplifiedType = SimplifiedTypeGen; +pub type StableSimplifiedType = SimplifiedTypeGen; /// See `simplify_type` /// @@ -107,6 +108,12 @@ pub fn simplify_type( } } +impl SimplifiedType { + pub fn to_stable(self, tcx: TyCtxt<'tcx>) -> StableSimplifiedType { + self.map_def(|def_id| tcx.def_path_hash(def_id)) + } +} + impl SimplifiedTypeGen { pub fn map_def(self, map: F) -> SimplifiedTypeGen where diff --git a/compiler/rustc_middle/src/ty/trait_def.rs b/compiler/rustc_middle/src/ty/trait_def.rs index ae86f51e6ac3f..922bb7fcdd16a 100644 --- a/compiler/rustc_middle/src/ty/trait_def.rs +++ b/compiler/rustc_middle/src/ty/trait_def.rs @@ -3,14 +3,13 @@ use crate::traits::specialization_graph; use crate::ty::fast_reject; use crate::ty::fold::TypeFoldable; use crate::ty::{Ty, TyCtxt}; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_errors::ErrorReported; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::definitions::DefPathHash; - -use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_errors::ErrorReported; use rustc_macros::HashStable; +use std::collections::BTreeMap; /// A trait's definition with type information. #[derive(HashStable)] @@ -70,7 +69,7 @@ pub enum TraitSpecializationKind { pub struct TraitImpls { blanket_impls: Vec, /// Impls indexed by their simplified self type, for fast lookup. - non_blanket_impls: FxHashMap>, + non_blanket_impls: BTreeMap>, } impl TraitImpls { @@ -182,7 +181,7 @@ impl<'tcx> TyCtxt<'tcx> { // // I think we'll cross that bridge when we get to it. if let Some(simp) = fast_reject::simplify_type(self, self_ty, true) { - if let Some(impls) = impls.non_blanket_impls.get(&simp) { + if let Some(impls) = impls.non_blanket_impls.get(&simp.to_stable(self)) { for &impl_def_id in impls { if let result @ Some(_) = f(impl_def_id) { return result; @@ -222,7 +221,7 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait if let Some(simplified_self_ty) = simplified_self_ty { impls .non_blanket_impls - .entry(simplified_self_ty) + .entry(simplified_self_ty.to_stable(tcx)) .or_default() .push(impl_def_id); } else { @@ -241,7 +240,11 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait } if let Some(simplified_self_ty) = fast_reject::simplify_type(tcx, impl_self_ty, false) { - impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id); + impls + .non_blanket_impls + .entry(simplified_self_ty.to_stable(tcx)) + .or_default() + .push(impl_def_id); } else { impls.blanket_impls.push(impl_def_id); } diff --git a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs b/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs index c8bcab6efd7ca..1684c35702ace 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs @@ -2,7 +2,7 @@ use super::OverlapError; use crate::traits; use rustc_hir::def_id::DefId; -use rustc_middle::ty::fast_reject::{self, SimplifiedType}; +use rustc_middle::ty::fast_reject::{self, StableSimplifiedType}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; @@ -40,7 +40,7 @@ trait ChildrenExt { &mut self, tcx: TyCtxt<'tcx>, impl_def_id: DefId, - simplified_self: Option, + simplified_self: Option, ) -> Result; } @@ -50,7 +50,7 @@ impl ChildrenExt for Children { let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false) { debug!("insert_blindly: impl_def_id={:?} st={:?}", impl_def_id, st); - self.nonblanket_impls.entry(st).or_default().push(impl_def_id) + self.nonblanket_impls.entry(st.to_stable(tcx)).or_default().push(impl_def_id) } else { debug!("insert_blindly: impl_def_id={:?} st=None", impl_def_id); self.blanket_impls.push(impl_def_id) @@ -65,7 +65,7 @@ impl ChildrenExt for Children { let vec: &mut Vec; if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false) { debug!("remove_existing: impl_def_id={:?} st={:?}", impl_def_id, st); - vec = self.nonblanket_impls.get_mut(&st).unwrap(); + vec = self.nonblanket_impls.get_mut(&st.to_stable(tcx)).unwrap(); } else { debug!("remove_existing: impl_def_id={:?} st=None", impl_def_id); vec = &mut self.blanket_impls; @@ -81,7 +81,7 @@ impl ChildrenExt for Children { &mut self, tcx: TyCtxt<'tcx>, impl_def_id: DefId, - simplified_self: Option, + simplified_self: Option, ) -> Result { let mut last_lint = None; let mut replace_children = Vec::new(); @@ -222,7 +222,7 @@ fn iter_children(children: &mut Children) -> impl Iterator + '_ { fn filtered_children( children: &mut Children, - st: SimplifiedType, + st: StableSimplifiedType, ) -> impl Iterator + '_ { let nonblanket = children.nonblanket_impls.entry(st).or_default().iter(); children.blanket_impls.iter().chain(nonblanket).cloned() @@ -304,7 +304,8 @@ impl GraphExt for Graph { let mut parent = trait_def_id; let mut last_lint = None; - let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false); + let simplified = + fast_reject::simplify_type(tcx, trait_ref.self_ty(), false).map(|st| st.to_stable(tcx)); // Descend the specialization tree, where `parent` is the current parent node. loop { From 5ebbb287413fe36ebed1987cc77412f065bce822 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 7 Sep 2021 13:06:09 +0200 Subject: [PATCH 2/2] variable naming --- compiler/rustc_middle/src/traits/specialization_graph.rs | 8 ++++---- .../src/traits/specialize/specialization_graph.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_middle/src/traits/specialization_graph.rs b/compiler/rustc_middle/src/traits/specialization_graph.rs index c2ec12c0a75b3..ce09c7234796d 100644 --- a/compiler/rustc_middle/src/traits/specialization_graph.rs +++ b/compiler/rustc_middle/src/traits/specialization_graph.rs @@ -55,14 +55,14 @@ pub struct Children { // Impls of a trait (or specializations of a given impl). To allow for // quicker lookup, the impls are indexed by a simplified version of their // `Self` type: impls with a simplifiable `Self` are stored in - // `nonblanket_impls` keyed by it, while all other impls are stored in + // `non_blanket_impls` keyed by it, while all other impls are stored in // `blanket_impls`. // // A similar division is used within `TraitDef`, but the lists there collect // together *all* the impls for a trait, and are populated prior to building // the specialization graph. /// Impls of the trait. - pub nonblanket_impls: BTreeMap>, + pub non_blanket_impls: BTreeMap>, /// Blanket impls associated with the trait. pub blanket_impls: Vec, @@ -238,8 +238,8 @@ pub fn ancestors( impl<'a> HashStable> for Children { fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - let Children { ref nonblanket_impls, ref blanket_impls } = *self; + let Children { ref non_blanket_impls, ref blanket_impls } = *self; - ich::hash_stable_trait_impls(hcx, hasher, blanket_impls, nonblanket_impls); + ich::hash_stable_trait_impls(hcx, hasher, blanket_impls, non_blanket_impls); } } diff --git a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs b/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs index 1684c35702ace..baf8ff89ed69e 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs @@ -50,7 +50,7 @@ impl ChildrenExt for Children { let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false) { debug!("insert_blindly: impl_def_id={:?} st={:?}", impl_def_id, st); - self.nonblanket_impls.entry(st.to_stable(tcx)).or_default().push(impl_def_id) + self.non_blanket_impls.entry(st.to_stable(tcx)).or_default().push(impl_def_id) } else { debug!("insert_blindly: impl_def_id={:?} st=None", impl_def_id); self.blanket_impls.push(impl_def_id) @@ -65,7 +65,7 @@ impl ChildrenExt for Children { let vec: &mut Vec; if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false) { debug!("remove_existing: impl_def_id={:?} st={:?}", impl_def_id, st); - vec = self.nonblanket_impls.get_mut(&st.to_stable(tcx)).unwrap(); + vec = self.non_blanket_impls.get_mut(&st.to_stable(tcx)).unwrap(); } else { debug!("remove_existing: impl_def_id={:?} st=None", impl_def_id); vec = &mut self.blanket_impls; @@ -216,7 +216,7 @@ impl ChildrenExt for Children { } fn iter_children(children: &mut Children) -> impl Iterator + '_ { - let nonblanket = children.nonblanket_impls.iter_mut().flat_map(|(_, v)| v.iter()); + let nonblanket = children.non_blanket_impls.iter_mut().flat_map(|(_, v)| v.iter()); children.blanket_impls.iter().chain(nonblanket).cloned() } @@ -224,7 +224,7 @@ fn filtered_children( children: &mut Children, st: StableSimplifiedType, ) -> impl Iterator + '_ { - let nonblanket = children.nonblanket_impls.entry(st).or_default().iter(); + let nonblanket = children.non_blanket_impls.entry(st).or_default().iter(); children.blanket_impls.iter().chain(nonblanket).cloned() }