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

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

Merged
merged 2 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {
Copy link
Member Author

Choose a reason for hiding this comment

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

@compiler-errors do you know any better method for this?
I want the same Error type ordered into the same group.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the default for derive(PartialOrd)? I think it first sorts by enum discriminant and then by fields lexicographically.

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`.