]> git.lizzy.rs Git - rust.git/commitdiff
Properly fix issue 96638
authorJack Huey <31162821+jackh726@users.noreply.github.com>
Sat, 7 May 2022 18:12:01 +0000 (14:12 -0400)
committerJack Huey <31162821+jackh726@users.noreply.github.com>
Mon, 9 May 2022 17:53:16 +0000 (13:53 -0400)
compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs
compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

index 48a66e8026beedd0b129434fef0853b641a6f490..b7ba9d978784627a3752e6c374b067dc05d26538 100644 (file)
@@ -3,6 +3,7 @@
 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),
@@ -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
@@ -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>>>,
 }
 
@@ -52,8 +61,8 @@ pub(crate) fn new<F: FnMut(usize, usize) -> Compatibility<'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,
         }
     }
@@ -61,15 +70,15 @@ pub(crate) fn new<F: FnMut(usize, usize) -> Compatibility<'tcx>>(
     /// 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
@@ -78,13 +87,15 @@ fn satisfy_input(&mut self, input_idx: usize, arg_idx: usize) {
         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;
@@ -92,7 +103,7 @@ fn eliminate_satisfied(&mut self) -> Vec<(usize, usize)> {
         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;
@@ -121,9 +132,9 @@ fn find_issue(&self) -> Option<Issue> {
             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;
                     }
                 }
@@ -131,16 +142,16 @@ fn find_issue(&self) -> Option<Issue> {
             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
@@ -167,7 +178,7 @@ fn find_issue(&self) -> Option<Issue> {
                 _ => {
                     continue;
                 }
-            };
+            }
         }
 
         // We didn't find any of the individual issues above, but
@@ -254,11 +265,11 @@ fn find_issue(&self) -> Option<Issue> {
     // 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
@@ -269,28 +280,28 @@ pub(crate) fn find_errors(mut self) -> (Vec<Error<'tcx>>, Vec<Option<usize>>) {
         // 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];
@@ -302,24 +313,21 @@ pub(crate) fn find_errors(mut self) -> (Vec<Error<'tcx>>, Vec<Option<usize>>) {
                     // 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();
@@ -331,8 +339,8 @@ pub(crate) fn find_errors(mut self) -> (Vec<Error<'tcx>>, Vec<Option<usize>>) {
                 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);
                     }
                 }
             };
