fix problems with type checking

Author: jdonszelmann

compiler/rustc_hir_analysis/src/check/compare_eii.rs

@@ -1,3 +1,8 @@
+//! This module is very similar to `compare_impl_item`.
+//! Most logic is taken from there,
+//! since in a very similar way we're comparing some declaration of a signature to an implementation.
+//! The major difference is that we don't bother with self types, since for EIIs we're comparing freestanding item.
+
 use std::borrow::Cow;
 use std::iter;
 
@@ -8,14 +13,15 @@ use rustc_hir::{self as hir, FnSig, HirId, ItemKind};
 use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
 use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
-use rustc_middle::ty::{self, TyCtxt, TypingMode};
+use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt, TypingMode};
 use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::regions::InferCtxtRegionExt;
-use rustc_trait_selection::traits::ObligationCtxt;
+use rustc_trait_selection::traits::{self, ObligationCtxt};
 use tracing::{debug, instrument};
 
 use super::potentially_plural_count;
+use crate::check::compare_impl_item::{CheckRegionBoundsOnItemOutput, check_region_bounds_on_item};
 use crate::errors::{EiiWithGenerics, LifetimesOrBoundsMismatchOnEii};
 
 /// checks whether the signature of some `external_impl`, matches
@@ -58,15 +64,15 @@ pub(crate) fn compare_eii_function_types<'tcx>(
     let declaration_sig = infcx.enter_forall_and_leak_universe(declaration_sig);
     let declaration_sig = ocx.normalize(&norm_cause, param_env, declaration_sig);
 
-    let external_impl_sig = infcx.instantiate_binder_with_fresh_vars(
+    let unnormalized_external_impl_sig = infcx.instantiate_binder_with_fresh_vars(
         external_impl_span,
         infer::BoundRegionConversionTime::HigherRankedType,
         tcx.fn_sig(external_impl).instantiate(
             tcx,
             infcx.fresh_args_for_item(external_impl_span, external_impl.to_def_id()),
         ),
     );
-    let external_impl_sig = ocx.normalize(&norm_cause, param_env, external_impl_sig);
+    let external_impl_sig = ocx.normalize(&norm_cause, param_env, unnormalized_external_impl_sig);
     debug!(?external_impl_sig);
 
     // FIXME: We'd want to keep more accurate spans than "the method signature" when
@@ -93,6 +99,17 @@ pub(crate) fn compare_eii_function_types<'tcx>(
         return Err(emitted);
     }
 
+    if !(declaration_sig, external_impl_sig).references_error() {
+        for ty in unnormalized_external_impl_sig.inputs_and_output {
+            ocx.register_obligation(traits::Obligation::new(
+                infcx.tcx,
+                cause.clone(),
+                param_env,
+                ty::ClauseKind::WellFormed(ty.into()),
+            ));
+        }
+    }
+
     // Check that all obligations are satisfied by the implementation's
     // version.
     let errors = ocx.select_all_or_error();
@@ -116,6 +133,8 @@ pub(crate) fn compare_eii_function_types<'tcx>(
 /// Checks a bunch of different properties of the impl/trait methods for
 /// compatibility, such as asyncness, number of argument, self receiver kind,
 /// and number of early- and late-bound generics.
+///
+/// Corresponds to `check_method_is_structurally_compatible` for impl method compatibility checks.
 fn check_is_structurally_compatible<'tcx>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
@@ -125,7 +144,7 @@ fn check_is_structurally_compatible<'tcx>(
 ) -> Result<(), ErrorGuaranteed> {
     check_no_generics(tcx, external_impl, declaration, eii_name, eii_attr_span)?;
     compare_number_of_method_arguments(tcx, external_impl, declaration, eii_name, eii_attr_span)?;
-    check_region_bounds_on_impl_item(tcx, external_impl, declaration, eii_attr_span)?;
+    check_region_bounds_on_eii(tcx, external_impl, declaration, eii_attr_span)?;
     Ok(())
 }
 
@@ -148,6 +167,44 @@ fn check_no_generics<'tcx>(
     Ok(())
 }
 
