Skip to content

Commit

Permalink
Rollup merge of #96823 - jackh726:params-heuristics-fix, r=estebank
Browse files Browse the repository at this point in the history 
Properly fix #96638

Closes #96638

The main part of this change is `Error::Invalid` now returns both the input and arg indices. However, I realized the code here was kind of confusing and not internally consistent (and thus I was having trouble getting the right behavior). So I've also switched `input_indices` and `arg_indices` to more closely match some naming in `checks` (although I think a more thorough cleanup there could be beneficial). I've added comments, but essentially `input_indices` refers to *user provided* inputs and `arg_indices` refers to *expected* args.
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored May 10, 2022
2 parents 7b32e93 + 1d68e6d commit 9a3f17b
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 86 deletions.
94 changes: 51 additions & 43 deletions compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::cmp;
use rustc_middle::ty::error::TypeError;

// An issue that might be found in the compatibility matrix
#[derive(Debug)]
enum Issue {
/// The given argument is the invalid type for the input
Invalid(usize),
Expand All @@ -23,9 +24,10 @@ pub(crate) enum Compatibility<'tcx> {
}

/// Similar to `Issue`, but contains some extra information
#[derive(Debug)]
pub(crate) enum Error<'tcx> {
/// The given argument is the invalid type for the input
Invalid(usize, Compatibility<'tcx>),
/// The provided argument is the invalid type for the expected input
Invalid(usize, usize, Compatibility<'tcx>), // provided, expected
/// There is a missing input
Missing(usize),
/// There's a superfluous argument
Expand All @@ -37,8 +39,15 @@ pub(crate) enum Error<'tcx> {
}

pub(crate) struct ArgMatrix<'tcx> {
/// Maps the indices in the `compatibility_matrix` rows to the indices of
/// the *user provided* inputs
input_indexes: Vec<usize>,
/// Maps the indices in the `compatibility_matrix` columns to the indices
/// of the *expected* args
arg_indexes: Vec<usize>,
/// The first dimension (rows) are the remaining user provided inputs to
/// match and the second dimension (cols) are the remaining expected args
/// to match
compatibility_matrix: Vec<Vec<Compatibility<'tcx>>>,
}

Expand All @@ -52,24 +61,24 @@ impl<'tcx> ArgMatrix<'tcx> {
.map(|i| (0..minimum_input_count).map(|j| is_compatible(i, j)).collect())
.collect();
ArgMatrix {
input_indexes: (0..minimum_input_count).collect(),
arg_indexes: (0..provided_arg_count).collect(),
input_indexes: (0..provided_arg_count).collect(),
arg_indexes: (0..minimum_input_count).collect(),
compatibility_matrix,
}
}

/// Remove a given input from consideration
fn eliminate_input(&mut self, idx: usize) {
self.input_indexes.remove(idx);
for row in &mut self.compatibility_matrix {
row.remove(idx);
}
self.compatibility_matrix.remove(idx);
}

/// Remove a given argument from consideration
fn eliminate_arg(&mut self, idx: usize) {
self.arg_indexes.remove(idx);
self.compatibility_matrix.remove(idx);
for row in &mut self.compatibility_matrix {
row.remove(idx);
}
}

/// "satisfy" an input with a given arg, removing both from consideration
Expand All @@ -78,21 +87,23 @@ impl<'tcx> ArgMatrix<'tcx> {
self.eliminate_arg(arg_idx);
}

// Returns a `Vec` of (user input, expected arg) of matched arguments. These
// are inputs on the remaining diagonal that match.
fn eliminate_satisfied(&mut self) -> Vec<(usize, usize)> {
let mut i = cmp::min(self.input_indexes.len(), self.arg_indexes.len());
let mut eliminated = vec![];
while i > 0 {
let idx = i - 1;
if matches!(self.compatibility_matrix[idx][idx], Compatibility::Compatible) {
eliminated.push((self.arg_indexes[idx], self.input_indexes[idx]));
eliminated.push((self.input_indexes[idx], self.arg_indexes[idx]));
self.satisfy_input(idx, idx);
}
i -= 1;
}
return eliminated;
}

