type checking works (with some tests) for EII

jdonszelmann · jdonszelmann · commit 0923d9aea595 · 2025-04-18T17:33:47.000+02:00
diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl
@@ -294,6 +294,13 @@ hir_analysis_lifetime_not_captured = `impl Trait` captures lifetime parameter, b
     .label = lifetime captured due to being mentioned in the bounds of the `impl Trait`
     .param_label = this lifetime parameter is captured
 
+hir_analysis_lifetimes_or_bounds_mismatch_on_eii =
+    lifetime parameters or bounds `{$ident}` do not match the declaration
+    .label = lifetimes do not match
+    .generics_label = lifetimes in impl do not match this signature
+    .where_label = this `where` clause might not match the one in the trait
+    .bounds_label = this bound might be missing in the implementation
+
 hir_analysis_lifetimes_or_bounds_mismatch_on_trait =
     lifetime parameters or bounds on {$item_kind} `{$ident}` do not match the trait declaration
     .label = lifetimes do not match {$item_kind} in trait
diff --git a/compiler/rustc_hir_analysis/src/check/compare_eii.rs b/compiler/rustc_hir_analysis/src/check/compare_eii.rs
@@ -2,29 +2,219 @@ use std::borrow::Cow;
 use std::iter;
 
 use rustc_data_structures::fx::FxIndexSet;
-use rustc_errors::{Applicability, E0053, E0053, struct_span_code_err, struct_span_code_err};
+use rustc_errors::{
+    Applicability, Applicability, E0050, E0053, E0053, E0053, struct_span_code_err,
+    struct_span_code_err, struct_span_code_err,
+};
 use rustc_hir::def_id::{DefId, LocalDefId};
-use rustc_hir::{self as hir, self as hir, HirId, HirId, ItemKind, ItemKind};
+use rustc_hir::{
+    self as hir, self as hir, self as hir, FnSig, HirId, HirId, HirId, ItemKind, ItemKind, ItemKind,
+};
 use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
 use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
 use rustc_middle::ty;
 use rustc_middle::ty::TyCtxt;
-use rustc_middle::ty::error::{ExpectedFound, TypeError, TypeError};
+use rustc_middle::ty::error::{ExpectedFound, ExpectedFound, TypeError, TypeError, TypeError};
 use rustc_span::{ErrorGuaranteed, Ident, Span};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::regions::InferCtxtRegionExt;
 use rustc_trait_selection::traits::ObligationCtxt;
 use rustc_type_ir::TypingMode;
 use tracing::{debug, instrument};
 
+use super::potentially_plural_count;
+use crate::errors::LifetimesOrBoundsMismatchOnEII;
+
+/// 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.
+fn check_is_structurally_compatible<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    external_impl: LocalDefId,
+    declaration: DefId,
+    eii_name: Symbol,
+    eii_attr_span: Span,
+) -> Result<(), ErrorGuaranteed> {
+    // FIXME(jdonszelmann): check no generics
+    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)?;
+    Ok(())
+}
+
+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 declaration_generics = tcx.generics_of(declaration);
+    let declaration_params = declaration_generics.own_counts().lifetimes;
+
+    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(declaration_node) = tcx.hir_get_if_local(declaration)
+                && let Some(declaration_generics) = declaration_node.generics()
+            {
+                let mut impl_bounds = 0;
+                for p in declaration_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 declaration_generics.has_where_clause_predicates {
+                    where_span = Some(declaration_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,
+        });
+
+        diag.span_label(eii_attr_span, format!("required because of this attribute"));
+        return Err(diag.emit());
+    }
+
+    Ok(())
+}
+
+fn compare_number_of_method_arguments<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    external_impl: LocalDefId,
+    declaration: DefId,
+    eii_name: Symbol,
+    eii_attr_span: Span,
+) -> Result<(), ErrorGuaranteed> {
+    let external_impl_fty = tcx.fn_sig(external_impl);
+    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));
+
+        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,
+            E0050, // FIXME(jdonszelmann): new error code
+            "`{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(())
+}
+
 // 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>(
     tcx: TyCtxt<'tcx>,
     external_impl: LocalDefId,
     declaration: DefId,
