Skip to content

fix non_blanket_impls iteration order #88718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions compiler/rustc_middle/src/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Symbol> {
debug_assert!(!ich::IGNORED_ATTRIBUTES.is_empty());
Expand Down Expand Up @@ -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<fast_reject::SimplifiedType, Vec<DefId>>,
non_blanket_impls: &BTreeMap<fast_reject::StableSimplifiedType, Vec<DefId>>,
) {
{
let mut blanket_impls: SmallVec<[_; 8]> =
Expand All @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_middle/src/traits/specialization_graph.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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: FxHashMap<SimplifiedType, Vec<DefId>>,
pub non_blanket_impls: BTreeMap<StableSimplifiedType, Vec<DefId>>,

/// Blanket impls associated with the trait.
pub blanket_impls: Vec<DefId>,
Expand Down Expand Up @@ -238,8 +238,8 @@ pub fn ancestors(

impl<'a> HashStable<StableHashingContext<'a>> 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);
}
}
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
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;

use self::SimplifiedTypeGen::*;

pub type SimplifiedType = SimplifiedTypeGen<DefId>;
pub type StableSimplifiedType = SimplifiedTypeGen<DefPathHash>;

/// See `simplify_type`
///
Expand Down Expand Up @@ -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<D: Copy + Debug + Ord + Eq> SimplifiedTypeGen<D> {
pub fn map_def<U, F>(self, map: F) -> SimplifiedTypeGen<U>
where
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -70,7 +69,7 @@ pub enum TraitSpecializationKind {
pub struct TraitImpls {
blanket_impls: Vec<DefId>,
/// Impls indexed by their simplified self type, for fast lookup.
non_blanket_impls: FxHashMap<fast_reject::SimplifiedType, Vec<DefId>>,
non_blanket_impls: BTreeMap<fast_reject::StableSimplifiedType, Vec<DefId>>,
}

impl TraitImpls {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -40,7 +40,7 @@ trait ChildrenExt {
&mut self,
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
simplified_self: Option<StableSimplifiedType>,
) -> Result<Inserted, OverlapError>;
}

Expand All @@ -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.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)
Expand All @@ -65,7 +65,7 @@ impl ChildrenExt for Children {
let vec: &mut Vec<DefId>;
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.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;
Expand All @@ -81,7 +81,7 @@ impl ChildrenExt for Children {
&mut self,
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
simplified_self: Option<StableSimplifiedType>,
) -> Result<Inserted, OverlapError> {
let mut last_lint = None;
let mut replace_children = Vec::new();
Expand Down Expand Up @@ -216,15 +216,15 @@ impl ChildrenExt for Children {
}

fn iter_children(children: &mut Children) -> impl Iterator<Item = DefId> + '_ {
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()
}

fn filtered_children(
children: &mut Children,
st: SimplifiedType,
st: StableSimplifiedType,
) -> impl Iterator<Item = DefId> + '_ {
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()
}

Expand Down Expand Up @@ -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 {
Expand Down