Skip to content

Commit

Permalink
Implementation for 65853
Browse files Browse the repository at this point in the history
This attempts to bring better error messages to invalid method calls, by applying some heuristics to identify common mistakes.

The algorithm is inspired by Levenshtein distance and longest common sub-sequence.   In essence, we treat the types of the function, and the types of the arguments you provided as two "words" and compute the edits to get from one to the other.

We then modify that algorithm to detect 4 cases:

 - A function input is missing
 - An extra argument was provided
 - The type of an argument is straight up invalid
 - Two arguments have been swapped
 - A subset of the arguments have been shuffled

(We detect the last two as separate cases so that we can detect two swaps, instead of 4 parameters permuted.)

It helps to understand this argument by paying special attention to terminology: "inputs" refers to the inputs being *expected* by the function, and "arguments" refers to what has been provided at the call site.

The basic sketch of the algorithm is as follows:

 - Construct a boolean grid, with a row for each argument, and a column for each input.  The cell [i, j] is true if the i'th argument could satisfy the j'th input.
 - If we find an argument that could satisfy no inputs, provided for an input that can't be satisfied by any other argument, we consider this an "invalid type".
 - Extra arguments are those that can't satisfy any input, provided for an input that *could* be satisfied by another argument.
 - Missing inputs are inputs that can't be satisfied by any argument, where the provided argument could satisfy another input
 - Swapped / Permuted arguments are identified with a cycle detection algorithm.

As each issue is found, we remove the relevant inputs / arguments and check for more issues.  If we find no issues, we match up any "valid" arguments, and start again.

Note that there's a lot of extra complexity:
 - We try to stay efficient on the happy path, only computing the diagonal until we find a problem, and then filling in the rest of the matrix.
 - Closure arguments are wrapped in a tuple and need to be unwrapped
 - We need to resolve closure types after the rest, to allow the most specific type constraints
 - We need to handle imported C functions that might be variadic in their inputs.

I tried to document a lot of this in comments in the code and keep the naming clear.
  • Loading branch information
jackh726 committed Apr 16, 2022
1 parent e7575f9 commit b6c87c5
Show file tree
Hide file tree
Showing 180 changed files with 10,440 additions and 3,061 deletions.
9 changes: 5 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
mut values: Option<ValuePairs<'tcx>>,
terr: &TypeError<'tcx>,
swap_secondary_and_primary: bool,
force_label: bool,
) {
let span = cause.span(self.tcx);
debug!("note_type_err cause={:?} values={:?}, terr={:?}", cause, values, terr);
Expand Down Expand Up @@ -1623,7 +1624,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
TypeError::ObjectUnsafeCoercion(_) => {}
_ => {
let mut label_or_note = |span: Span, msg: &str| {
if &[span] == diag.span.primary_spans() {
if force_label || &[span] == diag.span.primary_spans() {
diag.span_label(span, msg);
} else {
diag.span_note(span, msg);
Expand Down Expand Up @@ -2171,7 +2172,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
struct_span_err!(self.tcx.sess, span, E0644, "{}", failure_str)
}
};
self.note_type_err(&mut diag, &trace.cause, None, Some(trace.values), terr, false);
self.note_type_err(&mut diag, &trace.cause, None, Some(trace.values), terr, false, false);
diag
}

Expand Down Expand Up @@ -2765,15 +2766,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

enum FailureCode {
pub enum FailureCode {
Error0038(DefId),
Error0317(&'static str),
Error0580(&'static str),
Error0308(&'static str),
Error0644(&'static str),
}

trait ObligationCauseExt<'tcx> {
pub trait ObligationCauseExt<'tcx> {
fn as_failure_code(&self, terr: &TypeError<'tcx>) -> FailureCode;
fn as_requirement_str(&self) -> &'static str;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,8 @@ impl<'tcx> ValuePairs<'tcx> {
/// See the `error_reporting` module for more details.
#[derive(Clone, Debug)]
pub struct TypeTrace<'tcx> {
cause: ObligationCause<'tcx>,
values: ValuePairs<'tcx>,
pub cause: ObligationCause<'tcx>,
pub values: ValuePairs<'tcx>,
}

/// The origin of a `r1 <= r2` constraint.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,15 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
}),
_ => None,
};
self.note_type_err(&mut diag, &obligation.cause, secondary_span, values, err, true);
self.note_type_err(
&mut diag,
&obligation.cause,
secondary_span,
values,
err,
true,
false,
);
self.note_obligation_cause(&mut diag, obligation);
diag.emit();
});
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
found,
expected,
None,
coercion_error,
Some(coercion_error),
);
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ fn compare_predicate_entailment<'tcx>(
})),
&terr,
false,
false,
);

