Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Reduce queries/map lookups done by coherence
This has negligible perf impact, but it does improve the code a bit.

* Only query the specialization graph of any trait once instead of once per
  impl
* Loop over impls only once, precomputing impl DefId and TraitRef
  • Loading branch information
jonas-schievink committed Feb 9, 2020
1 parent 71c7e14 commit 5ab1ab4
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 46 deletions.
12 changes: 5 additions & 7 deletions src/librustc_typeck/coherence/builtin.rs
Expand Up @@ -18,14 +18,12 @@ use rustc_hir::def_id::DefId;
use rustc_hir::ItemKind;

pub fn check_trait(tcx: TyCtxt<'_>, trait_def_id: DefId) {
let lang_items = tcx.lang_items();
Checker { tcx, trait_def_id }
.check(tcx.lang_items().drop_trait(), visit_implementation_of_drop)
.check(tcx.lang_items().copy_trait(), visit_implementation_of_copy)
.check(tcx.lang_items().coerce_unsized_trait(), visit_implementation_of_coerce_unsized)
.check(
tcx.lang_items().dispatch_from_dyn_trait(),
visit_implementation_of_dispatch_from_dyn,
);
.check(lang_items.drop_trait(), visit_implementation_of_drop)
.check(lang_items.copy_trait(), visit_implementation_of_copy)
.check(lang_items.coerce_unsized_trait(), visit_implementation_of_coerce_unsized)
.check(lang_items.dispatch_from_dyn_trait(), visit_implementation_of_dispatch_from_dyn);
}

struct Checker<'tcx> {
Expand Down
67 changes: 30 additions & 37 deletions src/librustc_typeck/coherence/mod.rs
Expand Up @@ -10,36 +10,28 @@ use rustc::ty::query::Providers;
use rustc::ty::{self, TyCtxt, TypeFoldable};
use rustc_errors::struct_span_err;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::HirId;

mod builtin;
mod inherent_impls;
mod inherent_impls_overlap;
mod orphan;
mod unsafety;

fn check_impl(tcx: TyCtxt<'_>, hir_id: HirId) {
let impl_def_id = tcx.hir().local_def_id(hir_id);
fn check_impl(tcx: TyCtxt<'_>, impl_def_id: DefId, trait_ref: ty::TraitRef<'_>) {
debug!(
"(checking implementation) adding impl for trait '{:?}', item '{}'",
trait_ref,
tcx.def_path_str(impl_def_id)
);

// If there are no traits, then this implementation must have a
// base type.

if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id) {
debug!(
"(checking implementation) adding impl for trait '{:?}', item '{}'",
trait_ref,
tcx.def_path_str(impl_def_id)
);

// Skip impls where one of the self type is an error type.
// This occurs with e.g., resolve failures (#30589).
if trait_ref.references_error() {
return;
}

enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id);
enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id);
// Skip impls where one of the self type is an error type.
// This occurs with e.g., resolve failures (#30589).
if trait_ref.references_error() {
return;
}

enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id);
enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id);
}

fn enforce_trait_manually_implementable(tcx: TyCtxt<'_>, impl_def_id: DefId, trait_def_id: DefId) {
Expand Down Expand Up @@ -129,12 +121,17 @@ pub fn provide(providers: &mut Providers<'_>) {
}

fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) {
// Trigger building the specialization graph for the trait. This will detect and report any
// overlap errors.
tcx.specialization_graph_of(def_id);

let impls = tcx.hir().trait_impls(def_id);
for &impl_id in impls {
check_impl(tcx, impl_id);
}
for &impl_id in impls {
check_impl_overlap(tcx, impl_id);
for &hir_id in impls {
let impl_def_id = tcx.hir().local_def_id(hir_id);
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();

check_impl(tcx, impl_def_id, trait_ref);
check_object_overlap(tcx, impl_def_id, trait_ref);
}
builtin::check_trait(tcx, def_id);
}
Expand All @@ -152,24 +149,20 @@ pub fn check_coherence(tcx: TyCtxt<'_>) {
tcx.ensure().crate_inherent_impls_overlap_check(LOCAL_CRATE);
}

/// Overlap: no two impls for the same trait are implemented for the
/// same type. Likewise, no two inherent impls for a given type
/// constructor provide a method with the same name.
fn check_impl_overlap<'tcx>(tcx: TyCtxt<'tcx>, hir_id: HirId) {
let impl_def_id = tcx.hir().local_def_id(hir_id);
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
/// Checks whether an impl overlaps with the automatic `impl Trait for dyn Trait`.
fn check_object_overlap<'tcx>(
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
trait_ref: ty::TraitRef<'tcx>,
) {
let trait_def_id = trait_ref.def_id;

if trait_ref.references_error() {
debug!("coherence: skipping impl {:?} with error {:?}", impl_def_id, trait_ref);
return;
}

// Trigger building the specialization graph for the trait of this impl.
// This will detect any overlap errors.
tcx.specialization_graph_of(trait_def_id);

// check for overlap with the automatic `impl Trait for Trait`
// check for overlap with the automatic `impl Trait for dyn Trait`
if let ty::Dynamic(ref data, ..) = trait_ref.self_ty().kind {
// This is something like impl Trait1 for Trait2. Illegal
// if Trait1 is a supertrait of Trait2 or Trait2 is not object safe.
Expand Down
@@ -1,10 +1,10 @@
error[E0391]: cycle detected when processing `Trait`
error[E0391]: cycle detected when building specialization graph of trait `Trait`
--> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:8:1
|
LL | trait Trait<T> { type Assoc; }
| ^^^^^^^^^^^^^^
|
= note: ...which again requires processing `Trait`, completing the cycle
= note: ...which again requires building specialization graph of trait `Trait`, completing the cycle
note: cycle used when coherence checking all impls of trait `Trait`
--> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:8:1
|
Expand Down

0 comments on commit 5ab1ab4

Please sign in to comment.