Auto merge of rust-lang#93348 - spastorino:fix-perf-overlap-mode2, r=…

…nikomatsakis

 Move overlap_mode into trait level attribute

r? `@nikomatsakis`

Should fix some performance regressions noted on rust-lang#93175

compiler/rustc_feature/src/active.rs

@@ -161,6 +161,9 @@ declare_features! (
     (active, staged_api, "1.0.0", None, None),
     /// Added for testing E0705; perma-unstable.
     (active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)),
+    /// Use for stable + negative coherence and strict coherence depending on trait's
+    /// rustc_strict_coherence value.
+    (active, with_negative_coherence, "1.60.0", None, None),
     // !!!!    !!!!    !!!!    !!!!   !!!!    !!!!    !!!!    !!!!    !!!!    !!!!    !!!!
     // Features are listed in alphabetical order. Tidy will fail if you don't keep it this way.
     // !!!!    !!!!    !!!!    !!!!   !!!!    !!!!    !!!!    !!!!    !!!!    !!!!    !!!!

compiler/rustc_feature/src/builtin_attrs.rs

@@ -697,7 +697,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
     rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),
     rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word), WarnFollowing),
-    rustc_attr!(TEST, rustc_with_negative_coherence, Normal, template!(Word), WarnFollowing),
     rustc_attr!(TEST, rustc_variance, Normal, template!(Word), WarnFollowing),
     rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ..."), WarnFollowing),
     rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing),

compiler/rustc_middle/src/traits/specialization_graph.rs

@@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt};
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::ErrorReported;
 use rustc_hir::def_id::{DefId, DefIdMap};
+use rustc_span::symbol::sym;
 
 /// 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
@@ -45,6 +46,41 @@ impl Graph {
     }
 }
 
+/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
+/// purposes.
+#[derive(Copy, Clone, PartialEq, Eq, Hash, HashStable, Debug, TyEncodable, TyDecodable)]
+pub enum OverlapMode {
+    /// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
+    Stable,
+    /// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
+    WithNegative,
+    /// Just check for negative impls, not for "where clause not implemented": used for testing.
+    Strict,
+}
+
+impl OverlapMode {
+    pub fn get<'tcx>(tcx: TyCtxt<'tcx>, trait_id: DefId) -> OverlapMode {
+        let with_negative_coherence = tcx.features().with_negative_coherence;
+        let strict_coherence = tcx.has_attr(trait_id, sym::rustc_strict_coherence);
+
+        if with_negative_coherence {
+            if strict_coherence { OverlapMode::Strict } else { OverlapMode::WithNegative }
+        } else if strict_coherence {
+            bug!("To use strict_coherence you need to set with_negative_coherence feature flag");
+        } else {
+            OverlapMode::Stable
+        }
+    }
+
+    pub fn use_negative_impl(&self) -> bool {
+        *self == OverlapMode::Strict || *self == OverlapMode::WithNegative
+    }
+
+    pub fn use_implicit_negative(&self) -> bool {
+        *self == OverlapMode::Stable || *self == OverlapMode::WithNegative
+    }
+}
+
 /// Children of a given impl, grouped into blanket/non-blanket varieties as is
 /// done in `TraitDef`.
 #[derive(Default, TyEncodable, TyDecodable, Debug, HashStable)]

compiler/rustc_span/src/symbol.rs

