Rollup merge of #68966 - jonas-schievink:coherence-perf, r=varkor

Improve performance of coherence checks

The biggest perf improvement in here is expected to come from the removal of the remaining #43355 warning code since that effectively runs the expensive parts of coherence *twice*, which can even be visualized by obtaining a flamegraph:

![image](https://user-images.githubusercontent.com/5047365/74091959-e1f41200-4a8b-11ea-969d-2849d3f86c63.png)

Author: Dylan-DPC

src/librustc/query/mod.rs

@@ -647,7 +647,8 @@ rustc_queries! {
         query trait_impls_of(key: DefId) -> &'tcx ty::trait_def::TraitImpls {
             desc { |tcx| "trait impls of `{}`", tcx.def_path_str(key) }
         }
-        query specialization_graph_of(_: DefId) -> &'tcx specialization_graph::Graph {
+        query specialization_graph_of(key: DefId) -> &'tcx specialization_graph::Graph {
+            desc { |tcx| "building specialization graph of trait `{}`", tcx.def_path_str(key) }
             cache_on_disk_if { true }
         }
         query is_object_safe(key: DefId) -> bool {

src/librustc/traits/coherence.rs

@@ -6,7 +6,6 @@
 
 use crate::infer::{CombinedSnapshot, InferOk};
 use crate::traits::select::IntercrateAmbiguityCause;
-use crate::traits::IntercrateMode;
 use crate::traits::SkipLeakCheck;
 use crate::traits::{self, Normalized, Obligation, ObligationCause, SelectionContext};
 use crate::ty::fold::TypeFoldable;
@@ -27,7 +26,7 @@ enum InCrate {
 #[derive(Debug, Copy, Clone)]
 pub enum Conflict {
     Upstream,
-    Downstream { used_to_be_broken: bool },
+    Downstream,
 }
 
 pub struct OverlapResult<'tcx> {
@@ -53,7 +52,6 @@ pub fn overlapping_impls<F1, F2, R>(
     tcx: TyCtxt<'_>,
     impl1_def_id: DefId,
     impl2_def_id: DefId,
-    intercrate_mode: IntercrateMode,
     skip_leak_check: SkipLeakCheck,
     on_overlap: F1,
     no_overlap: F2,
@@ -65,13 +63,12 @@ where
     debug!(
         "overlapping_impls(\
            impl1_def_id={:?}, \
-           impl2_def_id={:?},
-           intercrate_mode={:?})",
-        impl1_def_id, impl2_def_id, intercrate_mode
+           impl2_def_id={:?})",
+        impl1_def_id, impl2_def_id,
     );
 
     let overlaps = tcx.infer_ctxt().enter(|infcx| {
-        let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
+        let selcx = &mut SelectionContext::intercrate(&infcx);
         overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some()
     });
 
@@ -83,7 +80,7 @@ where
     // this time tracking intercrate ambuiguity causes for better
     // diagnostics. (These take time and can lead to false errors.)
     tcx.infer_ctxt().enter(|infcx| {
-        let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
+        let selcx = &mut SelectionContext::intercrate(&infcx);
         selcx.enable_tracking_intercrate_ambiguity_causes();
         on_overlap(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).unwrap())
     })
