Move generic arg / param validation to create_substs_for_generic_args

Author: varkor

src/librustc_ast_passes/ast_validation.rs

@@ -594,23 +594,15 @@ impl<'a> AstValidator<'a> {
     }
 }
 
-enum GenericPosition {
-    Param,
-    Arg,
-}
-
-fn validate_generics_order<'a>(
+fn validate_generic_param_order<'a>(
     sess: &Session,
     handler: &rustc_errors::Handler,
     generics: impl Iterator<Item = (ParamKindOrd, Option<&'a [GenericBound]>, Span, Option<String>)>,
-    pos: GenericPosition,
     span: Span,
 ) {
     let mut max_param: Option<ParamKindOrd> = None;
     let mut out_of_order = FxHashMap::default();
     let mut param_idents = vec![];
-    let mut found_type = false;
-    let mut found_const = false;
 
     for (kind, bounds, span, ident) in generics {
         if let Some(ident) = ident {
@@ -624,11 +616,6 @@ fn validate_generics_order<'a>(
             }
             Some(_) | None => *max_param = Some(kind),
         };
-        match kind {
-            ParamKindOrd::Type => found_type = true,
-            ParamKindOrd::Const => found_const = true,
-            _ => {}
-        }
     }
 
     let mut ordered_params = "<".to_string();
@@ -651,42 +638,26 @@ fn validate_generics_order<'a>(
     }
     ordered_params += ">";
 
-    let pos_str = match pos {
-        GenericPosition::Param => "parameter",
-        GenericPosition::Arg => "argument",
-    };
-
     for (param_ord, (max_param, spans)) in &out_of_order {
-        let mut err = handler.struct_span_err(
-            spans.clone(),
-            &format!(
-                "{} {pos}s must be declared prior to {} {pos}s",
-                param_ord,
-                max_param,
-                pos = pos_str,
-            ),
-        );
-        if let GenericPosition::Param = pos {
-            err.span_suggestion(
-                span,
+        let mut err =
+            handler.struct_span_err(
+                spans.clone(),
                 &format!(
-                    "reorder the {}s: lifetimes, then types{}",
-                    pos_str,
-                    if sess.features_untracked().const_generics { ", then consts" } else { "" },
+                    "{} parameters must be declared prior to {} parameters",
+                    param_ord, max_param,
                 ),
-                ordered_params.clone(),
-                Applicability::MachineApplicable,
             );
-        }
+        err.span_suggestion(
+            span,
+            &format!(
+                "reorder the parameters: lifetimes, then types{}",
+                if sess.features_untracked().const_generics { ", then consts" } else { "" },
+            ),
+            ordered_params.clone(),
+            Applicability::MachineApplicable,
+        );
         err.emit();
     }
-
-    // FIXME(const_generics): we shouldn't have to abort here at all, but we currently get ICEs
-    // if we don't. Const parameters and type parameters can currently conflict if they
-    // are out-of-order.
-    if !out_of_order.is_empty() && found_type && found_const {
-        FatalError.raise();
-    }
 }
 
 impl<'a> Visitor<'a> for AstValidator<'a> {
@@ -1000,24 +971,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         match *generic_args {
             GenericArgs::AngleBracketed(ref data) => {
                 walk_list!(self, visit_generic_arg, &data.args);
-                validate_generics_order(
-                    self.session,
-                    self.err_handler(),
-                    data.args.iter().map(|arg| {
-                        (
-                            match arg {
-                                GenericArg::Lifetime(..) => ParamKindOrd::Lifetime,
-                                GenericArg::Type(..) => ParamKindOrd::Type,
-                                GenericArg::Const(..) => ParamKindOrd::Const,
-                            },
-                            None,
-                            arg.span(),
-                            None,
-                        )
-                    }),
-                    GenericPosition::Arg,
-                    generic_args.span(),
-                );
 
                 // Type bindings such as `Item = impl Debug` in `Iterator<Item = Debug>`
                 // are allowed to contain nested `impl Trait`.
@@ -1054,7 +1007,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             }
         }
 
-        validate_generics_order(
+        validate_generic_param_order(
             self.session,
             self.err_handler(),
             generics.params.iter().map(|param| {
@@ -1069,7 +1022,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 };
                 (kind, Some(&*param.bounds), param.ident.span, ident)
             }),
-            GenericPosition::Param,
             generics.span,
         );
 

src/librustc_error_codes/error_codes.rs

@@ -417,6 +417,7 @@ E0743: include_str!("./error_codes/E0743.md"),
 E0744: include_str!("./error_codes/E0744.md"),
 E0745: include_str!("./error_codes/E0745.md"),
 E0746: include_str!("./error_codes/E0746.md"),
