Auto merge of #98221 - cjgillot:single-coh, r=lcnr

Perform coherence checking per impl.

r? `@ghost`

Author: bors

compiler/rustc_middle/src/query/mod.rs

@@ -906,9 +906,10 @@ rustc_queries! {
 
     /// Checks whether all impls in the crate pass the overlap check, returning
     /// which impls fail it. If all impls are correct, the returned slice is empty.
-    query orphan_check_crate(_: ()) -> &'tcx [LocalDefId] {
-        desc {
-            "checking whether the immpl in the this crate follow the orphan rules",
+    query orphan_check_impl(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
+        desc { |tcx|
+            "checking whether impl `{}` follows the orphan rules",
+            tcx.def_path_str(key.to_def_id()),
         }
     }
 

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

@@ -452,7 +452,7 @@ fn report_conflicting_impls(
     match used_to_be_allowed {
         None => {
             let reported = if overlap.with_impl.is_local()
-                || !tcx.orphan_check_crate(()).contains(&impl_def_id)
+                || tcx.orphan_check_impl(impl_def_id).is_ok()
             {
                 let err = struct_span_err!(tcx.sess, impl_span, E0119, "");
                 Some(decorate(

compiler/rustc_typeck/src/coherence/mod.rs

@@ -146,7 +146,7 @@ pub fn provide(providers: &mut Providers) {
     use self::builtin::coerce_unsized_info;
     use self::inherent_impls::{crate_incoherent_impls, crate_inherent_impls, inherent_impls};
     use self::inherent_impls_overlap::crate_inherent_impls_overlap_check;
-    use self::orphan::orphan_check_crate;
+    use self::orphan::orphan_check_impl;
 
     *providers = Providers {
         coherent_trait,
@@ -155,7 +155,7 @@ pub fn provide(providers: &mut Providers) {
         inherent_impls,
         crate_inherent_impls_overlap_check,
         coerce_unsized_info,
-        orphan_check_crate,
+        orphan_check_impl,
         ..*providers
     };
 }
@@ -171,21 +171,12 @@ fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) {
 
         check_impl(tcx, impl_def_id, trait_ref);
         check_object_overlap(tcx, impl_def_id, trait_ref);
-    }
-    builtin::check_trait(tcx, def_id);
-}
 
-pub fn check_coherence(tcx: TyCtxt<'_>) {
-    tcx.sess.time("unsafety_checking", || unsafety::check(tcx));
-    tcx.ensure().orphan_check_crate(());
-
-    for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
-        tcx.ensure().coherent_trait(trait_def_id);
+        tcx.sess.time("unsafety_checking", || unsafety::check_item(tcx, impl_def_id));
+        tcx.sess.time("orphan_checking", || tcx.ensure().orphan_check_impl(impl_def_id));
     }
 
-    // these queries are executed for side-effects (error reporting):
-    tcx.ensure().crate_inherent_impls(());
-    tcx.ensure().crate_inherent_impls_overlap_check(());
+    builtin::check_trait(tcx, def_id);
 }
 
 /// Checks whether an impl overlaps with the automatic `impl Trait for dyn Trait`.

compiler/rustc_typeck/src/coherence/orphan.rs

@@ -18,26 +18,29 @@ use rustc_span::Span;
 use rustc_trait_selection::traits;
 use std::ops::ControlFlow;
 
-pub(super) fn orphan_check_crate(tcx: TyCtxt<'_>, (): ()) -> &[LocalDefId] {
-    let mut errors = Vec::new();
-    for (&trait_def_id, impls_of_trait) in tcx.all_local_trait_impls(()) {
-        for &impl_of_trait in impls_of_trait {
-            match orphan_check_impl(tcx, impl_of_trait) {
-                Ok(()) => {}
-                Err(_) => errors.push(impl_of_trait),
-            }
-        }
+#[instrument(skip(tcx), level = "debug")]
+pub(crate) fn orphan_check_impl(
+    tcx: TyCtxt<'_>,
+    impl_def_id: LocalDefId,
+) -> Result<(), ErrorGuaranteed> {
+    let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
+    if let Some(err) = trait_ref.error_reported() {
+        return Err(err);
+    }
 
-        if tcx.trait_is_auto(trait_def_id) {
-            lint_auto_trait_impls(tcx, trait_def_id, impls_of_trait);
-        }
+    let ret = do_orphan_check_impl(tcx, trait_ref, impl_def_id);
+    if tcx.trait_is_auto(trait_ref.def_id) {
+        lint_auto_trait_impl(tcx, trait_ref, impl_def_id);
     }
-    tcx.arena.alloc_slice(&errors)
+
+    ret
 }
 
-#[instrument(skip(tcx), level = "debug")]
-fn orphan_check_impl(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGuaranteed> {
-    let trait_ref = tcx.impl_trait_ref(def_id).unwrap();
+fn do_orphan_check_impl<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    trait_ref: ty::TraitRef<'tcx>,
+    def_id: LocalDefId,
+) -> Result<(), ErrorGuaranteed> {
     let trait_def_id = trait_ref.def_id;
 
     let item = tcx.hir().item(hir::ItemId { def_id });
@@ -329,89 +332,82 @@ fn emit_orphan_check_error<'tcx>(
 
 /// Lint impls of auto traits if they are likely to have
 /// unsound or surprising effects on auto impls.
-fn lint_auto_trait_impls(tcx: TyCtxt<'_>, trait_def_id: DefId, impls: &[LocalDefId]) {
-    let mut non_covering_impls = Vec::new();
-    for &impl_def_id in impls {
-        let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
-        if trait_ref.references_error() {
-            return;
-        }
+fn lint_auto_trait_impl<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    trait_ref: ty::TraitRef<'tcx>,
+    impl_def_id: LocalDefId,
+) {
+    if tcx.impl_polarity(impl_def_id) != ImplPolarity::Positive {
+        return;
+    }
 
-        if tcx.impl_polarity(impl_def_id) != ImplPolarity::Positive {
+    assert_eq!(trait_ref.substs.len(), 1);
+    let self_ty = trait_ref.self_ty();
+    let (self_type_did, substs) = match self_ty.kind() {
+        ty::Adt(def, substs) => (def.did(), substs),
+        _ => {
+            // FIXME: should also lint for stuff like `&i32` but
+            // considering that auto traits are unstable, that
+            // isn't too important for now as this only affects
+            // crates using `nightly`, and std.
             return;
         }
+    };
 
-        assert_eq!(trait_ref.substs.len(), 1);
-        let self_ty = trait_ref.self_ty();
-        let (self_type_did, substs) = match self_ty.kind() {
-            ty::Adt(def, substs) => (def.did(), substs),
-            _ => {
-                // FIXME: should also lint for stuff like `&i32` but
-                // considering that auto traits are unstable, that
-                // isn't too important for now as this only affects
-                // crates using `nightly`, and std.
-                continue;
-            }
-        };
+    // Impls which completely cover a given root type are fine as they
+    // disable auto impls entirely. So only lint if the substs
+    // are not a permutation of the identity substs.
+    let Err(arg) = tcx.uses_unique_generic_params(substs, IgnoreRegions::Yes) else {
+        // ok
+        return;
+    };
 
-        // Impls which completely cover a given root type are fine as they
-        // disable auto impls entirely. So only lint if the substs
-        // are not a permutation of the identity substs.
-        match tcx.uses_unique_generic_params(substs, IgnoreRegions::Yes) {
-            Ok(()) => {} // ok
-            Err(arg) => {
-                // Ideally:
-                //
-                // - compute the requirements for the auto impl candidate
-                // - check whether these are implied by the non covering impls
-                // - if not, emit the lint
-                //
-                // What we do here is a bit simpler:
-                //
-                // - badly check if an auto impl candidate definitely does not apply
-                //   for the given simplified type
-                // - if so, do not lint
-                if fast_reject_auto_impl(tcx, trait_def_id, self_ty) {
-                    // ok
-                } else {
-                    non_covering_impls.push((impl_def_id, self_type_did, arg));
-                }
-            }
-        }
+    // Ideally:
+    //
+    // - compute the requirements for the auto impl candidate
+    // - check whether these are implied by the non covering impls
+    // - if not, emit the lint
+    //
+    // What we do here is a bit simpler:
+    //
+    // - badly check if an auto impl candidate definitely does not apply
+    //   for the given simplified type
+    // - if so, do not lint
+    if fast_reject_auto_impl(tcx, trait_ref.def_id, self_ty) {
+        // ok
+        return;
     }
 
-    for &(impl_def_id, self_type_did, arg) in &non_covering_impls {
-        tcx.struct_span_lint_hir(
-            lint::builtin::SUSPICIOUS_AUTO_TRAIT_IMPLS,
-            tcx.hir().local_def_id_to_hir_id(impl_def_id),
-            tcx.def_span(impl_def_id),
-            |err| {
-                let item_span = tcx.def_span(self_type_did);
-                let self_descr = tcx.def_kind(self_type_did).descr(self_type_did);
-                let mut err = err.build(&format!(
-                    "cross-crate traits with a default impl, like `{}`, \
+    tcx.struct_span_lint_hir(
+        lint::builtin::SUSPICIOUS_AUTO_TRAIT_IMPLS,
+        tcx.hir().local_def_id_to_hir_id(impl_def_id),
+        tcx.def_span(impl_def_id),
+        |err| {
+            let item_span = tcx.def_span(self_type_did);
+            let self_descr = tcx.def_kind(self_type_did).descr(self_type_did);
+            let mut err = err.build(&format!(
+                "cross-crate traits with a default impl, like `{}`, \
                          should not be specialized",
-                    tcx.def_path_str(trait_def_id),
-                ));
-                match arg {
-                    ty::util::NotUniqueParam::DuplicateParam(arg) => {
-                        err.note(&format!("`{}` is mentioned multiple times", arg));
-                    }
-                    ty::util::NotUniqueParam::NotParam(arg) => {
-                        err.note(&format!("`{}` is not a generic parameter", arg));
-                    }
+                tcx.def_path_str(trait_ref.def_id),
+            ));
+            match arg {
+                ty::util::NotUniqueParam::DuplicateParam(arg) => {
+                    err.note(&format!("`{}` is mentioned multiple times", arg));
                 }
-                err.span_note(
-                    item_span,
-                    &format!(
-                        "try using the same sequence of generic parameters as the {} definition",
-                        self_descr,
-                    ),
-                );
-                err.emit();
-            },
-        );
-    }
+                ty::util::NotUniqueParam::NotParam(arg) => {
+                    err.note(&format!("`{}` is not a generic parameter", arg));
+                }
+            }
+            err.span_note(
+                item_span,
+                &format!(
+                    "try using the same sequence of generic parameters as the {} definition",
+                    self_descr,
+                ),
+            );
+            err.emit();
+        },
+    );
 }
 
 fn fast_reject_auto_impl<'tcx>(tcx: TyCtxt<'tcx>, trait_def_id: DefId, self_ty: Ty<'tcx>) -> bool {

compiler/rustc_typeck/src/coherence/unsafety.rs

@@ -6,37 +6,18 @@ use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::Unsafety;
 use rustc_middle::ty::TyCtxt;
+use rustc_span::def_id::LocalDefId;
 
-pub fn check(tcx: TyCtxt<'_>) {
-    for id in tcx.hir().items() {
-        if matches!(tcx.def_kind(id.def_id), DefKind::Impl) {
-            let item = tcx.hir().item(id);
-            if let hir::ItemKind::Impl(ref impl_) = item.kind {
-                check_unsafety_coherence(
-                    tcx,
-                    item,
-                    Some(&impl_.generics),
-                    impl_.unsafety,
-                    impl_.polarity,
-                );
-            }
-        }
-    }
-}
+pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
+    debug_assert!(matches!(tcx.def_kind(def_id), DefKind::Impl));
+    let item = tcx.hir().expect_item(def_id);
+    let hir::ItemKind::Impl(ref impl_) = item.kind else { bug!() };
 
-fn check_unsafety_coherence<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    item: &hir::Item<'_>,
-    impl_generics: Option<&hir::Generics<'_>>,
-    unsafety: hir::Unsafety,
-    polarity: hir::ImplPolarity,
-) {
     if let Some(trait_ref) = tcx.impl_trait_ref(item.def_id) {
         let trait_def = tcx.trait_def(trait_ref.def_id);
-        let unsafe_attr = impl_generics.and_then(|generics| {
-            generics.params.iter().find(|p| p.pure_wrt_drop).map(|_| "may_dangle")
-        });
-        match (trait_def.unsafety, unsafe_attr, unsafety, polarity) {
+        let unsafe_attr =
+            impl_.generics.params.iter().find(|p| p.pure_wrt_drop).map(|_| "may_dangle");
+        match (trait_def.unsafety, unsafe_attr, impl_.unsafety, impl_.polarity) {
             (Unsafety::Normal, None, Unsafety::Unsafe, hir::ImplPolarity::Positive) => {
                 struct_span_err!(
                     tcx.sess,

compiler/rustc_typeck/src/lib.rs

@@ -513,7 +513,15 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
     })?;
 
     tcx.sess.track_errors(|| {
-        tcx.sess.time("coherence_checking", || coherence::check_coherence(tcx));
+        tcx.sess.time("coherence_checking", || {
+            for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
+                tcx.ensure().coherent_trait(trait_def_id);
+            }
+
+            // these queries are executed for side-effects (error reporting):
+            tcx.ensure().crate_inherent_impls(());
+            tcx.ensure().crate_inherent_impls_overlap_check(());
+        });
     })?;
 
     if tcx.features().rustc_attrs {

src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr

@@ -1,3 +1,15 @@
+error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
+   |
+LL | impl !Marker1 for dyn Object + Marker2 { }
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1`
+
+error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:17:1
+   |
+LL | impl !Marker2 for dyn Object + Marker2 { }
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2`
+
 error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
   --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:23:1
    |
@@ -21,18 +33,6 @@ error[E0321]: cross-crate traits with a default impl, like `Send`, can only be i
 LL | impl !Send for dyn Object + Marker2 {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type
 
-error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1`
-  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
-   |
-LL | impl !Marker1 for dyn Object + Marker2 { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1`
-
-error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2`
-  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:17:1
-   |
-LL | impl !Marker2 for dyn Object + Marker2 { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2`
-
 error: aborting due to 5 previous errors
 
 Some errors have detailed explanations: E0117, E0321, E0371.