return Err(diag.emit());
Expand Down Expand Up @@ -1074,6 +1075,7 @@ crate fn compare_const_impl<'tcx>(
})),
&terr,
false,
false,
);
diag.emit();
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr_ty: Ty<'tcx>,
expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
error: TypeError<'tcx>,
error: Option<TypeError<'tcx>>,
) {
self.annotate_expected_due_to_let_ty(err, expr, error);
self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr);
Expand Down Expand Up @@ -150,7 +150,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr_ty = self.resolve_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e.clone());

self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr, e);
self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr, Some(e));

(expected, Some(err))
}
Expand All @@ -159,7 +159,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'_>,
error: TypeError<'_>,
error: Option<TypeError<'_>>,
) {
let parent = self.tcx.hir().get_parent_node(expr.hir_id);
match (self.tcx.hir().find(parent), error) {
Expand All @@ -173,7 +173,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Assign(lhs, rhs, _), ..
})),
TypeError::Sorts(ExpectedFound { expected, .. }),
Some(TypeError::Sorts(ExpectedFound { expected, .. })),
) if rhs.hir_id == expr.hir_id && !expected.is_closure() => {
// We ignore closures explicitly because we already point at them elsewhere.
// Point at the assigned-to binding.
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

#[instrument(skip(self, expr), level = "debug")]
fn check_expr_kind(
pub(super) fn check_expr_kind(
&self,
expr: &'tcx hir::Expr<'tcx>,
expected: Expectation<'tcx>,
Expand Down Expand Up @@ -1367,11 +1367,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) {
let tcx = self.tcx;

let adt_ty_hint = self
.expected_inputs_for_expected_output(span, expected, adt_ty, &[adt_ty])
.get(0)
.cloned()
.unwrap_or(adt_ty);
let expected_inputs =
self.expected_inputs_for_expected_output(span, expected, adt_ty, &[adt_ty]);
let adt_ty_hint = if let Some(expected_inputs) = expected_inputs {
expected_inputs.get(0).cloned().unwrap_or(adt_ty)
} else {
adt_ty
};
// re-link the regions that EIfEO can erase.
self.demand_eqtype(span, adt_ty_hint, adt_ty);

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,9 +755,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected_ret: Expectation<'tcx>,
formal_ret: Ty<'tcx>,
formal_args: &[Ty<'tcx>],
) -> Vec<Ty<'tcx>> {
) -> Option<Vec<Ty<'tcx>>> {
let formal_ret = self.resolve_vars_with_obligations(formal_ret);
let Some(ret_ty) = expected_ret.only_has_type(self) else { return Vec::new() };
let Some(ret_ty) = expected_ret.only_has_type(self) else { return None };

// HACK(oli-obk): This is a hack to keep RPIT and TAIT in sync wrt their behaviour.
// Without it, the inference
Expand All @@ -779,7 +779,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let ty::subst::GenericArgKind::Type(ty) = ty.unpack() {
if let ty::Opaque(def_id, _) = *ty.kind() {
if self.infcx.opaque_type_origin(def_id, DUMMY_SP).is_some() {
return Vec::new();
return None;
}
}
}
Expand Down Expand Up @@ -820,7 +820,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Record all the argument types, with the substitutions
// produced from the above subtyping unification.
Ok(formal_args.iter().map(|&ty| self.resolve_vars_if_possible(ty)).collect())
Ok(Some(formal_args.iter().map(|&ty| self.resolve_vars_if_possible(ty)).collect()))
})
.unwrap_or_default();
debug!(?formal_args, ?formal_ret, ?expect_args, ?expected_ret);
Expand Down
Loading

0 comments on commit b6c87c5

Please sign in to comment.