@@ -1206,7 +1206,6 @@ symbols! {
         rustc_trivial_field_reads,
         rustc_unsafe_specialization_marker,
         rustc_variance,
-        rustc_with_negative_coherence,
         rustdoc,
         rustdoc_internals,
         rustfmt,
@@ -1490,6 +1489,7 @@ symbols! {
         width,
         windows,
         windows_subsystem,
+        with_negative_coherence,
         wrapping_add,
         wrapping_mul,
         wrapping_sub,

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -13,8 +13,8 @@ use crate::traits::{
     self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation,
     PredicateObligations, SelectionContext,
 };
-use rustc_ast::Attribute;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_middle::traits::specialization_graph::OverlapMode;
 use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
 use rustc_middle::ty::fold::TypeFoldable;
 use rustc_middle::ty::subst::Subst;
@@ -62,6 +62,7 @@ pub fn overlapping_impls<F1, F2, R>(
     impl1_def_id: DefId,
     impl2_def_id: DefId,
     skip_leak_check: SkipLeakCheck,
+    overlap_mode: OverlapMode,
     on_overlap: F1,
     no_overlap: F2,
 ) -> R
@@ -99,7 +100,7 @@ where
 
     let overlaps = tcx.infer_ctxt().enter(|infcx| {
         let selcx = &mut SelectionContext::intercrate(&infcx);
-        overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some()
+        overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some()
     });
 
     if !overlaps {
@@ -112,7 +113,9 @@ where
     tcx.infer_ctxt().enter(|infcx| {
         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())
+        on_overlap(
+            overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap(),
+        )
     })
 }
 
@@ -138,68 +141,26 @@ fn with_fresh_ty_vars<'cx, 'tcx>(
     header
 }
 
-/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
-/// purposes.
-#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
-enum OverlapMode {
-    /// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
-    Stable,
-    /// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
-    WithNegative,
-    /// Just check for negative impls, not for "where clause not implemented": used for testing.
-    Strict,
-}
-
-impl OverlapMode {
-    fn use_negative_impl(&self) -> bool {
-        *self == OverlapMode::Strict || *self == OverlapMode::WithNegative
-    }
-
-    fn use_implicit_negative(&self) -> bool {
-        *self == OverlapMode::Stable || *self == OverlapMode::WithNegative
-    }
-}
-
-fn overlap_mode<'tcx>(tcx: TyCtxt<'tcx>, impl1_def_id: DefId, impl2_def_id: DefId) -> OverlapMode {
-    // Find the possible coherence mode override opt-in attributes for each `DefId`
-    let find_coherence_attr = |attr: &Attribute| {
-        let name = attr.name_or_empty();
-        match name {
-            sym::rustc_with_negative_coherence | sym::rustc_strict_coherence => Some(name),
-            _ => None,
-        }
-    };
-    let impl1_coherence_mode = tcx.get_attrs(impl1_def_id).iter().find_map(find_coherence_attr);
-    let impl2_coherence_mode = tcx.get_attrs(impl2_def_id).iter().find_map(find_coherence_attr);
-
-    // If there are any (that currently happens in tests), they need to match. Otherwise, the
-    // default 1.0 rules are used.
-    match (impl1_coherence_mode, impl2_coherence_mode) {
-        (None, None) => OverlapMode::Stable,
-        (Some(sym::rustc_with_negative_coherence), Some(sym::rustc_with_negative_coherence)) => {
-            OverlapMode::WithNegative
-        }
-        (Some(sym::rustc_strict_coherence), Some(sym::rustc_strict_coherence)) => {
-            OverlapMode::Strict
-        }
-        (Some(mode), _) | (_, Some(mode)) => {
-            bug!("Use the same coherence mode on both impls: {}", mode)
-        }
-    }
-}
-
 /// Can both impl `a` and impl `b` be satisfied by a common type (including
 /// where-clauses)? If so, returns an `ImplHeader` that unifies the two impls.
 fn overlap<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     skip_leak_check: SkipLeakCheck,
     impl1_def_id: DefId,
     impl2_def_id: DefId,
+    overlap_mode: OverlapMode,
 ) -> Option<OverlapResult<'tcx>> {
     debug!("overlap(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);
 
     selcx.infcx().probe_maybe_skip_leak_check(skip_leak_check.is_yes(), |snapshot| {
-        overlap_within_probe(selcx, skip_leak_check, impl1_def_id, impl2_def_id, snapshot)
+        overlap_within_probe(
+            selcx,
+            skip_leak_check,
+            impl1_def_id,
+            impl2_def_id,
+            overlap_mode,
+            snapshot,
+        )
     })
 }
 
@@ -208,12 +169,10 @@ fn overlap_within_probe<'cx, 'tcx>(
     skip_leak_check: SkipLeakCheck,
     impl1_def_id: DefId,
     impl2_def_id: DefId,
+    overlap_mode: OverlapMode,
     snapshot: &CombinedSnapshot<'_, 'tcx>,
 ) -> Option<OverlapResult<'tcx>> {
     let infcx = selcx.infcx();
