Auto merge of #133365 - compiler-errors:compare-impl-item, r=lcnr

Make `compare_impl_item` into a query Turns `compare_impl_item` into a query (generalizing the existing query for `compare_impl_const`), and uses that in `Instance::resolve` to fail resolution when an implementation is incompatible with the trait it comes from. Fixes #119701 Fixes #121127 Fixes #121411 Fixes #129075 Fixes #129127 Fixes #129214 Fixes #131294
rust-lang · Dec 1, 2024 · 4af7fa7 · 4af7fa7
2 parents 7442931 + 1e655ef
commit 4af7fa7
Show file tree

Hide file tree

Showing 18 changed files with 123 additions and 228 deletions.
diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs
@@ -33,7 +33,7 @@ use tracing::{debug, instrument};
 use ty::TypingMode;
 use {rustc_attr as attr, rustc_hir as hir};
 
-use super::compare_impl_item::{check_type_bounds, compare_impl_method, compare_impl_ty};
+use super::compare_impl_item::check_type_bounds;
 use super::*;
 use crate::check::intrinsicck::InlineAsmCtxt;
 
@@ -1044,18 +1044,23 @@ fn check_impl_items_against_trait<'tcx>(
             tcx.dcx().span_delayed_bug(tcx.def_span(impl_item), "missing associated item in trait");
             continue;
         };
-        match ty_impl_item.kind {
-            ty::AssocKind::Const => {
-                tcx.ensure().compare_impl_const((
-                    impl_item.expect_local(),
-                    ty_impl_item.trait_item_def_id.unwrap(),
-                ));
-            }
-            ty::AssocKind::Fn => {
-                compare_impl_method(tcx, ty_impl_item, ty_trait_item, trait_ref);
-            }
-            ty::AssocKind::Type => {
-                compare_impl_ty(tcx, ty_impl_item, ty_trait_item, trait_ref);
+
+        let res = tcx.ensure().compare_impl_item(impl_item.expect_local());
+
+        if res.is_ok() {
+            match ty_impl_item.kind {
+                ty::AssocKind::Fn => {
+                    compare_impl_item::refine::check_refining_return_position_impl_trait_in_trait(
+                        tcx,
+                        ty_impl_item,
+                        ty_trait_item,
+                        tcx.impl_trait_ref(ty_impl_item.container_id(tcx))
+                            .unwrap()
+                            .instantiate_identity(),
+                    );
+                }
+                ty::AssocKind::Const => {}
+                ty::AssocKind::Type => {}
             }
         }
 

diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
@@ -33,7 +33,25 @@ use tracing::{debug, instrument};
 use super::potentially_plural_count;
 use crate::errors::{LifetimesOrBoundsMismatchOnTrait, MethodShouldReturnFuture};
 
-mod refine;
+pub(super) mod refine;
+
+/// Call the query `tcx.compare_impl_item()` directly instead.
+pub(super) fn compare_impl_item(
+    tcx: TyCtxt<'_>,
+    impl_item_def_id: LocalDefId,
+) -> Result<(), ErrorGuaranteed> {
+    let impl_item = tcx.associated_item(impl_item_def_id);
+    let trait_item = tcx.associated_item(impl_item.trait_item_def_id.unwrap());
+    let impl_trait_ref =
+        tcx.impl_trait_ref(impl_item.container_id(tcx)).unwrap().instantiate_identity();
+    debug!(?impl_trait_ref);
+
+    match impl_item.kind {
+        ty::AssocKind::Fn => compare_impl_method(tcx, impl_item, trait_item, impl_trait_ref),
+        ty::AssocKind::Type => compare_impl_ty(tcx, impl_item, trait_item, impl_trait_ref),
+        ty::AssocKind::Const => compare_impl_const(tcx, impl_item, trait_item, impl_trait_ref),
+    }
+}
 
 /// Checks that a method from an impl conforms to the signature of
 /// the same method as declared in the trait.
@@ -44,22 +62,15 @@ mod refine;
 /// - `trait_m`: the method in the trait
 /// - `impl_trait_ref`: the TraitRef corresponding to the trait implementation
 #[instrument(level = "debug", skip(tcx))]
-pub(super) fn compare_impl_method<'tcx>(
+fn compare_impl_method<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_m: ty::AssocItem,
     trait_m: ty::AssocItem,
     impl_trait_ref: ty::TraitRef<'tcx>,
-) {
-    let _: Result<_, ErrorGuaranteed> = try {
-        check_method_is_structurally_compatible(tcx, impl_m, trait_m, impl_trait_ref, false)?;
-        compare_method_predicate_entailment(tcx, impl_m, trait_m, impl_trait_ref)?;
-        refine::check_refining_return_position_impl_trait_in_trait(
-            tcx,
-            impl_m,
-            trait_m,
-            impl_trait_ref,
-        );
-    };
+) -> Result<(), ErrorGuaranteed> {
+    check_method_is_structurally_compatible(tcx, impl_m, trait_m, impl_trait_ref, false)?;
+    compare_method_predicate_entailment(tcx, impl_m, trait_m, impl_trait_ref)?;
+    Ok(())
 }
 
 /// Checks a bunch of different properties of the impl/trait methods for
@@ -1721,17 +1732,12 @@ fn compare_generic_param_kinds<'tcx>(
     Ok(())
 }
 