+E0747: include_str!("./error_codes/E0747.md"),
 ;
 //  E0006, // merged with E0005
 //  E0008, // cannot bind by-move into a pattern guard

src/librustc_error_codes/error_codes/E0747.md

@@ -0,0 +1,10 @@
+Generic arguments must be provided in the same order as the corresponding generic
+parameters are declared.
+
+Erroneous code example:
+
+```compile_fail,E0747
+struct S<'a, T>(&'a T);
+
+type X = S<(), 'static>; // error: the type argument is provided before the lifetime argument
+```

src/librustc_typeck/astconv.rs

@@ -481,6 +481,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         parent_substs: &[subst::GenericArg<'tcx>],
         has_self: bool,
         self_ty: Option<Ty<'tcx>>,
+        arg_count_mismatch: bool,
         args_for_def_id: impl Fn(DefId) -> (Option<&'b GenericArgs<'b>>, bool),
         provided_kind: impl Fn(&GenericParamDef, &GenericArg<'_>) -> subst::GenericArg<'tcx>,
         mut inferred_kind: impl FnMut(
@@ -504,7 +505,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         // methods in `subst.rs`, so that we can iterate over the arguments and
         // parameters in lock-step linearly, instead of trying to match each pair.
         let mut substs: SmallVec<[subst::GenericArg<'tcx>; 8]> = SmallVec::with_capacity(count);
-
         // Iterate over each segment of the path.
         while let Some((def_id, defs)) = stack.pop() {
             let mut params = defs.params.iter().peekable();
@@ -541,6 +541,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             let mut args =
                 generic_args.iter().flat_map(|generic_args| generic_args.args.iter()).peekable();
 
+            let arg_kind = |arg| match arg {
+                &GenericArg::Lifetime(_) => "lifetime",
+                &GenericArg::Type(_) => "type",
+                &GenericArg::Const(_) => "constant",
+            };
+
+            // If we encounter a type or const when we expect a lifetime, we infer the lifetimes.
+            // If we later encounter a lifetime, we know that the arguments were provided in the
+            // wrong order. `force_infer_lt` records the type or const that forced lifetimes to be
+            // inferred, so we can use it for diagnostics later.
+            let mut force_infer_lt = None;
+
             loop {
                 // We're going to iterate through the generic arguments that the user
                 // provided, matching them with the generic parameters we expect.
@@ -561,28 +573,74 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                                 // We expected a lifetime argument, but got a type or const
                                 // argument. That means we're inferring the lifetimes.
                                 substs.push(inferred_kind(None, param, infer_args));
+                                force_infer_lt = Some(arg);
                                 params.next();
                             }
-                            (_, _) => {
+                            (_, kind) => {
                                 // We expected one kind of parameter, but the user provided
-                                // another. 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 this argument.
-                                args.next();
+                                // another. This is an error. However, if we already know that
+                                // the arguments don't match up with the parameters, we won't issue
+                                // an additional error, as the user already knows what's wrong.
+                                if !arg_count_mismatch {
+                                    let param_kind = match kind {
+                                        GenericParamDefKind::Lifetime => "lifetime",
+                                        GenericParamDefKind::Type { .. } => "type",
+                                        GenericParamDefKind::Const => "constant",
+                                    };
+                                    struct_span_err!(
+                                        tcx.sess,
+                                        arg.span(),
+                                        E0747,
+                                        "{} provided when a {} was expected",
+                                        arg_kind(arg),
+                                        param_kind,
+                                    )
+                                    .emit();
+                                }
+
+                                // We've reported the error, but we want to make sure that this
+                                // problem doesn't bubble down and create additional, irrelevant
+                                // errors. In this case, we're simply going to ignore the argument
+                                // and any following arguments. The rest of the parameters will be
+                                // inferred.
+                                while args.next().is_some() {}
                             }
                         }
                     }
-                    (Some(_), None) => {
+                    (Some(&arg), 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();
+                        // There are two situations in which we can encounter this issue.
+                        //
+                        //  1.  The number of arguments is incorrect. In this case, an error
+                        //      will already have been emitted, and we can ignore it. This case
+                        //      also occurs when late-bound lifetime parameters are present, yet
+                        //      the lifetime arguments have also been explicitly specified by the
+                        //      user.
+                        //  2.  We've inferred some lifetimes, which have been provided later (i.e.
+                        //      after a type or const). We want to throw an error in this case.
+
+                        if !arg_count_mismatch {
+                            let kind = arg_kind(arg);
+                            assert_eq!(kind, "lifetime");
+                            let provided =
+                                force_infer_lt.expect("lifetimes ought to have been inferred");
+                            struct_span_err!(
+                                tcx.sess,
+                                provided.span(),
+                                E0747,
+                                "{} provided when a {} was expected",
+                                arg_kind(provided),
+                                kind,
+                            )
+                            .emit();
+                        }
+
+                        break;
                     }
                     (None, Some(&param)) => {
                         // If there are fewer arguments than parameters, it means
                         // we're inferring the remaining arguments.
                         substs.push(inferred_kind(Some(&substs), param, infer_args));
-                        args.next();
                         params.next();
                     }
                     (None, None) => break,
@@ -658,7 +716,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             assert!(self_ty.is_none() && parent_substs.is_empty());
         }
 
-        let (_, potential_assoc_types) = Self::check_generic_arg_count(
+        let (arg_count_mismatch, potential_assoc_types) = Self::check_generic_arg_count(
             tcx,
             span,
             &generic_params,
@@ -691,6 +749,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             parent_substs,
             self_ty.is_some(),
             self_ty,
+            arg_count_mismatch,
             // Provide the generic args, and whether types should be inferred.
             |did| {
                 if did == def_id {

src/librustc_typeck/check/method/confirm.rs

@@ -299,7 +299,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         // If they were not explicitly supplied, just construct fresh
         // variables.
         let generics = self.tcx.generics_of(pick.item.def_id);
-        AstConv::check_generic_arg_count_for_call(
+        let arg_count_mismatch = AstConv::check_generic_arg_count_for_call(
             self.tcx, self.span, &generics, &seg, true, // `is_method_call`
         );
 
@@ -313,6 +313,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
             parent_substs,
             false,
             None,
+            arg_count_mismatch,
             // Provide the generic args, and whether types should be inferred.
             |def_id| {
                 // The last component of the returned tuple here is unimportant.

src/librustc_typeck/check/mod.rs

@@ -5520,6 +5520,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 &[][..],
                 has_self,
                 self_ty,
+                !infer_args_for_err.is_empty(),
                 // Provide the generic args, and whether types should be inferred.
                 |def_id| {
                     if let Some(&PathSeg(_, index)) =

src/librustc_typeck/collect.rs

@@ -1269,7 +1269,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics {
 
     let object_lifetime_defaults = tcx.object_lifetime_defaults(hir_id);
 
-    // Now create the real type parameters.
+    // Now create the real type and const parameters.
     let type_start = own_start - has_self as u32 + params.len() as u32;
     let mut i = 0;
     params.extend(ast_generics.params.iter().filter_map(|param| {

src/test/ui/const-generics/const-arg-type-arg-misordered.rs

@@ -0,0 +1,10 @@
+#![feature(const_generics)]
+//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash
+
+type Array<T, const N: usize> = [T; N];
+
+fn foo<const N: usize>() -> Array<N, ()> { //~ ERROR constant provided when a type was expected
+    unimplemented!()
+}
+
+fn main() {}

src/test/ui/const-generics/const-arg-type-arg-misordered.stderr

@@ -0,0 +1,17 @@
+warning: the feature `const_generics` is incomplete and may cause the compiler to crash
+  --> $DIR/const-arg-type-arg-misordered.rs:1:12
+   |
+LL | #![feature(const_generics)]
+   |            ^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+
+error[E0747]: constant provided when a type was expected
+  --> $DIR/const-arg-type-arg-misordered.rs:6:35
+   |
+LL | fn foo<const N: usize>() -> Array<N, ()> {
+   |                                   ^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0747`.

src/test/ui/const-generics/const-param-after-const-literal-arg.rs

@@ -0,0 +1,10 @@
+// check-pass
+
+#![feature(const_generics)]
+//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash
+
+struct Foo<const A: usize, const B: usize>;
+
+impl<const A: usize> Foo<1, A> {} // ok
+
+fn main() {}

src/test/ui/const-generics/const-param-after-const-literal-arg.stderr

@@ -0,0 +1,8 @@
+warning: the feature `const_generics` is incomplete and may cause the compiler to crash
+  --> $DIR/const-param-after-const-literal-arg.rs:3:12
+   |
+LL | #![feature(const_generics)]
+   |            ^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+

src/test/ui/const-generics/const-param-before-other-params.rs

@@ -1,4 +1,5 @@
 #![feature(const_generics)]
+//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash
 
 fn bar<const X: (), 'a>(_: &'a ()) {
     //~^ ERROR lifetime parameters must be declared prior to const parameters