index 277743e4a46c19c3c20746509fc934b65204b3c1..847c2c32dba293f6487091932015837559d34954 100644 (file)
@@ -274,9 +274,9 @@ pub(in super::super) fn check_argument_types(
         // A "softer" version of the helper above, which checks types without persisting them,
         // and treats error types differently
         // This will allow us to "probe" for other argument orders that would likely have been correct
-        let check_compatible = |arg_idx, input_idx| {
-            let formal_input_ty: Ty<'tcx> = formal_input_tys[input_idx];
-            let expected_input_ty: Ty<'tcx> = expected_input_tys[input_idx];
+        let check_compatible = |input_idx, arg_idx| {
+            let formal_input_ty: Ty<'tcx> = formal_input_tys[arg_idx];
+            let expected_input_ty: Ty<'tcx> = expected_input_tys[arg_idx];
 
             // If either is an error type, we defy the usual convention and consider them to *not* be
             // coercible.  This prevents our error message heuristic from trying to pass errors into
@@ -285,7 +285,7 @@ pub(in super::super) fn check_argument_types(
                 return Compatibility::Incompatible(None);
             }
 
-            let provided_arg: &hir::Expr<'tcx> = &provided_args[arg_idx];
+            let provided_arg: &hir::Expr<'tcx> = &provided_args[input_idx];
             let expectation = Expectation::rvalue_hint(self, expected_input_ty);
             // FIXME: check that this is safe; I don't believe this commits any of the obligations, but I can't be sure.
             //
@@ -429,11 +429,11 @@ pub(in super::super) fn check_argument_types(
             let found_errors = !errors.is_empty();
 
             errors.drain_filter(|error| {
-                let Error::Invalid(input_idx, Compatibility::Incompatible(error)) = error else { return false };
-                let expected_ty = expected_input_tys[*input_idx];
-                let Some(Some((provided_ty, _))) = final_arg_types.get(*input_idx) else { return false };
+                let Error::Invalid(input_idx, arg_idx, Compatibility::Incompatible(error)) = error else { return false };
+                let expected_ty = expected_input_tys[*arg_idx];
+                let provided_ty = final_arg_types[*input_idx].map(|ty| ty.0).unwrap();
                 let cause = &self.misc(provided_args[*input_idx].span);
-                let trace = TypeTrace::types(cause, true, expected_ty, *provided_ty);
+                let trace = TypeTrace::types(cause, true, expected_ty, provided_ty);
                 if let Some(e) = error {
                     if !matches!(trace.cause.as_failure_code(e), FailureCode::Error0308(_)) {
                         self.report_and_explain_type_error(trace, e).emit();
@@ -562,11 +562,14 @@ pub(in super::super) fn check_argument_types(
 
             // Next special case: if there is only one "Incompatible" error, just emit that
             if errors.len() == 1 {
-                if let Some(Error::Invalid(input_idx, Compatibility::Incompatible(Some(error)))) =
-                    errors.iter().next()
+                if let Some(Error::Invalid(
+                    input_idx,
+                    arg_idx,
+                    Compatibility::Incompatible(Some(error)),
+                )) = errors.iter().next()
                 {
-                    let expected_ty = expected_input_tys[*input_idx];
-                    let provided_ty = final_arg_types[*input_idx].map(|ty| ty.0).unwrap();
+                    let expected_ty = expected_input_tys[*arg_idx];
+                    let provided_ty = final_arg_types[*arg_idx].map(|ty| ty.0).unwrap();
                     let expected_ty = self.resolve_vars_if_possible(expected_ty);
                     let provided_ty = self.resolve_vars_if_possible(provided_ty);
                     let cause = &self.misc(provided_args[*input_idx].span);
@@ -631,19 +634,13 @@ enum SuggestionText {
             let mut errors = errors.into_iter().peekable();
             while let Some(error) = errors.next() {
                 match error {
-                    Error::Invalid(input_idx, compatibility) => {
-                        let expected_ty = expected_input_tys[input_idx];
-                        let provided_ty = final_arg_types
-                            .get(input_idx)
-                            .and_then(|x| x.as_ref())
-                            .map(|ty| ty.0)
-                            .unwrap_or(tcx.ty_error());
+                    Error::Invalid(input_idx, arg_idx, compatibility) => {
+                        let expected_ty = expected_input_tys[arg_idx];
+                        let provided_ty = final_arg_types[input_idx].map(|ty| ty.0).unwrap();
                         let expected_ty = self.resolve_vars_if_possible(expected_ty);
                         let provided_ty = self.resolve_vars_if_possible(provided_ty);
                         if let Compatibility::Incompatible(error) = &compatibility {
-                            let cause = &self.misc(
-                                provided_args.get(input_idx).map(|i| i.span).unwrap_or(call_span),
-                            );
+                            let cause = &self.misc(provided_args[input_idx].span);
                             let trace = TypeTrace::types(cause, true, expected_ty, provided_ty);
                             if let Some(e) = error {
                                 self.note_type_err(
@@ -658,16 +655,14 @@ enum SuggestionText {
                             }
                         }
 
-                        if let Some(expr) = provided_args.get(input_idx) {
-                            self.emit_coerce_suggestions(
-                                &mut err,
-                                &expr,
-                                final_arg_types[input_idx].map(|ty| ty.0).unwrap(),
-                                final_arg_types[input_idx].map(|ty| ty.1).unwrap(),
-                                None,
-                                None,
-                            );
-                        }
+                        self.emit_coerce_suggestions(
+                            &mut err,
+                            &provided_args[input_idx],
+                            final_arg_types[input_idx].map(|ty| ty.0).unwrap(),
+                            final_arg_types[input_idx].map(|ty| ty.1).unwrap(),
+                            None,
+                            None,
+                        );
                     }
                     Error::Extra(arg_idx) => {
                         let arg_type = if let Some((_, ty)) = final_arg_types[arg_idx] {
@@ -843,12 +838,12 @@ enum SuggestionText {
                         }
                     }
                     Error::Swap(input_idx, other_input_idx, arg_idx, other_arg_idx) => {
-                        let first_span = provided_args[arg_idx].span;
-                        let second_span = provided_args[other_arg_idx].span;
+                        let first_span = provided_args[input_idx].span;
+                        let second_span = provided_args[other_input_idx].span;
 
                         let first_expected_ty =
-                            self.resolve_vars_if_possible(expected_input_tys[input_idx]);
-                        let first_provided_ty = if let Some((ty, _)) = final_arg_types[arg_idx] {
+                            self.resolve_vars_if_possible(expected_input_tys[arg_idx]);
+                        let first_provided_ty = if let Some((ty, _)) = final_arg_types[input_idx] {
                             format!(",found `{}`", ty)
                         } else {
                             "".into()
@@ -858,9 +853,9 @@ enum SuggestionText {
                             format!("expected `{}`{}", first_expected_ty, first_provided_ty),
                         ));
                         let other_expected_ty =
-                            self.resolve_vars_if_possible(expected_input_tys[other_input_idx]);
+                            self.resolve_vars_if_possible(expected_input_tys[other_arg_idx]);
                         let other_provided_ty =
-                            if let Some((ty, _)) = final_arg_types[other_arg_idx] {
+                            if let Some((ty, _)) = final_arg_types[other_input_idx] {
                                 format!(",found `{}`", ty)
                             } else {
                                 "".into()
@@ -926,14 +921,14 @@ enum SuggestionText {
                     "{}(",
                     source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| String::new())
                 );
-                for (idx, arg) in matched_inputs.iter().enumerate() {
-                    let suggestion_text = if let Some(arg) = arg {
-                        let arg_span = provided_args[*arg].span.source_callsite();
+                for (arg_index, input_idx) in matched_inputs.iter().enumerate() {
+                    let suggestion_text = if let Some(input_idx) = input_idx {
+                        let arg_span = provided_args[*input_idx].span.source_callsite();
                         let arg_text = source_map.span_to_snippet(arg_span).unwrap();
                         arg_text
                     } else {
                         // Propose a placeholder of the correct type
-                        let expected_ty = expected_input_tys[idx];
+                        let expected_ty = expected_input_tys[arg_index];
                         let input_ty = self.resolve_vars_if_possible(expected_ty);
                         if input_ty.is_unit() {
                             "()".to_string()
@@ -942,7 +937,7 @@ enum SuggestionText {
                         }
                     };
                     suggestion += &suggestion_text;
-                    if idx < minimum_input_count - 1 {
+                    if arg_index < minimum_input_count - 1 {
                         suggestion += ", ";
                     }
                 }