Skip to content

Commit

Permalink
Rollup merge of #112762 - chenyukang:yukang-fix-112507-argument-check…
Browse files Browse the repository at this point in the history
…ing, r=compiler-errors

Sort the errors from arguments checking so that suggestions are handled properly

Fixes #112507

The algorithm of `find_issue` does not make sure the index comes out in order, which will make suggesting `remove` or `add` arguments broken in some cases.

Modifying the algorithm to obey order involves much more trivial change, so it's better to order the `errors` after iterations.
  • Loading branch information
GuillaumeGomez committed Jun 20, 2023
2 parents a318824 + aba1cf1 commit 6c5e212
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 5 deletions.
41 changes: 36 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/arg_matrix.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::cmp;

use core::cmp::Ordering;
use rustc_index::IndexVec;
use rustc_middle::ty::error::TypeError;
use std::cmp;

rustc_index::newtype_index! {
#[debug_format = "ExpectedIdx({})"]
Expand Down Expand Up @@ -34,14 +34,14 @@ enum Issue {
Permutation(Vec<Option<usize>>),
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) enum Compatibility<'tcx> {
Compatible,
Incompatible(Option<TypeError<'tcx>>),
}

/// Similar to `Issue`, but contains some extra information
#[derive(Debug)]
#[derive(Debug, PartialEq, Eq)]
pub(crate) enum Error<'tcx> {
/// The provided argument is the invalid type for the expected input
Invalid(ProvidedIdx, ExpectedIdx, Compatibility<'tcx>),
Expand All @@ -55,6 +55,34 @@ pub(crate) enum Error<'tcx> {
Permutation(Vec<(ExpectedIdx, ProvidedIdx)>),
}

impl Ord for Error<'_> {
fn cmp(&self, other: &Self) -> Ordering {
let key = |error: &Error<'_>| -> usize {
match error {
Error::Invalid(..) => 0,
Error::Extra(_) => 1,
Error::Missing(_) => 2,
Error::Swap(..) => 3,
Error::Permutation(..) => 4,
}
};
match (self, other) {
(Error::Invalid(a, _, _), Error::Invalid(b, _, _)) => a.cmp(b),
(Error::Extra(a), Error::Extra(b)) => a.cmp(b),
(Error::Missing(a), Error::Missing(b)) => a.cmp(b),
(Error::Swap(a, b, ..), Error::Swap(c, d, ..)) => a.cmp(c).then(b.cmp(d)),
(Error::Permutation(a), Error::Permutation(b)) => a.cmp(b),
_ => key(self).cmp(&key(other)),
}
}
}

impl PartialOrd for Error<'_> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

pub(crate) struct ArgMatrix<'tcx> {
/// Maps the indices in the `compatibility_matrix` rows to the indices of
/// the *user provided* inputs
Expand Down Expand Up @@ -177,7 +205,7 @@ impl<'tcx> ArgMatrix<'tcx> {
// If an argument is unsatisfied, and the input in its position is useless
// then the most likely explanation is that we just got the types wrong
(true, true, true, true) => return Some(Issue::Invalid(i)),
// Otherwise, if an input is useless, then indicate that this is an extra argument
// Otherwise, if an input is useless then indicate that this is an extra input
(true, _, true, _) => return Some(Issue::Extra(i)),
// Otherwise, if an argument is unsatisfiable, indicate that it's missing
(_, true, _, true) => return Some(Issue::Missing(i)),
Expand Down Expand Up @@ -376,6 +404,9 @@ impl<'tcx> ArgMatrix<'tcx> {
};
}

// sort errors with same type by the order they appear in the source
// so that suggestion will be handled properly, see #112507
errors.sort();
return (errors, matched_inputs);
}
}
12 changes: 12 additions & 0 deletions tests/ui/argument-suggestions/issue-112507.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pub enum Value {
Float(Option<f64>),
}

fn main() {
let _a = Value::Float( //~ ERROR this enum variant takes 1 argument but 4 arguments were supplied
0,
None,
None,
0,
);
}
27 changes: 27 additions & 0 deletions tests/ui/argument-suggestions/issue-112507.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0061]: this enum variant takes 1 argument but 4 arguments were supplied
--> $DIR/issue-112507.rs:6:14
|
LL | let _a = Value::Float(
| ^^^^^^^^^^^^
LL | 0,
| - unexpected argument of type `{integer}`
LL | None,
LL | None,
| ---- unexpected argument of type `Option<_>`
LL | 0,
| - unexpected argument of type `{integer}`
|
note: tuple variant defined here
--> $DIR/issue-112507.rs:2:5
|
LL | Float(Option<f64>),
| ^^^^^
help: remove the extra arguments
|
LL ~ ,
LL ~ None);
|

error: aborting due to previous error

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

0 comments on commit 6c5e212

Please sign in to comment.