-    let tcx = infcx.tcx;
-
-    let overlap_mode = overlap_mode(tcx, impl1_def_id, impl2_def_id);
 
     if overlap_mode.use_negative_impl() {
         if negative_impl(selcx, impl1_def_id, impl2_def_id)

compiler/rustc_trait_selection/src/traits/specialize/mod.rs

@@ -257,6 +257,7 @@ pub(super) fn specialization_graph_provider(
     trait_id: DefId,
 ) -> specialization_graph::Graph {
     let mut sg = specialization_graph::Graph::new();
+    let overlap_mode = specialization_graph::OverlapMode::get(tcx, trait_id);
 
     let mut trait_impls: Vec<_> = tcx.all_impls(trait_id).collect();
 
@@ -270,7 +271,7 @@ pub(super) fn specialization_graph_provider(
     for impl_def_id in trait_impls {
         if let Some(impl_def_id) = impl_def_id.as_local() {
             // This is where impl overlap checking happens:
-            let insert_result = sg.insert(tcx, impl_def_id.to_def_id());
+            let insert_result = sg.insert(tcx, impl_def_id.to_def_id(), overlap_mode);
             // Report error if there was one.
             let (overlap, used_to_be_allowed) = match insert_result {
                 Err(overlap) => (Some(overlap), None),

compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs

@@ -41,6 +41,7 @@ trait ChildrenExt<'tcx> {
         tcx: TyCtxt<'tcx>,
         impl_def_id: DefId,
         simplified_self: Option<SimplifiedType>,
+        overlap_mode: OverlapMode,
     ) -> Result<Inserted, OverlapError>;
 }
 
@@ -92,6 +93,7 @@ impl ChildrenExt<'_> for Children {
         tcx: TyCtxt<'_>,
         impl_def_id: DefId,
         simplified_self: Option<SimplifiedType>,
+        overlap_mode: OverlapMode,
     ) -> Result<Inserted, OverlapError> {
         let mut last_lint = None;
         let mut replace_children = Vec::new();
@@ -142,6 +144,7 @@ impl ChildrenExt<'_> for Children {
                     possible_sibling,
                     impl_def_id,
                     traits::SkipLeakCheck::default(),
+                    overlap_mode,
                     |_| true,
                     || false,
                 );
@@ -166,6 +169,7 @@ impl ChildrenExt<'_> for Children {
                 possible_sibling,
                 impl_def_id,
                 traits::SkipLeakCheck::Yes,
+                overlap_mode,
                 |overlap| {
                     if let Some(overlap_kind) =
                         tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
@@ -273,6 +277,7 @@ pub trait GraphExt {
         &mut self,
         tcx: TyCtxt<'_>,
         impl_def_id: DefId,
+        overlap_mode: OverlapMode,
     ) -> Result<Option<FutureCompatOverlapError>, OverlapError>;
 
     /// Insert cached metadata mapping from a child impl back to its parent.
@@ -287,6 +292,7 @@ impl GraphExt for Graph {
         &mut self,
         tcx: TyCtxt<'_>,
         impl_def_id: DefId,
+        overlap_mode: OverlapMode,
     ) -> Result<Option<FutureCompatOverlapError>, OverlapError> {
         assert!(impl_def_id.is_local());
 
@@ -327,8 +333,12 @@ impl GraphExt for Graph {
         loop {
             use self::Inserted::*;
 
-            let insert_result =
-                self.children.entry(parent).or_default().insert(tcx, impl_def_id, simplified)?;
+            let insert_result = self.children.entry(parent).or_default().insert(
+                tcx,
+                impl_def_id,
+                simplified,
+                overlap_mode,
+            )?;
 
             match insert_result {
                 BecameNewSibling(opt_lint) => {

compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs

@@ -4,6 +4,7 @@ use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_hir::itemlikevisit::ItemLikeVisitor;
 use rustc_index::vec::IndexVec;
+use rustc_middle::traits::specialization_graph::OverlapMode;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::Symbol;
 use rustc_trait_selection::traits::{self, SkipLeakCheck};
@@ -99,14 +100,20 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
         }
     }
 
-    fn check_for_overlapping_inherent_impls(&self, impl1_def_id: DefId, impl2_def_id: DefId) {
+    fn check_for_overlapping_inherent_impls(
+        &self,
+        overlap_mode: OverlapMode,
+        impl1_def_id: DefId,
+        impl2_def_id: DefId,
+    ) {
         traits::overlapping_impls(
             self.tcx,
             impl1_def_id,
             impl2_def_id,
             // We go ahead and just skip the leak check for
             // inherent impls without warning.
             SkipLeakCheck::Yes,
+            overlap_mode,
             |overlap| {
                 self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap);
                 false
@@ -131,6 +138,8 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
                     return;
                 }
 
+                let overlap_mode = OverlapMode::get(self.tcx, item.def_id.to_def_id());
+
                 let impls_items = impls
                     .iter()
                     .map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id)))
@@ -145,6 +154,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
                         for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] {
                             if self.impls_have_common_items(impl_items1, impl_items2) {
                                 self.check_for_overlapping_inherent_impls(
+                                    overlap_mode,
                                     impl1_def_id,
                                     impl2_def_id,
                                 );
@@ -288,6 +298,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
                                 let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx];
                                 if self.impls_have_common_items(impl_items1, impl_items2) {
                                     self.check_for_overlapping_inherent_impls(
+                                        overlap_mode,
                                         impl1_def_id,
                                         impl2_def_id,
                                     );

src/test/ui/coherence/auxiliary/option_future.rs

@@ -1,8 +1,8 @@
 #![crate_type = "lib"]
 #![feature(negative_impls)]
 #![feature(rustc_attrs)]
+#![feature(with_negative_coherence)]
 
 pub trait Future {}
 
-#[rustc_with_negative_coherence]
 impl<E> !Future for Option<E> where E: Sized {}

src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs

@@ -1,17 +1,17 @@
 #![feature(negative_impls)]
 #![feature(rustc_attrs)]
 #![feature(trait_alias)]
+#![feature(with_negative_coherence)]
 
 trait A {}
 trait B {}
 trait AB = A + B;
 
 impl !A for u32 {}
 
-trait C {}
 #[rustc_strict_coherence]
+trait C {}
 impl<T: AB> C for T {}
-#[rustc_strict_coherence]
 impl C for u32 {}
 //~^ ERROR: conflicting implementations of trait `C` for type `u32` [E0119]
 // FIXME this should work, we should implement an `assemble_neg_candidates` fn

src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr

@@ -3,7 +3,6 @@ error[E0119]: conflicting implementations of trait `C` for type `u32`
    |
 LL | impl<T: AB> C for T {}
    | ------------------- first implementation here
-LL | #[rustc_strict_coherence]
 LL | impl C for u32 {}
    | ^^^^^^^^^^^^^^ conflicting implementation for `u32`