Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when labeling arguments for call_once and friends #129320

Merged
merged 1 commit into from
Sep 13, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
@@ -58,7 +58,7 @@ pub(crate) fn check_legal_trait_for_method_call(
enum CallStep<'tcx> {
Builtin(Ty<'tcx>),
DeferredClosure(LocalDefId, ty::FnSig<'tcx>),
/// E.g., enum variant constructors.
/// Call overloading when callee implements one of the Fn* traits.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a drive-by change which I think is more accurate.

Overloaded(MethodCallee<'tcx>),
}

63 changes: 43 additions & 20 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
@@ -506,6 +506,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn_def_id,
call_span,
call_expr,
tuple_arguments,
);
}
}
@@ -520,6 +521,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn_def_id: Option<DefId>,
call_span: Span,
call_expr: &'tcx hir::Expr<'tcx>,
tuple_arguments: TupleArgumentsFlag,
) -> ErrorGuaranteed {
// Next, let's construct the error
let (error_span, call_ident, full_call_span, call_name, is_method) = match &call_expr.kind {
@@ -865,6 +867,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&matched_inputs,
&formal_and_expected_inputs,
is_method,
tuple_arguments,
);
suggest_confusable(&mut err);
return err.emit();
@@ -1001,6 +1004,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&matched_inputs,
&formal_and_expected_inputs,
is_method,
tuple_arguments,
);
suggest_confusable(&mut err);
return err.emit();
@@ -1448,6 +1452,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&matched_inputs,
&formal_and_expected_inputs,
is_method,
tuple_arguments,
);

// And add a suggestion block for all of the parameters
@@ -2219,21 +2224,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
matched_inputs: &IndexVec<ExpectedIdx, Option<ProvidedIdx>>,
formal_and_expected_inputs: &IndexVec<ExpectedIdx, (Ty<'tcx>, Ty<'tcx>)>,
is_method: bool,
tuple_arguments: TupleArgumentsFlag,
) {
let Some(mut def_id) = callable_def_id else {
return;
};

// 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)
// Possibly points at either impl or trait item, so try to get it
// to point to trait item, then get the parent.
// This parent might be an impl in the case of an inherent function,
// but the next check will fail.
// 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,
// so we fetch the parent and verify if it's a trait item.
&& let maybe_trait_item_def_id = assoc_item.trait_item_def_id.unwrap_or(def_id)
&& let maybe_trait_def_id = self.tcx.parent(maybe_trait_item_def_id)
// 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() {
@@ -2303,7 +2314,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
let mut spans: MultiSpan = def_span.into();

let params_with_generics = self.get_hir_params_with_generics(def_id, is_method);
if let Some(params_with_generics) = self.get_hir_params_with_generics(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 = || {
@@ -2324,7 +2338,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if matched_inputs[unmatching_idx.into()].is_none()
&& let Some(unmatched_idx_param_generic) =
params_with_generics[unmatching_idx].0
&& unmatched_idx_param_generic.name.ident() == generic.name.ident()
&& unmatched_idx_param_generic.name.ident()
== generic.name.ident()
{
// We found a parameter that didn't match that needed to
return true;
@@ -2377,7 +2392,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let other_param_matched_names: Vec<String> = other_params_matched
.iter()
.map(|(_, other_param)| {
if let hir::PatKind::Binding(_, _, ident, _) = other_param.pat.kind {
if let hir::PatKind::Binding(_, _, ident, _) = other_param.pat.kind
{
format!("`{ident}`")
} else {
"{unknown}".to_string()
@@ -2430,7 +2446,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.into_iter()
.flat_map(|x| x.params)
.filter(|x| {
generics_with_unmatched_params.iter().any(|y| x.name.ident() == y.name.ident())
generics_with_unmatched_params
.iter()
.any(|y| x.name.ident() == y.name.ident())
})
{
let param_idents_matching: Vec<String> = params_with_generics
@@ -2462,7 +2480,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}
}

}
err.span_note(spans, format!("{} defined here", self.tcx.def_descr(def_id)));
} else if let Some(hir::Node::Expr(e)) = self.tcx.hir().get_if_local(def_id)
&& let hir::ExprKind::Closure(hir::Closure { body, .. }) = &e.kind
@@ -2535,8 +2553,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
};

let params_with_generics = self.get_hir_params_with_generics(def_id, is_method);

if let Some(params_with_generics) = self.get_hir_params_with_generics(def_id, is_method) {
debug_assert_eq!(params_with_generics.len(), matched_inputs.len());
for (idx, (generic_param, _)) in params_with_generics.iter().enumerate() {
if matched_inputs[idx.into()].is_none() {
continue;
@@ -2591,18 +2609,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.span_label(*matched_arg_span, label);
}
}
}

/// Returns the parameters of a function, with their generic parameters if those are the full
/// type of that parameter. Returns `None` if the function body is unavailable (eg is an instrinsic).
fn get_hir_params_with_generics(
&self,
def_id: DefId,
is_method: bool,
) -> Vec<(Option<&hir::GenericParam<'_>>, &hir::Param<'_>)> {
let fn_node = self.tcx.hir().get_if_local(def_id);
) -> Option<Vec<(Option<&hir::GenericParam<'_>>, &hir::Param<'_>)>> {
let fn_node = self.tcx.hir().get_if_local(def_id)?;

let generic_params: Vec<Option<&hir::GenericParam<'_>>> = fn_node
.and_then(|node| node.fn_decl())
.fn_decl()?
.inputs
.into_iter()
.flat_map(|decl| decl.inputs)
.skip(if is_method { 1 } else { 0 })
.map(|param| {
if let hir::TyKind::Path(QPath::Resolved(
@@ -2611,7 +2632,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)) = param.kind
{
fn_node
.and_then(|node| node.generics())
.generics()
.into_iter()
.flat_map(|generics| generics.params)
.find(|param| &param.def_id.to_def_id() == res_def_id)
@@ -2621,14 +2642,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
.collect();

let params: Vec<&hir::Param<'_>> = fn_node
.and_then(|node| node.body_id())
let params: Vec<&hir::Param<'_>> = self
.tcx
.hir()
.body(fn_node.body_id()?)
.params
.into_iter()
.flat_map(|id| self.tcx.hir().body(id).params)
.skip(if is_method { 1 } else { 0 })
.collect();

generic_params.into_iter().zip(params).collect()
Some(generic_params.into_iter().zip_eq(params).collect())
}
}

5 changes: 0 additions & 5 deletions tests/crashes/128848.rs

This file was deleted.

10 changes: 10 additions & 0 deletions tests/ui/mismatched_types/mismatch-args-crash-issue-128848.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![feature(fn_traits)]

// Regression test for https://github.com/rust-lang/rust/issues/128848

fn f<T>(a: T, b: T, c: T) {
f.call_once()
//~^ ERROR this method takes 1 argument but 0 arguments were supplied
}

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/mismatched_types/mismatch-args-crash-issue-128848.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0061]: this method takes 1 argument but 0 arguments were supplied
--> $DIR/mismatch-args-crash-issue-128848.rs:6:7
|
LL | f.call_once()
| ^^^^^^^^^-- argument #1 of type `(_, _, _)` is missing
|
note: method defined here
--> $SRC_DIR/core/src/ops/function.rs:LL:COL
help: provide the argument
|
LL | f.call_once(/* args */)
| ~~~~~~~~~~~~

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0061`.