+fn check_region_bounds_on_eii<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    external_impl: LocalDefId,
+    declaration: DefId,
+    eii_attr_span: Span,
+) -> Result<(), ErrorGuaranteed> {
+    let external_impl_generics = tcx.generics_of(external_impl.to_def_id());
+    let external_impl_params = external_impl_generics.own_counts().lifetimes;
+
+    let declaration_generics = tcx.generics_of(declaration);
+    let declaration_params = declaration_generics.own_counts().lifetimes;
+
+    let Some(CheckRegionBoundsOnItemOutput { span, generics_span, bounds_span, where_span }) =
+        check_region_bounds_on_item(
+            tcx,
+            external_impl,
+            declaration,
+            external_impl_generics,
+            external_impl_params,
+            declaration_generics,
+            declaration_params,
+        )
+    else {
+        return Ok(());
+    };
+
+    let mut diag = tcx.dcx().create_err(LifetimesOrBoundsMismatchOnEii {
+        span,
+        ident: tcx.item_name(external_impl.to_def_id()),
+        generics_span,
+        bounds_span,
+        where_span,
+    });
+
+    diag.span_label(eii_attr_span, format!("required because of this attribute"));
+    return Err(diag.emit());
+}
+
 fn compare_number_of_method_arguments<'tcx>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
@@ -159,152 +216,71 @@ fn compare_number_of_method_arguments<'tcx>(
     let declaration_fty = tcx.fn_sig(declaration);
     let declaration_number_args = declaration_fty.skip_binder().inputs().skip_binder().len();
     let external_impl_number_args = external_impl_fty.skip_binder().inputs().skip_binder().len();
+
+    // if the number of args are equal, we're trivially done
+    if declaration_number_args == external_impl_number_args {
+        return Ok(());
+    }
+
     let external_impl_name = tcx.item_name(external_impl.to_def_id());
 
