Skip to content

make for_all_relevant_impls O(1) again #43723

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

Merged
merged 1 commit into from
Aug 8, 2017
Merged
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
9 changes: 4 additions & 5 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
let mut self_match_impls = vec![];
let mut fuzzy_match_impls = vec![];

self.tcx.trait_def(trait_ref.def_id)
.for_each_relevant_impl(self.tcx, trait_self_ty, |def_id| {
self.tcx.for_each_relevant_impl(
trait_ref.def_id, trait_self_ty, |def_id| {
let impl_substs = self.fresh_substs_for_item(obligation.cause.span, def_id);
let impl_trait_ref = tcx
.impl_trait_ref(def_id)
Expand Down Expand Up @@ -396,10 +396,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
trait_ref.skip_binder().self_ty(),
true);
let mut impl_candidates = Vec::new();
let trait_def = self.tcx.trait_def(trait_ref.def_id());

match simp {
Some(simp) => trait_def.for_each_impl(self.tcx, |def_id| {
Some(simp) => self.tcx.for_each_impl(trait_ref.def_id(), |def_id| {
let imp = self.tcx.impl_trait_ref(def_id).unwrap();
let imp_simp = fast_reject::simplify_type(self.tcx,
imp.self_ty(),
Expand All @@ -411,7 +410,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
impl_candidates.push(imp);
}),
None => trait_def.for_each_impl(self.tcx, |def_id| {
None => self.tcx.for_each_impl(trait_ref.def_id(), |def_id| {
impl_candidates.push(
self.tcx.impl_trait_ref(def_id).unwrap());
})
Expand Down
6 changes: 2 additions & 4 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1576,10 +1576,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
{
debug!("assemble_candidates_from_impls(obligation={:?})", obligation);

let def = self.tcx().trait_def(obligation.predicate.def_id());

def.for_each_relevant_impl(
self.tcx(),
self.tcx().for_each_relevant_impl(
obligation.predicate.def_id(),
obligation.predicate.0.trait_ref.self_ty(),
|impl_def_id| {
self.probe(|this, snapshot| { /* [1] */
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
-> Rc<specialization_graph::Graph> {
let mut sg = specialization_graph::Graph::new();

let mut trait_impls: Vec<DefId> = tcx.trait_impls_of(trait_id).iter().collect();
let mut trait_impls = Vec::new();
tcx.for_each_impl(trait_id, |impl_did| trait_impls.push(impl_did));

// The coherence checking implementation seems to rely on impls being
// iterated over (roughly) in definition order, so we are sorting by
Expand Down
15 changes: 1 addition & 14 deletions src/librustc/ty/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,12 +470,6 @@ impl<'tcx> QueryDescription for queries::trait_impls_of<'tcx> {
}
}

impl<'tcx> QueryDescription for queries::relevant_trait_impls_for<'tcx> {
fn describe(tcx: TyCtxt, (def_id, ty): (DefId, SimplifiedType)) -> String {
format!("relevant impls for: `({}, {:?})`", tcx.item_path_str(def_id), ty)
}
}

impl<'tcx> QueryDescription for queries::is_object_safe<'tcx> {
fn describe(tcx: TyCtxt, def_id: DefId) -> String {
format!("determine object safety of trait `{}`", tcx.item_path_str(def_id))
Expand Down Expand Up @@ -966,10 +960,7 @@ define_maps! { <'tcx>
[] const_is_rvalue_promotable_to_static: ConstIsRvaluePromotableToStatic(DefId) -> bool,
[] is_mir_available: IsMirAvailable(DefId) -> bool,

[] trait_impls_of: TraitImpls(DefId) -> ty::trait_def::TraitImpls,
// Note that TraitDef::for_each_relevant_impl() will do type simplication for you.
[] relevant_trait_impls_for: relevant_trait_impls_for((DefId, SimplifiedType))
-> ty::trait_def::TraitImpls,
[] trait_impls_of: TraitImpls(DefId) -> Rc<ty::trait_def::TraitImpls>,
[] specialization_graph_of: SpecializationGraph(DefId) -> Rc<specialization_graph::Graph>,
[] is_object_safe: ObjectSafety(DefId) -> bool,

Expand Down Expand Up @@ -1049,10 +1040,6 @@ fn crate_variances<'tcx>(_: CrateNum) -> DepConstructor<'tcx> {
DepConstructor::CrateVariances
}

fn relevant_trait_impls_for<'tcx>((def_id, t): (DefId, SimplifiedType)) -> DepConstructor<'tcx> {
DepConstructor::RelevantTraitImpls(def_id, t)
}

fn is_copy_dep_node<'tcx>(_: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> DepConstructor<'tcx> {
DepConstructor::IsCopy
}
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2507,7 +2507,6 @@ pub fn provide(providers: &mut ty::maps::Providers) {
param_env,
trait_of_item,
trait_impls_of: trait_def::trait_impls_of_provider,
relevant_trait_impls_for: trait_def::relevant_trait_impls_provider,
..*providers
};
}
Expand All @@ -2517,7 +2516,6 @@ pub fn provide_extern(providers: &mut ty::maps::Providers) {
adt_sized_constraint,
adt_dtorck_constraint,
trait_impls_of: trait_def::trait_impls_of_provider,
relevant_trait_impls_for: trait_def::relevant_trait_impls_provider,
param_env,
..*providers
};
Expand Down
168 changes: 64 additions & 104 deletions src/librustc/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use hir;
use hir::def_id::DefId;
use hir::map::DefPathHash;
use traits::specialization_graph;
use ty::fast_reject;
use ty::fold::TypeFoldable;
use ty::{Ty, TyCtxt};

use rustc_data_structures::fx::FxHashMap;

use std::rc::Rc;
use hir;

/// A trait's definition with type information.
pub struct TraitDef {
Expand All @@ -36,60 +39,12 @@ pub struct TraitDef {
pub def_path_hash: DefPathHash,
}

// We don't store the list of impls in a flat list because each cached list of
// `relevant_impls_for` we would then duplicate all blanket impls. By keeping
// blanket and non-blanket impls separate, we can share the list of blanket
// impls.
#[derive(Clone)]
pub struct TraitImpls {
blanket_impls: Rc<Vec<DefId>>,
non_blanket_impls: Rc<Vec<DefId>>,
blanket_impls: Vec<DefId>,
/// Impls indexed by their simplified self-type, for fast lookup.
non_blanket_impls: FxHashMap<fast_reject::SimplifiedType, Vec<DefId>>,
}

impl TraitImpls {
pub fn iter(&self) -> TraitImplsIter {
TraitImplsIter {
blanket_impls: self.blanket_impls.clone(),
non_blanket_impls: self.non_blanket_impls.clone(),
index: 0
}
}
}

#[derive(Clone)]
pub struct TraitImplsIter {
blanket_impls: Rc<Vec<DefId>>,
non_blanket_impls: Rc<Vec<DefId>>,
index: usize,
}

impl Iterator for TraitImplsIter {
type Item = DefId;

fn next(&mut self) -> Option<DefId> {
if self.index < self.blanket_impls.len() {
let bi_index = self.index;
self.index += 1;
Some(self.blanket_impls[bi_index])
} else {
let nbi_index = self.index - self.blanket_impls.len();
if nbi_index < self.non_blanket_impls.len() {
self.index += 1;
Some(self.non_blanket_impls[nbi_index])
} else {
None
}
}
}

fn size_hint(&self) -> (usize, Option<usize>) {
let items_left = (self.blanket_impls.len() + self.non_blanket_impls.len()) - self.index;
(items_left, Some(items_left))
}
}

impl ExactSizeIterator for TraitImplsIter {}

impl<'a, 'gcx, 'tcx> TraitDef {
pub fn new(def_id: DefId,
unsafety: hir::Unsafety,
Expand All @@ -111,20 +66,36 @@ impl<'a, 'gcx, 'tcx> TraitDef {
-> specialization_graph::Ancestors {
specialization_graph::ancestors(tcx, self.def_id, of_impl)
}
}

pub fn for_each_impl<F: FnMut(DefId)>(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, mut f: F) {
for impl_def_id in tcx.trait_impls_of(self.def_id).iter() {
impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn for_each_impl<F: FnMut(DefId)>(self, def_id: DefId, mut f: F) {
let impls = self.trait_impls_of(def_id);

for &impl_def_id in impls.blanket_impls.iter() {
f(impl_def_id);
}

for v in impls.non_blanket_impls.values() {
for &impl_def_id in v {
f(impl_def_id);
}
}
}

/// Iterate over every impl that could possibly match the
/// self-type `self_ty`.
pub fn for_each_relevant_impl<F: FnMut(DefId)>(&self,
tcx: TyCtxt<'a, 'gcx, 'tcx>,
pub fn for_each_relevant_impl<F: FnMut(DefId)>(self,
def_id: DefId,
self_ty: Ty<'tcx>,
mut f: F)
{
let impls = self.trait_impls_of(def_id);

for &impl_def_id in impls.blanket_impls.iter() {
f(impl_def_id);
}

// simplify_type(.., false) basically replaces type parameters and
// projections with infer-variables. This is, of course, done on
// the impl trait-ref when it is instantiated, but not on the
Expand All @@ -137,23 +108,39 @@ impl<'a, 'gcx, 'tcx> TraitDef {
// replace `S` with anything - this impl of course can't be
// selected, and as there are hundreds of similar impls,
// considering them would significantly harm performance.
let relevant_impls = if let Some(simplified_self_ty) =
fast_reject::simplify_type(tcx, self_ty, true) {
tcx.relevant_trait_impls_for((self.def_id, simplified_self_ty))
} else {
tcx.trait_impls_of(self.def_id)
};

for impl_def_id in relevant_impls.iter() {
f(impl_def_id);
// This depends on the set of all impls for the trait. That is
// unfortunate. When we get red-green recompilation, we would like
// to have a way of knowing whether the set of relevant impls
// changed. The most naive
// way would be to compute the Vec of relevant impls and see whether
// it differs between compilations. That shouldn't be too slow by
// itself - we do quite a bit of work for each relevant impl anyway.
//
// If we want to be faster, we could have separate queries for
// blanket and non-blanket impls, and compare them separately.
//
// 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) {
for &impl_def_id in impls {
f(impl_def_id);
}
}
} else {
for v in impls.non_blanket_impls.values() {
for &impl_def_id in v {
f(impl_def_id);
}
}
}
}
}

// Query provider for `trait_impls_of`.
pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
trait_id: DefId)
-> TraitImpls {
-> Rc<TraitImpls> {
let remote_impls = if trait_id.is_local() {
// Traits defined in the current crate can't have impls in upstream
// crates, so we don't bother querying the cstore.
Expand All @@ -163,7 +150,7 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
};

let mut blanket_impls = Vec::new();
let mut non_blanket_impls = Vec::new();
let mut non_blanket_impls = FxHashMap();

let local_impls = tcx.hir
.trait_impls(trait_id)
Expand All @@ -176,47 +163,20 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
continue
}

if fast_reject::simplify_type(tcx, impl_self_ty, false).is_some() {
non_blanket_impls.push(impl_def_id);
if let Some(simplified_self_ty) =
fast_reject::simplify_type(tcx, impl_self_ty, false)
{
non_blanket_impls
.entry(simplified_self_ty)
.or_insert(vec![])
.push(impl_def_id);
} else {
blanket_impls.push(impl_def_id);
}
}

TraitImpls {
blanket_impls: Rc::new(blanket_impls),
non_blanket_impls: Rc::new(non_blanket_impls),
}
}

// Query provider for `relevant_trait_impls_for`.
pub(super) fn relevant_trait_impls_provider<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
(trait_id, self_ty): (DefId, fast_reject::SimplifiedType))
-> TraitImpls
{
let all_trait_impls = tcx.trait_impls_of(trait_id);

let relevant: Vec<DefId> = all_trait_impls
.non_blanket_impls
.iter()
.cloned()
.filter(|&impl_def_id| {
let impl_self_ty = tcx.type_of(impl_def_id);
let impl_simple_self_ty = fast_reject::simplify_type(tcx,
impl_self_ty,
false).unwrap();
impl_simple_self_ty == self_ty
})
.collect();

if all_trait_impls.non_blanket_impls.len() == relevant.len() {
// If we didn't filter anything out, re-use the existing vec.
all_trait_impls
} else {
TraitImpls {
blanket_impls: all_trait_impls.blanket_impls.clone(),
non_blanket_impls: Rc::new(relevant),
}
}
Rc::new(TraitImpls {
blanket_impls: blanket_impls,
non_blanket_impls: non_blanket_impls,
})
}
2 changes: 1 addition & 1 deletion src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {

let mut dtor_did = None;
let ty = self.type_of(adt_did);
self.trait_def(drop_trait).for_each_relevant_impl(self, ty, |impl_did| {
self.for_each_relevant_impl(drop_trait, ty, |impl_did| {
if let Some(item) = self.associated_items(impl_did).next() {
if let Ok(()) = validate(self, impl_did) {
dtor_did = Some(item.def_id);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDebugImplementations {
};

if self.impling_types.is_none() {
let debug_def = cx.tcx.trait_def(debug);
let mut impls = NodeSet();
debug_def.for_each_impl(cx.tcx, |d| {
cx.tcx.for_each_impl(debug, |d| {
if let Some(ty_def) = cx.tcx.type_of(d).ty_to_def_id() {
if let Some(node_id) = cx.tcx.hir.as_local_node_id(ty_def) {
impls.insert(node_id);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
let mut span = None;

self.tcx
.trait_def(drop_trait_id)
.for_each_relevant_impl(self.tcx, self.mir.return_ty, |impl_did| {
.for_each_relevant_impl(drop_trait_id, self.mir.return_ty, |impl_did| {
self.tcx.hir
.as_local_node_id(impl_did)
.and_then(|impl_node_id| self.tcx.hir.find(impl_node_id))
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_typeck/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,10 +736,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
import_id: Option<ast::NodeId>,
trait_def_id: DefId,
item: ty::AssociatedItem) {
let trait_def = self.tcx.trait_def(trait_def_id);

// FIXME(arielb1): can we use for_each_relevant_impl here?
trait_def.for_each_impl(self.tcx, |impl_def_id| {
self.tcx.for_each_impl(trait_def_id, |impl_def_id| {
debug!("assemble_extension_candidates_for_trait_impl: trait_def_id={:?} \
impl_def_id={:?}",
trait_def_id,
Expand Down