// Check for the above mismatch cases
// Find some issue in the compatibility matrix
fn find_issue(&self) -> Option<Issue> {
let mat = &self.compatibility_matrix;
let ai = &self.arg_indexes;
Expand Down Expand Up @@ -121,26 +132,26 @@ impl<'tcx> ArgMatrix<'tcx> {
if is_arg {
for j in 0..ii.len() {
// If we find at least one input this argument could satisfy
// this argument isn't completely useless
if matches!(mat[i][j], Compatibility::Compatible) {
useless = false;
// this argument isn't unsatisfiable
if matches!(mat[j][i], Compatibility::Compatible) {
unsatisfiable = false;
break;
}
}
}
if is_input {
for j in 0..ai.len() {
// If we find at least one argument that could satisfy this input
// this argument isn't unsatisfiable
if matches!(mat[j][i], Compatibility::Compatible) {
unsatisfiable = false;
// this argument isn't useless
if matches!(mat[i][j], Compatibility::Compatible) {
useless = false;
break;
}
}
}

match (is_arg, is_input, useless, unsatisfiable) {
// If an input is unsatisfied, and the argument in its position is useless
match (is_input, is_arg, useless, unsatisfiable) {
// 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
Expand All @@ -167,7 +178,7 @@ impl<'tcx> ArgMatrix<'tcx> {
_ => {
continue;
}
};
}
}

// We didn't find any of the individual issues above, but
Expand Down Expand Up @@ -254,11 +265,11 @@ impl<'tcx> ArgMatrix<'tcx> {
// We'll want to know which arguments and inputs these rows and columns correspond to
// even after we delete them.
pub(crate) fn find_errors(mut self) -> (Vec<Error<'tcx>>, Vec<Option<usize>>) {
let provided_arg_count = self.arg_indexes.len();
let provided_arg_count = self.input_indexes.len();

let mut errors: Vec<Error<'tcx>> = vec![];
// For each expected argument, the matched *actual* input
let mut matched_inputs: Vec<Option<usize>> = vec![None; self.input_indexes.len()];
let mut matched_inputs: Vec<Option<usize>> = vec![None; self.arg_indexes.len()];

// Before we start looking for issues, eliminate any arguments that are already satisfied,
// so that an argument which is already spoken for by the input it's in doesn't
Expand All @@ -269,28 +280,28 @@ impl<'tcx> ArgMatrix<'tcx> {
// Without this elimination, the first argument causes the second argument
// to show up as both a missing input and extra argument, rather than
// just an invalid type.
for (arg, inp) in self.eliminate_satisfied() {
matched_inputs[inp] = Some(arg);
for (inp, arg) in self.eliminate_satisfied() {
matched_inputs[arg] = Some(inp);
}

while self.input_indexes.len() > 0 || self.arg_indexes.len() > 0 {
// Check for the first relevant issue
match self.find_issue() {
Some(Issue::Invalid(idx)) => {
let compatibility = self.compatibility_matrix[idx][idx].clone();
let input_idx = self.input_indexes[idx];
let arg_idx = self.arg_indexes[idx];
self.satisfy_input(idx, idx);
errors.push(Error::Invalid(input_idx, compatibility));
errors.push(Error::Invalid(input_idx, arg_idx, compatibility));
}
Some(Issue::Extra(idx)) => {
let arg_idx = self.arg_indexes[idx];
self.eliminate_arg(idx);
errors.push(Error::Extra(arg_idx));
}
Some(Issue::Missing(idx)) => {
let input_idx = self.input_indexes[idx];
self.eliminate_input(idx);
errors.push(Error::Missing(input_idx));
errors.push(Error::Extra(input_idx));
}
Some(Issue::Missing(idx)) => {
let arg_idx = self.arg_indexes[idx];
self.eliminate_arg(idx);
errors.push(Error::Missing(arg_idx));
}
Some(Issue::Swap(idx, other)) => {
let input_idx = self.input_indexes[idx];
Expand All @@ -302,24 +313,21 @@ impl<'tcx> ArgMatrix<'tcx> {
// Subtract 1 because we already removed the "min" row
self.satisfy_input(max - 1, min);
errors.push(Error::Swap(input_idx, other_input_idx, arg_idx, other_arg_idx));
matched_inputs[input_idx] = Some(other_arg_idx);
matched_inputs[other_input_idx] = Some(arg_idx);
matched_inputs[other_arg_idx] = Some(input_idx);
matched_inputs[arg_idx] = Some(other_input_idx);
}
Some(Issue::Permutation(args)) => {
// FIXME: If satisfy_input ever did anything non-trivial (emit obligations to help type checking, for example)
// we'd want to call this function with the correct arg/input pairs, but for now, we just throw them in a bucket.
// This works because they force a cycle, so each row is guaranteed to also be a column
let mut idxs: Vec<usize> = args.iter().filter_map(|&a| a).collect();

let mut real_idxs = vec![None; provided_arg_count];
for (src, dst) in
args.iter().enumerate().filter_map(|(src, dst)| dst.map(|dst| (src, dst)))
{
let src_arg = self.arg_indexes[src];
let dst_arg = self.arg_indexes[dst];
let dest_input = self.input_indexes[dst];
real_idxs[src_arg] = Some((dst_arg, dest_input));
matched_inputs[dest_input] = Some(src_arg);
let src_input_idx = self.input_indexes[src];
let dst_input_idx = self.input_indexes[dst];
let dest_arg_idx = self.arg_indexes[dst];
real_idxs[src_input_idx] = Some((dest_arg_idx, dst_input_idx));
matched_inputs[dest_arg_idx] = Some(src_input_idx);
}
idxs.sort();
idxs.reverse();
Expand All @@ -331,8 +339,8 @@ impl<'tcx> ArgMatrix<'tcx> {
None => {
// We didn't find any issues, so we need to push the algorithm forward
// First, eliminate any arguments that currently satisfy their inputs
for (arg, inp) in self.eliminate_satisfied() {
matched_inputs[inp] = Some(arg);
for (inp, arg) in self.eliminate_satisfied() {
matched_inputs[arg] = Some(inp);
}
}
};
Expand Down
Loading

0 comments on commit 9a3f17b

Please sign in to comment.