+    eii_name: Symbol,
+    eii_attr_span: Span,
 ) -> Result<(), ErrorGuaranteed> {
+    check_is_structurally_compatible(tcx, external_impl, declaration, eii_name, eii_attr_span)?;
+
     let external_impl_span = tcx.def_span(external_impl);
     let cause = ObligationCause::new(
         external_impl_span,
@@ -52,6 +242,12 @@ pub(crate) fn compare_eii_function_types<'tcx>(
     // type.
 
     let 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 external_impl_sig = infcx.instantiate_binder_with_fresh_vars(
         external_impl_span,
         infer::HigherRankedType,
@@ -60,16 +256,9 @@ pub(crate) fn compare_eii_function_types<'tcx>(
             infcx.fresh_args_for_item(external_impl_span, external_impl.to_def_id()),
         ),
     );
-
-    let norm_cause = ObligationCause::misc(external_impl_span, external_impl);
     let external_impl_sig = ocx.normalize(&norm_cause, param_env, external_impl_sig);
     debug!(?external_impl_sig);
 
-    let declaration_sig = tcx.fn_sig(declaration).instantiate_identity();
-    let declaration_sig =
-        tcx.liberate_late_bound_regions(external_impl.to_def_id(), declaration_sig);
-    let declaration_sig = ocx.normalize(&norm_cause, param_env, declaration_sig);
-
     // FIXME: 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
@@ -201,8 +390,7 @@ fn extract_spans_for_error_reporting<'tcx>(
     };
 
     let declaration_args = declaration.as_local().map(|def_id| {
-        let hir_id: HirId = tcx.local_def_id_to_hir_id(def_id);
-        if let Some(sig) = tcx.hir_fn_sig_by_hir_id(hir_id) {
+        if let Some(sig) = get_declaration_sig(tcx, def_id) {
             sig.decl.inputs.iter().map(|t| t.span).chain(iter::once(sig.decl.output.span()))
         } else {
             panic!("expected {def_id:?} to be a foreign function");
@@ -218,3 +406,8 @@ fn extract_spans_for_error_reporting<'tcx>(
         _ => (cause.span, tcx.hir().span_if_local(declaration), external_impl_name),
     }
 }
+
+fn get_declaration_sig<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Option<&'tcx FnSig<'tcx>> {
+    let hir_id: HirId = tcx.local_def_id_to_hir_id(def_id);
+    tcx.hir_fn_sig_by_hir_id(hir_id)
+}
diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs
@@ -1298,7 +1298,7 @@ fn check_item_fn(
     enter_wf_checking_ctxt(tcx, span, def_id, |wfcx| {
         // does the function have an EiiImpl attribute? that contains the defid of a *macro*
         // that was used to mark the implementation. This is a two step process.
-        for EIIImpl { eii_macro, .. } in
+        for EIIImpl { eii_macro, span, .. } in
             find_attr!(tcx.get_all_attrs(def_id), AttributeKind::EiiImpl(impls) => impls)
                 .into_iter()
                 .flatten()
@@ -1307,7 +1307,13 @@ fn check_item_fn(
             // signature that we'd like to compare the function we're currently checking with
             if let Some(eii_extern_item) = find_attr!(tcx.get_all_attrs(*eii_macro), AttributeKind::EiiMacroFor {eii_extern_item, ..} => *eii_extern_item)
             {
-                let _ = compare_eii_function_types(tcx, def_id, eii_extern_item);
+                let _ = compare_eii_function_types(
+                    tcx,
+                    def_id,
+                    eii_extern_item,
+                    tcx.item_name(*eii_macro),
+                    *span,
+                );
             } else {
                 panic!(
                     "EII impl macro {eii_macro:?} did not have an eii macro for attribute pointing to a function"
diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs
@@ -207,6 +207,21 @@ pub(crate) struct LifetimesOrBoundsMismatchOnTrait {
     pub ident: Ident,
 }
 
+#[derive(Diagnostic)]
+#[diag(hir_analysis_lifetimes_or_bounds_mismatch_on_eii)]
+pub(crate) struct LifetimesOrBoundsMismatchOnEII {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[label(hir_analysis_generics_label)]
+    pub generics_span: Option<Span>,
+    #[label(hir_analysis_where_label)]
+    pub where_span: Option<Span>,
+    #[label(hir_analysis_bounds_label)]
+    pub bounds_span: Vec<Span>,
+    pub ident: Symbol,
+}
+
 #[derive(Diagnostic)]
 #[diag(hir_analysis_drop_impl_on_wrong_item, code = E0120)]
 pub(crate) struct DropImplOnWrongItem {
diff --git a/tests/ui/eii/cross_crate_wrong_ty.rs b/tests/ui/eii/cross_crate_wrong_ty.rs
@@ -0,0 +1,18 @@
+//@ compile-flags: --crate-type rlib
+//@ aux-build: cross_crate_eii_declaration.rs
+#![feature(eii)]
+#![feature(decl_macro)]
+#![feature(rustc_attrs)]
+#![feature(eii_internals)]
+
+extern crate cross_crate_eii_declaration;
+
+#[unsafe(cross_crate_eii_declaration::foo)]
+fn other() -> u64 {
+//~^ ERROR `other` has 0 parameters but #[foo] requires it to have 1
+    0
+}
+
+fn main() {
+    cross_crate_eii_declaration::bar(0);
+}
diff --git a/tests/ui/eii/cross_crate_wrong_ty.stderr b/tests/ui/eii/cross_crate_wrong_ty.stderr
@@ -0,0 +1,11 @@
+error[E0050]: `other` has 0 parameters but #[foo] requires it to have 1
+  --> $DIR/cross_crate_wrong_ty.rs:11:1
+   |
+LL | #[unsafe(cross_crate_eii_declaration::foo)]
+   | ------------------------------------------- required because of this attribute
+LL | fn other() -> u64 {
+   | ^^^^^^^^^^^^^^^^^ expected 1 parameter, found 0
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0050`.
diff --git a/tests/ui/eii/subtype_1.rs b/tests/ui/eii/subtype_1.rs
@@ -1,4 +1,5 @@
 //@ compile-flags: --crate-type rlib
+//FIXME: known ICE
 #![feature(eii)]
 #![feature(decl_macro)]
 #![feature(rustc_attrs)]
@@ -16,6 +17,7 @@ unsafe extern "Rust" {
 
 #[foo]
 fn other<'a, 'b>(x: &'b u64) -> &'b u64 {
+//~^ ERROR lifetime parameters or bounds `other` do not match the declaration
     &0
 }
 
diff --git a/tests/ui/eii/subtype_1.stderr b/tests/ui/eii/subtype_1.stderr
@@ -0,0 +1,13 @@
+error: lifetime parameters or bounds `other` do not match the declaration
+  --> $DIR/subtype_1.rs:19:9
+   |
+LL |     safe fn bar<'a, 'b>(x: &'b u64) -> &'a u64;
+   |                -------- lifetimes in impl do not match this signature
+...
+LL | #[foo]
+   | ------ required because of this attribute
+LL | fn other<'a, 'b>(x: &'b u64) -> &'b u64 {
+   |         ^^^^^^^^ lifetimes do not match
+
+error: aborting due to 1 previous error
+
diff --git a/tests/ui/eii/subtype_2.rs b/tests/ui/eii/subtype_2.rs
diff --git a/tests/ui/eii/subtype_2.stderr b/tests/ui/eii/subtype_2.stderr
diff --git a/tests/ui/eii/wrong_ty_2.rs b/tests/ui/eii/wrong_ty_2.rs
diff --git a/tests/ui/eii/wrong_ty_2.stderr b/tests/ui/eii/wrong_ty_2.stderr
