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,17 +13,20 @@ 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::{
+    CheckNumberOfEarlyBoundRegionsError, check_number_of_early_bound_regions,
+};
 use crate::errors::{EiiWithGenerics, LifetimesOrBoundsMismatchOnEii};
 
-/// checks whether the signature of some `external_impl`, matches
+/// Checks whether the signature of some `external_impl`, matches
 /// the signature of `declaration`, which it is supposed to be compatible
 /// with in order to implement the item.
 pub(crate) fn compare_eii_function_types<'tcx>(
@@ -43,33 +51,43 @@ pub(crate) fn compare_eii_function_types<'tcx>(
     let infcx = &tcx.infer_ctxt().build(TypingMode::non_body_analysis());
     let ocx = ObligationCtxt::new_with_diagnostics(infcx);
 
-    // We now need to check that the signature of the impl method is
-    // compatible with that of the trait method. We do this by
+    // We now need to check that the signature of the implementation is
+    // compatible with that of the declaration. We do this by
     // checking that `impl_fty <: trait_fty`.
     //
-    // FIXME: We manually instantiate the trait method here as we need
+    // FIXME: We manually instantiate the declaration here as we need
     // to manually compute its implied bounds. Otherwise this could just
     // be ocx.sub(impl_sig, trait_sig).
 
-    let wf_tys = FxIndexSet::default();
+    let mut wf_tys = FxIndexSet::default();
     let norm_cause = ObligationCause::misc(external_impl_span, external_impl);
 
     let declaration_sig = tcx.fn_sig(declaration).instantiate_identity();
-    let declaration_sig = infcx.enter_forall_and_leak_universe(declaration_sig);
-    let declaration_sig = ocx.normalize(&norm_cause, param_env, declaration_sig);
+    let declaration_sig = tcx.liberate_late_bound_regions(external_impl.into(), declaration_sig);
+    debug!(?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
+    // Next, add all inputs and output as well-formed tys. Importantly,
+    // we have to do this before normalization, since the normalized ty may
+    // not contain the input parameters. See issue #87748.
+    wf_tys.extend(declaration_sig.inputs_and_output.iter());
+    let declaration_sig = ocx.normalize(&norm_cause, param_env, declaration_sig);
+    // We also have to add the normalized declaration
+    // as we don't normalize during implied bounds computation.
+    wf_tys.extend(external_impl_sig.inputs_and_output.iter());
+
+    // FIXME: Copied over from compare impl items, same issue:
+    // We'd want to keep more accurate spans than "the method signature" when
     // processing the comparison between the trait and impl fn, but we sadly lose them
     // and point at the whole signature when a trait bound or specific input or output
     // type would be more appropriate. In other places we have a `Vec<Span>`
@@ -93,6 +111,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.evaluate_obligations_error_on_ambiguity();
@@ -116,6 +145,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,
@@ -124,11 +155,12 @@ fn check_is_structurally_compatible<'tcx>(
     eii_attr_span: Span,
 ) -> 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_number_of_arguments(tcx, external_impl, declaration, eii_name, eii_attr_span)?;
+    check_early_region_bounds(tcx, external_impl, declaration, eii_attr_span)?;
     Ok(())
 }
 
+/// externally implementable items can't have generics
 fn check_no_generics<'tcx>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
@@ -148,7 +180,45 @@ fn check_no_generics<'tcx>(
     Ok(())
 }
 
-fn compare_number_of_method_arguments<'tcx>(
+fn check_early_region_bounds<'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 Err(CheckNumberOfEarlyBoundRegionsError { span, generics_span, bounds_span, where_span }) =
+        check_number_of_early_bound_regions(
+            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 check_number_of_arguments<'tcx>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
     declaration: DefId,
@@ -159,152 +229,91 @@ 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();
-    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| {
-                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));
-
-        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());
+    // if the number of args are equal, we're trivially done
+    if declaration_number_args == external_impl_number_args {
+        Ok(())
+    } else {
+        Err(report_number_of_arguments_mismatch(
+            tcx,
+            external_impl,
+            declaration,
+            eii_name,
+            eii_attr_span,
+            declaration_number_args,
+            external_impl_number_args,
+        ))
     }
-
-    Ok(())
 }
 
-fn check_region_bounds_on_impl_item<'tcx>(
+fn report_number_of_arguments_mismatch<'tcx>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
     declaration: DefId,
+    eii_name: Symbol,
     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;
+    declaration_number_args: usize,
+    external_impl_number_args: usize,
+) -> ErrorGuaranteed {
+    let external_impl_name = tcx.item_name(external_impl.to_def_id());
 
-    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 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| {
+            if pos == 0 {
+                arg.span
+            } else {
+                arg.span.with_lo(external_impl_sig.decl.inputs[0].span.lo())
             }
-        }
-        let mut diag = tcx.dcx().create_err(LifetimesOrBoundsMismatchOnEii {
-            span,
-            ident: tcx.item_name(external_impl.to_def_id()),
-            generics_span,
-            bounds_span,
-            where_span,
-        });
+        })
+        .unwrap_or_else(|| tcx.def_span(external_impl));
 
-        diag.span_label(eii_attr_span, format!("required because of this attribute"));
-        return Err(diag.emit());
-    }
+    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
+    );
 
-    Ok(())
+    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"));
+
+    err.emit()
 }
 
 fn report_eii_mismatch<'tcx>(