about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorJubilee <workingjubilee@gmail.com>2025-02-05 19:53:51 -0800
committerGitHub <noreply@github.com>2025-02-05 19:53:51 -0800
commit736f9025816435c4eb98c2397ec6512d4f2cb94c (patch)
treecddb74cf868a237b64a4b065bfeae5aa6c74d9ae /compiler
parentffa67f9c445ac26d2513d8b6be5326dac955800a (diff)
parent4718cb40ecd5e68ae7c5d056d48c60b5e9c1d1ef (diff)
Rollup merge of #136583 - Jarcho:fn_ctxt2, r=compiler-errors
Only highlight unmatchable parameters at the definition site

Followup to #136497

This generally results more focused messages in the same vein as #99635 (see `test/ui/argument-suggestions/complex.rs`). There are still some cases (e.g. `test/ui/argument-suggestions/permuted_arguments.rs`) where it might be worth highlighting the arguments. This is mitigated by the fact that a suggestion with a suggested rearrangement is given.

r? `@compiler-errors`
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs316
1 files changed, 155 insertions, 161 deletions
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
index 66f38deefba..5ce9e0556b4 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
@@ -1,4 +1,4 @@
-use std::{iter, mem};
+use std::{fmt, iter, mem};
 
 use itertools::Itertools;
 use rustc_data_structures::fx::FxIndexSet;
@@ -13,7 +13,7 @@ use rustc_hir::{ExprKind, HirId, Node, QPath};
 use rustc_hir_analysis::check::intrinsicck::InlineAsmCtxt;
 use rustc_hir_analysis::check::potentially_plural_count;
 use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
-use rustc_index::{Idx, IndexVec};
+use rustc_index::IndexVec;
 use rustc_infer::infer::{DefineOpaqueTypes, InferOk, TypeTrace};
 use rustc_middle::ty::adjustment::AllowTwoPhase;
 use rustc_middle::ty::error::TypeError;
@@ -25,6 +25,7 @@ use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym};
 use rustc_trait_selection::error_reporting::infer::{FailureCode, ObligationCauseExt};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext};
+use smallvec::SmallVec;
 use tracing::debug;
 use {rustc_ast as ast, rustc_hir as hir};
 
@@ -44,6 +45,12 @@ use crate::{
     struct_span_code_err,
 };
 
