Sort the errors from arguments checking so that suggestions are handled properly #112762
Conversation
|
r? @oli-obk (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| // sort errors with same type by the order they appear in the source | ||
| // so that suggestion will be handled properly, see #112507 | ||
| errors.sort_by(|a, b| match (a, b) { | ||
| (Error::Missing(i), Error::Missing(j)) => i.cmp(j), |
There was a problem hiding this comment.
I think you could probably just derive Ord on Error and just use .sort()?
There was a problem hiding this comment.
we need to customize PartialOrd for Error and I think only ordering on Missing and Extra is enough(which will involve adding or removing arguments).
There was a problem hiding this comment.
I don't think it hurts to just totally sort the list. I think right now the sort implementation is not actually valid because it returns Equal for other elements 😅
There was a problem hiding this comment.
yes, I just validated with some playground code, it won't sort in some cases, let me fix it 👍
There was a problem hiding this comment.
But actually, this implementation is enough, because it makes sure the consecutive Extra and Missing ordered 😆, which is the scenario that makes sub-suggestions buggy.
Anyway, let's make it all ordered.
| @@ -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 { | |||
There was a problem hiding this comment.
@compiler-errors do you know any better method for this?
I want the same Error type ordered into the same group.
There was a problem hiding this comment.
Isn't this the default for derive(PartialOrd)? I think it first sorts by enum discriminant and then by fields lexicographically.
|
r=me after changing to |
compiler-errors
added
S-waiting-on-author
|
Need to derive for too many other substructs such as @bors r=compiler-errors |
bors
added
S-waiting-on-bors
…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#112464 (Fix windows `Socket::connect_timeout` overflow) - rust-lang#112720 (Rustdoc: search: color item type and reduce size to avoid clashing) - rust-lang#112762 (Sort the errors from arguments checking so that suggestions are handled properly) - rust-lang#112786 (change binders from tuple structs to named fields) - rust-lang#112794 (Fix linker failures when #[global_allocator] is used in a dependency) - rust-lang#112819 (Disable feature(unboxed_closures, fn_traits) in weird-exprs) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #112507
The algorithm of
find_issuedoes not make sure the index comes out in order, which will make suggestingremoveoraddarguments broken in some cases.Modifying the algorithm to obey order involves much more trivial change, so it's better to order the
errorsafter iterations.