-    if declaration_number_args != external_impl_number_args {
-        let declaration_span = declaration
-            .as_local()
-            .and_then(|def_id| {
-                let declaration_sig = get_declaration_sig(tcx, def_id).expect("foreign item sig");
-                let pos = declaration_number_args.saturating_sub(1);
-                declaration_sig.decl.inputs.get(pos).map(|arg| {
-                    if pos == 0 {
-                        arg.span
-                    } else {
-                        arg.span.with_lo(declaration_sig.decl.inputs[0].span.lo())
-                    }
-                })
-            })
-            .or_else(|| tcx.hir_span_if_local(declaration))
-            .unwrap_or_else(|| tcx.def_span(declaration));
-
-        let (_, external_impl_sig, _, _) = &tcx.hir_expect_item(external_impl).expect_fn();
-        let pos = external_impl_number_args.saturating_sub(1);
-        let impl_span = external_impl_sig
-            .decl
-            .inputs
-            .get(pos)
-            .map(|arg| {
+    let declaration_span = declaration
+        .as_local()
+        .and_then(|def_id| {
+            let declaration_sig = get_declaration_sig(tcx, def_id).expect("foreign item sig");
+            let pos = declaration_number_args.saturating_sub(1);
+            declaration_sig.decl.inputs.get(pos).map(|arg| {
                 if pos == 0 {
                     arg.span
                 } else {
-                    arg.span.with_lo(external_impl_sig.decl.inputs[0].span.lo())
+                    arg.span.with_lo(declaration_sig.decl.inputs[0].span.lo())
                 }
             })
-            .unwrap_or_else(|| tcx.def_span(external_impl));
-
-        let mut err = struct_span_code_err!(
-            tcx.dcx(),
-            impl_span,
-            E0805,
-            "`{external_impl_name}` has {} but #[{eii_name}] requires it to have {}",
-            potentially_plural_count(external_impl_number_args, "parameter"),
-            declaration_number_args
-        );
-
-        // if let Some(declaration_span) = declaration_span {
-        err.span_label(
-            declaration_span,
-            format!("requires {}", potentially_plural_count(declaration_number_args, "parameter")),
-        );
-        // }
-
-        err.span_label(
-            impl_span,
-            format!(
-                "expected {}, found {}",
-                potentially_plural_count(declaration_number_args, "parameter"),
-                external_impl_number_args
-            ),
-        );
-
-        err.span_label(eii_attr_span, format!("required because of this attribute"));
-
-        return Err(err.emit());
-    }
-
-    Ok(())
-}
+        })
+        .or_else(|| tcx.hir_span_if_local(declaration))
+        .unwrap_or_else(|| tcx.def_span(declaration));
+
+    let (_, external_impl_sig, _, _) = &tcx.hir_expect_item(external_impl).expect_fn();
+    let pos = external_impl_number_args.saturating_sub(1);
+    let impl_span = external_impl_sig
+        .decl
+        .inputs
+        .get(pos)
+        .map(|arg| {
+            if pos == 0 {
+                arg.span
+            } else {
+                arg.span.with_lo(external_impl_sig.decl.inputs[0].span.lo())
+            }
+        })
+        .unwrap_or_else(|| tcx.def_span(external_impl));
 
-fn check_region_bounds_on_impl_item<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    external_impl: LocalDefId,
-    declaration: DefId,
-    eii_attr_span: Span,
-) -> Result<(), ErrorGuaranteed> {
-    let external_impl_generics = tcx.generics_of(external_impl.to_def_id());
-    let external_impl_params = external_impl_generics.own_counts().lifetimes;
+    let mut err = struct_span_code_err!(
+        tcx.dcx(),
+        impl_span,
+        E0805,
+        "`{external_impl_name}` has {} but #[{eii_name}] requires it to have {}",
+        potentially_plural_count(external_impl_number_args, "parameter"),
+        declaration_number_args
+    );
 
-    let declaration_generics = tcx.generics_of(declaration);
-    let declaration_params = declaration_generics.own_counts().lifetimes;
+    err.span_label(
+        declaration_span,
+        format!("requires {}", potentially_plural_count(declaration_number_args, "parameter")),
+    );
 
-    debug!(?declaration_generics, ?external_impl_generics);
-
-    // Must have same number of early-bound lifetime parameters.
-    // Unfortunately, if the user screws up the bounds, then this
-    // will change classification between early and late. E.g.,
-    // if in trait we have `<'a,'b:'a>`, and in impl we just have
-    // `<'a,'b>`, then we have 2 early-bound lifetime parameters
-    // in trait but 0 in the impl. But if we report "expected 2
-    // but found 0" it's confusing, because it looks like there
-    // are zero. Since I don't quite know how to phrase things at
-    // the moment, give a kind of vague error message.
-    if declaration_params != external_impl_params {
-        let span = tcx
-            .hir_get_generics(external_impl)
-            .expect("expected impl item to have generics or else we can't compare them")
-            .span;
-
-        let mut generics_span = None;
-        let mut bounds_span = vec![];
-        let mut where_span = None;
-
-        if let Some(declaration_node) = tcx.hir_get_if_local(declaration)
-            && let Some(declaration_generics) = declaration_node.generics()
-        {
-            generics_span = Some(declaration_generics.span);
-            // FIXME: we could potentially look at the impl's bounds to not point at bounds that
-            // *are* present in the impl.
-            for p in declaration_generics.predicates {
-                if let hir::WherePredicateKind::BoundPredicate(pred) = p.kind {
-                    for b in pred.bounds {
-                        if let hir::GenericBound::Outlives(lt) = b {
-                            bounds_span.push(lt.ident.span);
-                        }
-                    }
-                }
-            }
-            if let Some(implementation_generics) = tcx.hir_get_generics(external_impl) {
-                let mut impl_bounds = 0;
-                for p in implementation_generics.predicates {
-                    if let hir::WherePredicateKind::BoundPredicate(pred) = p.kind {
-                        for b in pred.bounds {
-                            if let hir::GenericBound::Outlives(_) = b {
-                                impl_bounds += 1;
-                            }
-                        }
-                    }
-                }
-                if impl_bounds == bounds_span.len() {
-                    bounds_span = vec![];
-                } else if implementation_generics.has_where_clause_predicates {
-                    where_span = Some(implementation_generics.where_clause_span);
-                }
-            }
-        }
-        let mut diag = tcx.dcx().create_err(LifetimesOrBoundsMismatchOnEii {
-            span,
-            ident: tcx.item_name(external_impl.to_def_id()),
-            generics_span,
-            bounds_span,
-            where_span,
-        });
+    err.span_label(
+        impl_span,
+        format!(
+            "expected {}, found {}",
+            potentially_plural_count(declaration_number_args, "parameter"),
+            external_impl_number_args
+        ),
+    );
 
-        diag.span_label(eii_attr_span, format!("required because of this attribute"));
-        return Err(diag.emit());
-    }
+    err.span_label(eii_attr_span, format!("required because of this attribute"));
 
-    Ok(())
+    Err(err.emit())
 }
 
 fn report_eii_mismatch<'tcx>(