+rustc_index::newtype_index! {
+    #[orderable]
+    #[debug_format = "GenericIdx({})"]
+    pub(crate) struct GenericIdx {}
+}
+
 #[derive(Clone, Copy, Default)]
 pub(crate) enum DivergingBlockBehavior {
     /// This is the current stable behavior:
@@ -2291,7 +2298,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // If we're calling a method of a Fn/FnMut/FnOnce trait object implicitly
         // (eg invoking a closure) we want to point at the underlying callable,
         // not the method implicitly invoked (eg call_once).
-        if let Some(assoc_item) = self.tcx.opt_associated_item(def_id)
+        // TupleArguments is set only when this is an implicit call (my_closure(...)) rather than explicit (my_closure.call(...))
+        if tuple_arguments == TupleArguments
+            && let Some(assoc_item) = self.tcx.opt_associated_item(def_id)
             // Since this is an associated item, it might point at either an impl or a trait item.
             // We want it to always point to the trait item.
             // If we're pointing at an inherent function, we don't need to do anything,
@@ -2301,8 +2310,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // Just an easy way to check "trait_def_id == Fn/FnMut/FnOnce"
             && let Some(call_kind) = self.tcx.fn_trait_kind_from_def_id(maybe_trait_def_id)
             && let Some(callee_ty) = callee_ty
-            // TupleArguments is set only when this is an implicit call (my_closure(...)) rather than explicit (my_closure.call(...))
-            && tuple_arguments == TupleArguments
         {
             let callee_ty = callee_ty.peel_refs();
             match *callee_ty.kind() {
@@ -2371,174 +2378,136 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             && !def_span.is_dummy()
         {
             let mut spans: MultiSpan = def_span.into();
-
-            if let Some(params_with_generics) = self.get_hir_params_with_generics(def_id, is_method)
+            if let Some((params_with_generics, hir_generics)) =
+                self.get_hir_param_info(def_id, is_method)
             {
-                debug_assert_eq!(params_with_generics.len(), matched_inputs.len());
-
-                let mut generics_with_unmatched_params = Vec::new();
-
-                let check_for_matched_generics = || {
-                    if matched_inputs.iter().any(|x| x.is_some())
-                        && params_with_generics.iter().any(|(x, _)| x.is_some())
-                    {
-                        for (idx, (generic, _)) in params_with_generics.iter_enumerated() {
-                            // Param has to have a generic and be matched to be relevant
-                            if matched_inputs[idx].is_none() {
-                                continue;
-                            }
-
-                            let Some(generic) = generic else {
-                                continue;
-                            };
+                struct MismatchedParam<'a> {
+                    idx: ExpectedIdx,
+                    generic: GenericIdx,
+                    param: &'a FnParam<'a>,
+                    deps: SmallVec<[ExpectedIdx; 4]>,
+                }
 
-                            for unmatching_idx in
-                                idx.plus(1)..ExpectedIdx::from_usize(params_with_generics.len())
-                            {
-                                if matched_inputs[unmatching_idx].is_none()
-                                    && let Some(unmatched_idx_param_generic) =
-                                        params_with_generics[unmatching_idx].0
-                                    && unmatched_idx_param_generic.name.ident()
-                                        == generic.name.ident()
-                                {
-                                    // We found a parameter that didn't match that needed to
-                                    return true;
-                                }
-                            }
-                        }
+                debug_assert_eq!(params_with_generics.len(), matched_inputs.len());
+                // Gather all mismatched parameters with generics.
+                let mut mismatched_params = Vec::<MismatchedParam<'_>>::new();
+                if let Some(expected_idx) = expected_idx {
+                    let expected_idx = ExpectedIdx::from_usize(expected_idx);
+                    let &(expected_generic, ref expected_param) =
+                        &params_with_generics[expected_idx];
+                    if let Some(expected_generic) = expected_generic {
+                        mismatched_params.push(MismatchedParam {
+                            idx: expected_idx,
+                            generic: expected_generic,
+                            param: expected_param,
+                            deps: SmallVec::new(),
+                        });
+                    } else {
+                        // Still mark the mismatched parameter
+                        spans.push_span_label(expected_param.span(), "");
                     }
-                    false
-                };
-
-                let check_for_matched_generics = check_for_matched_generics();
-
-                for (idx, &(generic_param, param)) in
-                    params_with_generics.iter_enumerated().filter(|&(idx, _)| {
-                        check_for_matched_generics
-                            || expected_idx
-                                .is_none_or(|expected_idx| expected_idx == idx.as_usize())
-                    })
-                {
-                    let Some(generic_param) = generic_param else {
-                        spans.push_span_label(param.span(), "");
-                        continue;
-                    };
-
-                    let other_params_matched: Vec<(ExpectedIdx, FnParam<'_>)> =
-                        params_with_generics
-                            .iter_enumerated()
-                            .filter(|&(other_idx, &(other_generic_param, _))| {
-                                if other_idx == idx {
-                                    return false;
-                                }
-                                let Some(other_generic_param) = other_generic_param else {
-                                    return false;
-                                };
-                                if matched_inputs[idx].is_none()
-                                    && matched_inputs[other_idx].is_none()
-                                {
-                                    return false;
-                                }
-                                if matched_inputs[idx].is_some()
-                                    && matched_inputs[other_idx].is_some()
-                                {
-                                    return false;
-                                }
-                                other_generic_param.name.ident() == generic_param.name.ident()
-                            })
-                            .map(|(other_idx, &(_, other_param))| (other_idx, other_param))
-                            .collect();
-
-                    if !other_params_matched.is_empty() {
-                        let other_param_matched_names: Vec<String> = other_params_matched
-                            .iter()
-                            .map(|(idx, other_param)| {
-                                if let Some(name) = other_param.name() {
-                                    format!("`{name}`")
+                } else {
+                    mismatched_params.extend(
+                        params_with_generics.iter_enumerated().zip(matched_inputs).filter_map(
+                            |((idx, &(generic, ref param)), matched_idx)| {
+                                if matched_idx.is_some() {
+                                    None
+                                } else if let Some(generic) = generic {
+                                    Some(MismatchedParam {
+                                        idx,
+                                        generic,
+                                        param,
+                                        deps: SmallVec::new(),
+                                    })
                                 } else {
-                                    format!("parameter #{}", idx.as_u32() + 1)
+                                    // Still mark mismatched parameters
+                                    spans.push_span_label(param.span(), "");
+                                    None
                                 }
-                            })
-                            .collect();
+                            },
+                        ),
+                    );
+                }
 
-                        let matched_ty = self
-                            .resolve_vars_if_possible(formal_and_expected_inputs[idx].1)
-                            .sort_string(self.tcx);
+                if !mismatched_params.is_empty() {
+                    // For each mismatched paramter, create a two-way link to each matched parameter
+                    // of the same type.
+                    let mut dependants = IndexVec::<ExpectedIdx, _>::from_fn_n(
+                        |_| SmallVec::<[u32; 4]>::new(),
+                        params_with_generics.len(),
+                    );
+                    let mut generic_uses = IndexVec::<GenericIdx, _>::from_fn_n(
+                        |_| SmallVec::<[ExpectedIdx; 4]>::new(),
+                        hir_generics.params.len(),
+                    );
+                    for (idx, param) in mismatched_params.iter_mut().enumerate() {
+                        for ((other_idx, &(other_generic, _)), &other_matched_idx) in
+                            params_with_generics.iter_enumerated().zip(matched_inputs)
+                        {
+                            if other_generic == Some(param.generic) && other_matched_idx.is_some() {
+                                generic_uses[param.generic].extend([param.idx, other_idx]);
+                                dependants[other_idx].push(idx as u32);
+                                param.deps.push(other_idx);
+                            }
+                        }
+                    }
 
-                        if matched_inputs[idx].is_some() {
+                    // Highlight each mismatched type along with a note about which other parameters
+                    // the type depends on (if any).
+                    for param in &mismatched_params {
+                        if let Some(deps_list) = listify(&param.deps, |&dep| {
+                            params_with_generics[dep].1.display(dep.as_usize()).to_string()
+                        }) {
                             spans.push_span_label(
-                                param.span(),
+                                param.param.span(),
                                 format!(
-                                    "{} need{} to match the {} type of this parameter",
-                                    listify(&other_param_matched_names, |n| n.to_string())
-                                        .unwrap_or_default(),
-                                    pluralize!(if other_param_matched_names.len() == 1 {
-                                        0
-                                    } else {
-                                        1
-                                    }),
-                                    matched_ty,
+                                    "this parameter needs to match the {} type of {deps_list}",
+                                    self.resolve_vars_if_possible(
+                                        formal_and_expected_inputs[param.deps[0]].1
+                                    )
+                                    .sort_string(self.tcx),
                                 ),
                             );
                         } else {
+                            // Still mark mismatched parameters
+                            spans.push_span_label(param.param.span(), "");
+                        }
+                    }
+                    // Highligh each parameter being depended on for a generic type.
+                    for ((&(_, param), deps), &(_, expected_ty)) in
+                        params_with_generics.iter().zip(&dependants).zip(formal_and_expected_inputs)
+                    {
+                        if let Some(deps_list) = listify(deps, |&dep| {
+                            let param = &mismatched_params[dep as usize];
+                            param.param.display(param.idx.as_usize()).to_string()
+                        }) {
                             spans.push_span_label(
                                 param.span(),
                                 format!(
-                                    "this parameter needs to match the {} type of {}",
-                                    matched_ty,
-                                    listify(&other_param_matched_names, |n| n.to_string())
-                                        .unwrap_or_default(),
+                                    "{deps_list} need{} to match the {} type of this parameter",
+                                    pluralize!((deps.len() != 1) as u32),
+                                    self.resolve_vars_if_possible(expected_ty)
+                                        .sort_string(self.tcx),
                                 ),
                             );
                         }
-                        generics_with_unmatched_params.push(generic_param);
-                    } else {
-                        spans.push_span_label(param.span(), "");
                     }
-                }
-
-                for generic_param in self
-                    .tcx
-                    .hir()
-                    .get_if_local(def_id)
-                    .and_then(|node| node.generics())
-                    .into_iter()
-                    .flat_map(|x| x.params)
-                    .filter(|x| {
-                        generics_with_unmatched_params
-                            .iter()
-                            .any(|y| x.name.ident() == y.name.ident())
-                    })
-                {
-                    let param_idents_matching: Vec<String> = params_with_generics
-                        .iter_enumerated()
-                        .filter(|&(_, &(generic, _))| {
-                            if let Some(generic) = generic {
-                                generic.name.ident() == generic_param.name.ident()
-                            } else {
-                                false
-                            }
-                        })
-                        .map(|(idx, &(_, param))| {
-                            if let Some(name) = param.name() {
-                                format!("`{name}`")
-                            } else {
-                                format!("parameter #{}", idx.as_u32() + 1)
-                            }
-                        })
-                        .collect();
-
-                    if !param_idents_matching.is_empty() {
-                        spans.push_span_label(
-                            generic_param.span,
-                            format!(
-                                "{} {} reference this parameter `{}`",
-                                listify(&param_idents_matching, |n| n.to_string())
-                                    .unwrap_or_default(),
-                                if param_idents_matching.len() == 2 { "both" } else { "all" },
-                                generic_param.name.ident().name,
-                            ),
-                        );
+                    // Highlight each generic parameter in use.
+                    for (param, uses) in hir_generics.params.iter().zip(&mut generic_uses) {
+                        uses.sort();
+                        uses.dedup();
+                        if let Some(param_list) = listify(uses, |&idx| {
+                            params_with_generics[idx].1.display(idx.as_usize()).to_string()
+                        }) {
+                            spans.push_span_label(
+                                param.span,
+                                format!(
+                                    "{param_list} {} reference this parameter `{}`",
+                                    if uses.len() == 2 { "both" } else { "all" },
+                                    param.name.ident().name,
+                                ),
+                            );
+                        }
                     }
                 }
             }
@@ -2614,7 +2583,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return;
         };
 
-        if let Some(params_with_generics) = self.get_hir_params_with_generics(def_id, is_method) {
+        if let Some((params_with_generics, _)) = self.get_hir_param_info(def_id, is_method) {
             debug_assert_eq!(params_with_generics.len(), matched_inputs.len());
             for (idx, (generic_param, _)) in params_with_generics.iter_enumerated() {
                 if matched_inputs[idx].is_none() {
@@ -2642,7 +2611,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         if matched_inputs[other_idx].is_some() {
                             return false;
                         }
-                        other_generic_param.name.ident() == generic_param.name.ident()
+                        other_generic_param == generic_param
                     })
                     .count();
 
@@ -2674,11 +2643,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// Returns the parameters of a function, with their generic parameters if those are the full
     /// type of that parameter. Returns `None` if the function has no generics or the body is
     /// unavailable (eg is an instrinsic).
-    fn get_hir_params_with_generics(
+    fn get_hir_param_info(
         &self,
         def_id: DefId,
         is_method: bool,
-    ) -> Option<IndexVec<ExpectedIdx, (Option<&hir::GenericParam<'_>>, FnParam<'_>)>> {
+    ) -> Option<(IndexVec<ExpectedIdx, (Option<GenericIdx>, FnParam<'_>)>, &hir::Generics<'_>)>
+    {
         let (sig, generics, body_id, param_names) = match self.tcx.hir().get_if_local(def_id)? {
             hir::Node::TraitItem(&hir::TraitItem {
                 generics,
@@ -2708,7 +2678,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 &hir::Path { res: Res::Def(_, res_def_id), .. },
             )) = param.kind
             {
-                generics.params.iter().find(|param| param.def_id.to_def_id() == res_def_id)
+                generics
+                    .params
+                    .iter()
+                    .position(|param| param.def_id.to_def_id() == res_def_id)
+                    .map(GenericIdx::from_usize)
             } else {
                 None
             }
@@ -2720,12 +2694,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let params =
                     params.get(is_method as usize..params.len() - sig.decl.c_variadic as usize)?;
                 debug_assert_eq!(params.len(), fn_inputs.len());
-                Some(fn_inputs.zip(params.iter().map(|param| FnParam::Param(param))).collect())
+                Some((
+                    fn_inputs.zip(params.iter().map(|param| FnParam::Param(param))).collect(),
+                    generics,
+                ))
             }
             (None, Some(params)) => {
                 let params = params.get(is_method as usize..)?;
                 debug_assert_eq!(params.len(), fn_inputs.len());
-                Some(fn_inputs.zip(params.iter().map(|param| FnParam::Name(param))).collect())
+                Some((
+                    fn_inputs.zip(params.iter().map(|param| FnParam::Name(param))).collect(),
+                    generics,
+                ))
             }
         }
     }
@@ -2773,4 +2753,18 @@ impl FnParam<'_> {
             _ => None,
         }
     }
+
+    fn display(&self, idx: usize) -> impl '_ + fmt::Display {
+        struct D<'a>(FnParam<'a>, usize);
+        impl fmt::Display for D<'_> {
+            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+                if let Some(name) = self.0.name() {
+                    write!(f, "`{name}`")
+                } else {
+                    write!(f, "parameter #{}", self.1 + 1)
+                }
+            }
+        }
+        D(*self, idx)
+    }
 }