From 5ab1ab4959c3a5bb55123a7f48934684fd62bc51 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 8 Feb 2020 00:27:07 +0100 Subject: [PATCH] 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 --- src/librustc_typeck/coherence/builtin.rs | 12 ++-- src/librustc_typeck/coherence/mod.rs | 67 +++++++++---------- ...erence-inherited-assoc-ty-cycle-err.stderr | 4 +- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 79a006a898a89..1970b1e5c5deb 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -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> { diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 5583e3418b2a8..012cdb7b8aed7 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -10,7 +10,6 @@ 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; @@ -18,28 +17,21 @@ 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) { @@ -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); } @@ -152,12 +149,12 @@ 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() { @@ -165,11 +162,7 @@ fn check_impl_overlap<'tcx>(tcx: TyCtxt<'tcx>, hir_id: HirId) { 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. diff --git a/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr b/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr index e5cc298a4355d..71f997c54c6f2 100644 --- a/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr +++ b/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr @@ -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 { 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 |