-/// Use `tcx.compare_impl_const` instead
-pub(super) fn compare_impl_const_raw(
-    tcx: TyCtxt<'_>,
-    (impl_const_item_def, trait_const_item_def): (LocalDefId, DefId),
+fn compare_impl_const<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    impl_const_item: ty::AssocItem,
+    trait_const_item: ty::AssocItem,
+    impl_trait_ref: ty::TraitRef<'tcx>,
 ) -> Result<(), ErrorGuaranteed> {
-    let impl_const_item = tcx.associated_item(impl_const_item_def);
-    let trait_const_item = tcx.associated_item(trait_const_item_def);
-    let impl_trait_ref =
-        tcx.impl_trait_ref(impl_const_item.container_id(tcx)).unwrap().instantiate_identity();
-    debug!(?impl_trait_ref);
-
     compare_number_of_generics(tcx, impl_const_item, trait_const_item, false)?;
     compare_generic_param_kinds(tcx, impl_const_item, trait_const_item, false)?;
     check_region_bounds_on_impl_item(tcx, impl_const_item, trait_const_item, false)?;
@@ -1862,19 +1868,17 @@ fn compare_const_predicate_entailment<'tcx>(
 }
 
 #[instrument(level = "debug", skip(tcx))]
-pub(super) fn compare_impl_ty<'tcx>(
+fn compare_impl_ty<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_ty: ty::AssocItem,
     trait_ty: ty::AssocItem,
     impl_trait_ref: ty::TraitRef<'tcx>,
-) {
-    let _: Result<(), ErrorGuaranteed> = try {
-        compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
-        compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
-        check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
-        compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
-        check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)?;
-    };
+) -> Result<(), ErrorGuaranteed> {
+    compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
+    compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
+    check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
+    compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
+    check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)
 }
 
 /// The equivalent of [compare_method_predicate_entailment], but for associated types

diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs
@@ -17,7 +17,7 @@ use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt;
 use rustc_trait_selection::traits::{ObligationCtxt, elaborate, normalize_param_env_or_error};
 
 /// Check that an implementation does not refine an RPITIT from a trait method signature.
-pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
+pub(crate) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_m: ty::AssocItem,
     trait_m: ty::AssocItem,

diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs
@@ -108,7 +108,7 @@ pub fn provide(providers: &mut Providers) {
         adt_async_destructor,
         region_scope_tree,
         collect_return_position_impl_trait_in_trait_tys,
-        compare_impl_const: compare_impl_item::compare_impl_const_raw,
+        compare_impl_item: compare_impl_item::compare_impl_item,
         check_coroutine_obligations: check::check_coroutine_obligations,
         ..*providers
     };

diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -2311,10 +2311,13 @@ rustc_queries! {
         desc { "checking validity requirement for `{}`: {}", key.1.value, key.0 }
     }
 
-    query compare_impl_const(
-        key: (LocalDefId, DefId)
-    ) -> Result<(), ErrorGuaranteed> {
-        desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0) }
+    /// This takes the def-id of an associated item from a impl of a trait,
+    /// and checks its validity against the trait item it corresponds to.
+    ///
+    /// Any other def id will ICE.
+    query compare_impl_item(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
+        desc { |tcx| "checking assoc item `{}` is compatible with trait definition", tcx.def_path_str(key) }
+        ensure_forwards_result_if_red
     }
 
     query deduced_param_attrs(def_id: DefId) -> &'tcx [ty::DeducedParamAttrs] {

diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs
@@ -216,15 +216,15 @@ fn resolve_associated_item<'tcx>(
 
             let args = tcx.erase_regions(args);
 
-            // Check if we just resolved an associated `const` declaration from
-            // a `trait` to an associated `const` definition in an `impl`, where
-            // the definition in the `impl` has the wrong type (for which an
-            // error has already been/will be emitted elsewhere).
-            if leaf_def.item.kind == ty::AssocKind::Const
-                && trait_item_id != leaf_def.item.def_id
+            // We check that the impl item is compatible with the trait item
+            // because otherwise we may ICE in const eval due to type mismatches,
+            // signature incompatibilities, etc.
+            // NOTE: We could also only enforce this in `PostAnalysis`, which
+            // is what CTFE and MIR inlining would care about anyways.
+            if trait_item_id != leaf_def.item.def_id
                 && let Some(leaf_def_item) = leaf_def.item.def_id.as_local()
             {
-                tcx.compare_impl_const((leaf_def_item, trait_item_id))?;
+                tcx.ensure().compare_impl_item(leaf_def_item)?;
             }
 
             Some(ty::Instance::new(leaf_def.item.def_id, args))

diff --git a/tests/crashes/119701.rs b/tests/crashes/119701.rs
diff --git a/tests/crashes/121127.rs b/tests/crashes/121127.rs
diff --git a/tests/crashes/121411.rs b/tests/crashes/121411.rs
diff --git a/tests/crashes/129075.rs b/tests/crashes/129075.rs
diff --git a/tests/crashes/129127.rs b/tests/crashes/129127.rs
diff --git a/tests/crashes/129214.rs b/tests/crashes/129214.rs
diff --git a/tests/crashes/131294-2.rs b/tests/crashes/131294-2.rs
diff --git a/tests/crashes/131294.rs b/tests/crashes/131294.rs
diff --git a/tests/ui/const-generics/issues/issue-83765.stderr b/tests/ui/const-generics/issues/issue-83765.stderr
@@ -29,11 +29,11 @@ note: ...which requires computing candidate for `<LazyUpdim<'_, T, <T as TensorD
 LL | trait TensorDimension {
    | ^^^^^^^^^^^^^^^^^^^^^
    = note: ...which again requires resolving instance `<LazyUpdim<'_, T, <T as TensorDimension>::DIM, DIM> as TensorDimension>::DIM`, completing the cycle
-note: cycle used when checking that `<impl at $DIR/issue-83765.rs:56:1: 56:97>` is well-formed
-  --> $DIR/issue-83765.rs:56:1
+note: cycle used when checking assoc item `<impl at $DIR/issue-83765.rs:56:1: 56:97>::bget` is compatible with trait definition
+  --> $DIR/issue-83765.rs:58:5
    |
-LL | impl<'a, T: Broadcastable, const DIM: usize> Broadcastable for LazyUpdim<'a, T, { T::DIM }, DIM> {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     fn bget(&self, index: [usize; DIM]) -> Option<Self::Element> {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
 
 error[E0308]: method not compatible with trait