@@ -202,15 +199,7 @@ pub fn trait_ref_is_knowable<'tcx>(
     if orphan_check_trait_ref(tcx, trait_ref, InCrate::Remote).is_ok() {
         // A downstream or cousin crate is allowed to implement some
         // substitution of this trait-ref.
-
-        // A trait can be implementable for a trait ref by both the current
-        // crate and crates downstream of it. Older versions of rustc
-        // were not aware of this, causing incoherence (issue #43355).
-        let used_to_be_broken = orphan_check_trait_ref(tcx, trait_ref, InCrate::Local).is_ok();
-        if used_to_be_broken {
-            debug!("trait_ref_is_knowable({:?}) - USED TO BE BROKEN", trait_ref);
-        }
-        return Some(Conflict::Downstream { used_to_be_broken });
+        return Some(Conflict::Downstream);
     }
 
     if trait_ref_is_local_or_fundamental(tcx, trait_ref) {

src/librustc/traits/mod.rs

@@ -78,13 +78,6 @@ pub use self::chalk_fulfill::{
 
 pub use self::types::*;
 
-/// Whether to enable bug compatibility with issue #43355.
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
-pub enum IntercrateMode {
-    Issue43355,
-    Fixed,
-}
-
 /// Whether to skip the leak check, as part of a future compatibility warning step.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum SkipLeakCheck {

src/librustc/traits/select.rs

@@ -19,8 +19,8 @@ use super::DerivedObligationCause;
 use super::Selection;
 use super::SelectionResult;
 use super::TraitNotObjectSafe;
+use super::TraitQueryMode;
 use super::{BuiltinDerivedObligation, ImplDerivedObligation, ObligationCauseCode};
-use super::{IntercrateMode, TraitQueryMode};
 use super::{ObjectCastObligation, Obligation};
 use super::{ObligationCause, PredicateObligation, TraitObligation};
 use super::{OutputTypeParameterMismatch, Overflow, SelectionError, Unimplemented};
@@ -80,7 +80,7 @@ pub struct SelectionContext<'cx, 'tcx> {
     /// other words, we consider `$0: Bar` to be unimplemented if
     /// there is no type that the user could *actually name* that
     /// would satisfy it. This avoids crippling inference, basically.
-    intercrate: Option<IntercrateMode>,
+    intercrate: bool,
 
     intercrate_ambiguity_causes: Option<Vec<IntercrateAmbiguityCause>>,
 
@@ -218,22 +218,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: None,
+            intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode: TraitQueryMode::Standard,
         }
     }
 
-    pub fn intercrate(
-        infcx: &'cx InferCtxt<'cx, 'tcx>,
-        mode: IntercrateMode,
-    ) -> SelectionContext<'cx, 'tcx> {
-        debug!("intercrate({:?})", mode);
+    pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'tcx>) -> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: Some(mode),
+            intercrate: true,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode: TraitQueryMode::Standard,
@@ -248,7 +244,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: None,
+            intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls,
             query_mode: TraitQueryMode::Standard,
@@ -263,7 +259,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: None,
+            intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode,
@@ -276,7 +272,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// false overflow results (#47139) and because it costs
     /// computation time.
     pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) {
-        assert!(self.intercrate.is_some());
+        assert!(self.intercrate);
         assert!(self.intercrate_ambiguity_causes.is_none());
         self.intercrate_ambiguity_causes = Some(vec![]);
         debug!("selcx: enable_tracking_intercrate_ambiguity_causes");
@@ -286,7 +282,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// was enabled and disables tracking at the same time. If
     /// tracking is not enabled, just returns an empty vector.
     pub fn take_intercrate_ambiguity_causes(&mut self) -> Vec<IntercrateAmbiguityCause> {
-        assert!(self.intercrate.is_some());
+        assert!(self.intercrate);
         self.intercrate_ambiguity_causes.take().unwrap_or(vec![])
     }
 
@@ -562,7 +558,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     ) -> Result<EvaluationResult, OverflowError> {
         debug!("evaluate_trait_predicate_recursively({:?})", obligation);
 
-        if self.intercrate.is_none()
+        if !self.intercrate
             && obligation.is_global()
             && obligation.param_env.caller_bounds.iter().all(|bound| bound.needs_subst())
         {
@@ -727,7 +723,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             stack.fresh_trait_ref.skip_binder().input_types().any(|ty| ty.is_fresh());
         // This check was an imperfect workaround for a bug in the old
         // intercrate mode; it should be removed when that goes away.
-        if unbound_input_types && self.intercrate == Some(IntercrateMode::Issue43355) {
+        if unbound_input_types && self.intercrate {
             debug!(
                 "evaluate_stack({:?}) --> unbound argument, intercrate -->  ambiguous",
                 stack.fresh_trait_ref
@@ -1206,7 +1202,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn is_knowable<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) -> Option<Conflict> {
         debug!("is_knowable(intercrate={:?})", self.intercrate);
 
-        if !self.intercrate.is_some() {
+        if !self.intercrate {
             return None;
         }
 
@@ -1218,17 +1214,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // bound regions.
         let trait_ref = predicate.skip_binder().trait_ref;
 
-        let result = coherence::trait_ref_is_knowable(self.tcx(), trait_ref);
-        if let (
-            Some(Conflict::Downstream { used_to_be_broken: true }),
-            Some(IntercrateMode::Issue43355),
-        ) = (result, self.intercrate)
-        {
-            debug!("is_knowable: IGNORING conflict to be bug-compatible with #43355");
-            None
-        } else {
-            result
-        }
+        coherence::trait_ref_is_knowable(self.tcx(), trait_ref)
     }
 
     /// Returns `true` if the global caches can be used.
@@ -1249,7 +1235,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // the master cache. Since coherence executes pretty quickly,
         // it's not worth going to more trouble to increase the
         // hit-rate, I don't think.
-        if self.intercrate.is_some() {
+        if self.intercrate {
             return false;
         }
 
@@ -3305,7 +3291,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return Err(());
         }
 
-        if self.intercrate.is_none()
+        if !self.intercrate
             && self.tcx().impl_polarity(impl_def_id) == ty::ImplPolarity::Reservation
         {
             debug!("match_impl: reservation impls only apply in intercrate mode");

src/librustc/traits/specialize/mod.rs

@@ -332,14 +332,9 @@ pub(super) fn specialization_graph_provider(
                 let impl_span =
                     tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap());
                 let mut err = match used_to_be_allowed {
-                    Some(FutureCompatOverlapErrorKind::Issue43355) | None => {
-                        struct_span_err!(tcx.sess, impl_span, E0119, "{}", msg)
-                    }
+                    None => struct_span_err!(tcx.sess, impl_span, E0119, "{}", msg),
                     Some(kind) => {
                         let lint = match kind {
-                            FutureCompatOverlapErrorKind::Issue43355 => {
-                                unreachable!("converted to hard error above")
-                            }
                             FutureCompatOverlapErrorKind::Issue33140 => {
                                 ORDER_DEPENDENT_TRAIT_OBJECTS
                             }

src/librustc/traits/specialize/specialization_graph.rs

@@ -9,7 +9,6 @@ pub use rustc::traits::types::specialization_graph::*;
 
 #[derive(Copy, Clone, Debug)]
 pub enum FutureCompatOverlapErrorKind {
-    Issue43355,
     Issue33140,
     LeakCheck,
 }
@@ -107,16 +106,16 @@ impl<'tcx> Children {
                 }
             };
 
+            let allowed_to_overlap =
+                tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling);
+
             let (le, ge) = traits::overlapping_impls(
                 tcx,
                 possible_sibling,
                 impl_def_id,
-                traits::IntercrateMode::Issue43355,
                 traits::SkipLeakCheck::default(),
                 |overlap| {
-                    if let Some(overlap_kind) =
-                        tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
-                    {
+                    if let Some(overlap_kind) = &allowed_to_overlap {
                         match overlap_kind {
                             ty::ImplOverlapKind::Permitted { marker: _ } => {}
                             ty::ImplOverlapKind::Issue33140 => {
@@ -154,31 +153,14 @@ impl<'tcx> Children {
 
                 replace_children.push(possible_sibling);
             } else {
-                if let None = tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
-                    // do future-compat checks for overlap. Have issue #33140
-                    // errors overwrite issue #43355 errors when both are present.
-
-                    traits::overlapping_impls(
-                        tcx,
-                        possible_sibling,
-                        impl_def_id,
-                        traits::IntercrateMode::Fixed,
-                        traits::SkipLeakCheck::default(),
-                        |overlap| {
-                            last_lint = Some(FutureCompatOverlapError {
-                                error: overlap_error(overlap),
-                                kind: FutureCompatOverlapErrorKind::Issue43355,
-                            });
-                        },
-                        || (),
-                    );
+                if let None = allowed_to_overlap {
+                    // Do future-compat checks for overlap.
 
                     if last_lint.is_none() {
                         traits::overlapping_impls(
                             tcx,
                             possible_sibling,
                             impl_def_id,
-                            traits::IntercrateMode::Fixed,
                             traits::SkipLeakCheck::Yes,
                             |overlap| {
                                 last_lint = Some(FutureCompatOverlapError {

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> {

src/librustc_typeck/coherence/inherent_impls_overlap.rs

@@ -1,5 +1,5 @@
 use crate::namespace::Namespace;
-use rustc::traits::{self, IntercrateMode, SkipLeakCheck};
+use rustc::traits::{self, SkipLeakCheck};
 use rustc::ty::{AssocItem, TyCtxt};
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
@@ -93,7 +93,6 @@ impl InherentOverlapChecker<'tcx> {
             self.tcx,
             impl1_def_id,
             impl2_def_id,
-            IntercrateMode::Issue43355,
             // We go ahead and just skip the leak check for
             // inherent impls without warning.
             SkipLeakCheck::Yes,