Fix diagnostic regression

Author: varkor

src/librustc_passes/ast_validation.rs

@@ -539,7 +539,9 @@ impl<'a> Visitor<'a> for NestedImplTraitVisitor<'a> {
     fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
         match *generic_args {
             GenericArgs::AngleBracketed(ref data) => {
-                data.args.iter().for_each(|arg| self.visit_generic_arg(arg));
+                for arg in &data.args {
+                    self.visit_generic_arg(arg)
+                }
                 for type_binding in &data.bindings {
                     // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
                     // are allowed to contain nested `impl Trait`.

src/librustc_typeck/astconv.rs

@@ -94,11 +94,12 @@ struct ConvertedBinding<'tcx> {
 
 #[derive(PartialEq)]
 enum GenericArgPosition {
-    Datatype,
-    Function,
-    Method,
+    Type,
+    Value, // e.g. functions
+    MethodCall,
 }
 
+// FIXME(#53525): these error codes should all be unified.
 struct GenericArgMismatchErrorCode {
     lifetimes: (&'static str, &'static str),
     types: (&'static str, &'static str),
@@ -255,9 +256,9 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                 &empty_args
             },
             if is_method_call {
-                GenericArgPosition::Method
+                GenericArgPosition::MethodCall
             } else {
-                GenericArgPosition::Function
+                GenericArgPosition::Value
             },
             def.parent.is_none() && def.has_self, // `has_self`
             seg.infer_types || suppress_mismatch, // `infer_types`
@@ -285,7 +286,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
         // arguments in order to validate them with respect to the generic parameters.
         let param_counts = def.own_counts();
         let arg_counts = args.own_counts();
-        let infer_lifetimes = position != GenericArgPosition::Datatype && arg_counts.lifetimes == 0;
+        let infer_lifetimes = position != GenericArgPosition::Type && arg_counts.lifetimes == 0;
 
         let mut defaults: ty::GenericParamCount = Default::default();
         for param in &def.params {
@@ -297,7 +298,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
             };
         }
 
-        if position != GenericArgPosition::Datatype && !args.bindings.is_empty() {
+        if position != GenericArgPosition::Type && !args.bindings.is_empty() {
             AstConv::prohibit_assoc_ty_binding(tcx, args.bindings[0].span);
         }
 
@@ -308,7 +309,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                            if late bound lifetime parameters are present";
                 let note = "the late bound lifetime parameter is introduced here";
                 let span = args.args[0].span();
-                if position == GenericArgPosition::Function
+                if position == GenericArgPosition::Value
                     && arg_counts.lifetimes != param_counts.lifetimes {
                     let mut err = tcx.sess.struct_span_err(span, msg);
                     err.span_note(span_late, note);
@@ -328,7 +329,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                                 kind,
                                 required,
                                 permitted,
-                                provided| {
+                                provided,
+                                offset| {
             // We enforce the following: `required` <= `provided` <= `permitted`.
             // For kinds without defaults (i.e. lifetimes), `required == permitted`.
             // For other kinds (i.e. types), `permitted` may be greater than `required`.
@@ -348,8 +350,15 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                 (required, "")
             };
 
+            let mut span = span;
             let label = if required == permitted && provided > permitted {
                 let diff = provided - permitted;
+                if diff == 1 {
+                    // In the case when the user has provided too many arguments,
+                    // we want to point to the first unexpected argument.
+                    let first_superfluous_arg: &GenericArg = &args.args[offset + permitted];
+                    span = first_superfluous_arg.span();
+                }
                 format!(
                     "{}unexpected {} argument{}",
                     if diff != 1 { format!("{} ", diff) } else { String::new() },
@@ -394,6 +403,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                 param_counts.lifetimes,
                 param_counts.lifetimes,
                 arg_counts.lifetimes,
+                0,
             );
         }
         if !infer_types
@@ -404,6 +414,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                 param_counts.types - defaults.types - has_self as usize,
                 param_counts.types - has_self as usize,
                 arg_counts.types,
+                arg_counts.lifetimes,
             )
         } else {
             false
@@ -491,59 +502,50 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
                 // provided, matching them with the generic parameters we expect.
                 // Mismatches can occur as a result of elided lifetimes, or for malformed
                 // input. We try to handle both sensibly.
-                let mut progress_arg = true;
                 match (args.peek(), params.peek()) {
                     (Some(&arg), Some(&param)) => {
                         match (arg, &param.kind) {
-                            (GenericArg::Lifetime(_), GenericParamDefKind::Lifetime) => {
+                            (GenericArg::Lifetime(_), GenericParamDefKind::Lifetime)
+                            | (GenericArg::Type(_), GenericParamDefKind::Type { .. }) => {
                                 push_kind(&mut substs, provided_kind(param, arg));
+                                args.next();
                                 params.next();
                             }
                             (GenericArg::Lifetime(_), GenericParamDefKind::Type { .. }) => {
                                 // We expected a type argument, but got a lifetime
                                 // argument. This is an error, but we need to handle it
                                 // gracefully so we can report sensible errors. In this
-                                // case, we're simply going to infer the remaining
-                                // arguments.
-                                args.by_ref().for_each(drop); // Exhaust the iterator.
-                            }
-                            (GenericArg::Type(_), GenericParamDefKind::Type { .. }) => {
-                                push_kind(&mut substs, provided_kind(param, arg));
-                                params.next();
+                                // case, we're simply going to infer this argument.
+                                args.next();
                             }
                             (GenericArg::Type(_), GenericParamDefKind::Lifetime) => {
                                 // We expected a lifetime argument, but got a type
                                 // argument. That means we're inferring the lifetimes.
                                 push_kind(&mut substs, inferred_kind(None, param, infer_types));
                                 params.next();
-                                progress_arg = false;
                             }
                         }
                     }
                     (Some(_), None) => {
                         // We should never be able to reach this point with well-formed input.
                         // Getting to this point means the user supplied more arguments than
                         // there are parameters.
+                        args.next();
                     }
                     (None, Some(&param)) => {
                         // If there are fewer arguments than parameters, it means
                         // we're inferring the remaining arguments.
                         match param.kind {
-                            GenericParamDefKind::Lifetime => {
-                                push_kind(&mut substs, inferred_kind(None, param, infer_types));
-                            }
-                            GenericParamDefKind::Type { .. } => {
+                            GenericParamDefKind::Lifetime | GenericParamDefKind::Type { .. } => {
                                 let kind = inferred_kind(Some(&substs), param, infer_types);
                                 push_kind(&mut substs, kind);
                             }
                         }
+                        args.next();
                         params.next();
                     }
                     (None, None) => break,
                 }
-                if progress_arg {
-                    args.next();
-                }
             }
         }
 
@@ -582,12 +584,12 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
             span,
             &generic_params,
             &generic_args,
-            GenericArgPosition::Datatype,
+            GenericArgPosition::Type,
             has_self,
             infer_types,
             GenericArgMismatchErrorCode {
                 lifetimes: ("E0107", "E0107"),
-                types: ("E0243", "E0244"), // FIXME: E0243 and E0244 should be unified.
+                types: ("E0243", "E0244"),
             },
         );
 
@@ -616,17 +618,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
             |_| (Some(generic_args), infer_types),
             // Provide substitutions for parameters for which (valid) arguments have been provided.
             |param, arg| {
-                match param.kind {
-                    GenericParamDefKind::Lifetime => match arg {
-                        GenericArg::Lifetime(lt) => {
-                            self.ast_region_to_region(&lt, Some(param)).into()
-                        }
-                        _ => unreachable!(),
+                match (&param.kind, arg) {
+                    (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => {
+                        self.ast_region_to_region(&lt, Some(param)).into()
                     }
-                    GenericParamDefKind::Type { .. } => match arg {
-                        GenericArg::Type(ty) => self.ast_ty_to_ty(&ty).into(),
-                        _ => unreachable!(),
+                    (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
+                        self.ast_ty_to_ty(&ty).into()
                     }
+                    _ => unreachable!(),
                 }
             },
             // Provide substitutions for parameters for which arguments are inferred.

src/librustc_typeck/check/method/confirm.rs

@@ -344,17 +344,14 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
             },
             // Provide substitutions for parameters for which (valid) arguments have been provided.
             |param, arg| {
-                match param.kind {
-                    GenericParamDefKind::Lifetime => match arg {
-                        GenericArg::Lifetime(lt) => {
-                            AstConv::ast_region_to_region(self.fcx, lt, Some(param)).into()
-                        }
-                        _ => unreachable!(),
+                match (&param.kind, arg) {
+                    (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => {
+                        AstConv::ast_region_to_region(self.fcx, lt, Some(param)).into()
                     }
-                    GenericParamDefKind::Type { .. } => match arg {
-                        GenericArg::Type(ty) => self.to_ty(ty).into(),
-                        _ => unreachable!(),
+                    (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
+                        self.to_ty(ty).into()
                     }
+                    _ => unreachable!(),
                 }
             },
             // Provide substitutions for parameters for which arguments are inferred.

src/librustc_typeck/check/mod.rs

@@ -109,11 +109,11 @@ use session::{CompileIncomplete, config, Session};
 use TypeAndSubsts;
 use lint;
 use util::common::{ErrorReported, indenter};
-use util::nodemap::{DefIdMap, DefIdSet, FxHashMap, NodeMap};
+use util::nodemap::{DefIdMap, DefIdSet, FxHashMap, FxHashSet, NodeMap};
 
 use std::cell::{Cell, RefCell, Ref, RefMut};
 use rustc_data_structures::sync::Lrc;
-use std::collections::{hash_map::Entry, HashSet};
+use std::collections::hash_map::Entry;
 use std::cmp;
 use std::fmt::Display;
 use std::iter;
@@ -4908,7 +4908,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         // provided (if any) into their appropriate spaces. We'll also report
         // errors if type parameters are provided in an inappropriate place.
 
-        let mut generic_segs = HashSet::new();
+        let mut generic_segs = FxHashSet::default();
         for PathSeg(_, index) in &path_segs {
             generic_segs.insert(index);
         }
@@ -4937,24 +4937,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         // to add defaults. If the user provided *too many* types, that's
         // a problem.
 
-        let mut suppress_errors = FxHashMap();
+        let mut infer_args_for_err = FxHashSet::default();
         for &PathSeg(def_id, index) in &path_segs {
             let seg = &segments[index];
             let generics = self.tcx.generics_of(def_id);
-            // `impl Trait` is treated as a normal generic parameter internally,
-            // but we don't allow users to specify the parameter's value
-            // explicitly, so we have to do some error-checking here.
-            let suppress = AstConv::check_generic_arg_count_for_call(
+            // Argument-position `impl Trait` is treated as a normal generic
+            // parameter internally, but we don't allow users to specify the
+            // parameter's value explicitly, so we have to do some error-
+            // checking here.
+            let suppress_errors = AstConv::check_generic_arg_count_for_call(
                 self.tcx,
                 span,
                 &generics,
                 &seg,
                 false, // `is_method_call`
             );
-            if suppress {
+            if suppress_errors {
+                infer_args_for_err.insert(index);
                 self.set_tainted_by_errors(); // See issue #53251.
             }
-            suppress_errors.insert(index, suppress);
         }
 
         let has_self = path_segs.last().map(|PathSeg(def_id, _)| {
@@ -4976,7 +4977,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }) {
                     // If we've encountered an `impl Trait`-related error, we're just
                     // going to infer the arguments for better error messages.
-                    if !suppress_errors[&index] {
+                    if !infer_args_for_err.contains(&index) {
                         // Check whether the user has provided generic arguments.
                         if let Some(ref data) = segments[index].args {
                             return (Some(data), segments[index].infer_types);
@@ -4989,17 +4990,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             },
             // Provide substitutions for parameters for which (valid) arguments have been provided.
             |param, arg| {
-                match param.kind {
-                    GenericParamDefKind::Lifetime => match arg {
-                        GenericArg::Lifetime(lt) => {
-                            AstConv::ast_region_to_region(self, lt, Some(param)).into()
-                        }
-                        _ => unreachable!(),
+                match (&param.kind, arg) {
+                    (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => {
+                        AstConv::ast_region_to_region(self, lt, Some(param)).into()
                     }
-                    GenericParamDefKind::Type { .. } => match arg {
-                        GenericArg::Type(ty) => self.to_ty(ty).into(),
-                        _ => unreachable!(),
+                    (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
+                        self.to_ty(ty).into()
                     }
+                    _ => unreachable!(),
                 }
             },
             // Provide substitutions for parameters for which arguments are inferred.

src/test/ui/bad/bad-mid-path-type-params.stderr

@@ -1,26 +1,26 @@
 error[E0087]: wrong number of type arguments: expected 1, found 2
-  --> $DIR/bad-mid-path-type-params.rs:40:13
+  --> $DIR/bad-mid-path-type-params.rs:40:28
    |
 LL |     let _ = S::new::<isize,f64>(1, 1.0);
-   |             ^^^^^^^^^^^^^^^^^^^ unexpected type argument
+   |                            ^^^ unexpected type argument
 
 error[E0107]: wrong number of lifetime arguments: expected 0, found 1
-  --> $DIR/bad-mid-path-type-params.rs:43:13
+  --> $DIR/bad-mid-path-type-params.rs:43:17
    |
 LL |     let _ = S::<'a,isize>::new::<f64>(1, 1.0);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ unexpected lifetime argument
+   |                 ^^ unexpected lifetime argument
 
 error[E0087]: wrong number of type arguments: expected 1, found 2
-  --> $DIR/bad-mid-path-type-params.rs:46:17
+  --> $DIR/bad-mid-path-type-params.rs:46:36
    |
 LL |     let _: S2 = Trait::new::<isize,f64>(1, 1.0);
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^ unexpected type argument
+   |                                    ^^^ unexpected type argument
 
 error[E0088]: wrong number of lifetime arguments: expected 0, found 1
-  --> $DIR/bad-mid-path-type-params.rs:49:17
+  --> $DIR/bad-mid-path-type-params.rs:49:25
    |
 LL |     let _: S2 = Trait::<'a,isize>::new::<f64>(1, 1.0);
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unexpected lifetime argument
+   |                         ^^ unexpected lifetime argument
 
 error: aborting due to 4 previous errors
 

src/test/ui/constructor-lifetime-args.stderr

@@ -5,10 +5,10 @@ LL |     S::<'static>(&0, &0);
    |     ^^^^^^^^^^^^ expected 2 lifetime arguments
 
 error[E0088]: wrong number of lifetime arguments: expected 2, found 3
-  --> $DIR/constructor-lifetime-args.rs:29:5
+  --> $DIR/constructor-lifetime-args.rs:29:27
    |
 LL |     S::<'static, 'static, 'static>(&0, &0);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unexpected lifetime argument
+   |                           ^^^^^^^ unexpected lifetime argument
 
 error[E0090]: wrong number of lifetime arguments: expected 2, found 1
   --> $DIR/constructor-lifetime-args.rs:32:5
@@ -17,10 +17,10 @@ LL |     E::V::<'static>(&0);
    |     ^^^^^^^^^^^^^^^ expected 2 lifetime arguments
 
 error[E0088]: wrong number of lifetime arguments: expected 2, found 3
-  --> $DIR/constructor-lifetime-args.rs:34:5
+  --> $DIR/constructor-lifetime-args.rs:34:30
    |
 LL |     E::V::<'static, 'static, 'static>(&0);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unexpected lifetime argument
+   |                              ^^^^^^^ unexpected lifetime argument
 
 error: aborting due to 4 previous errors