Auto merge of rust-lang#68434 - varkor:astconv-mismatch-error, r=nikomatsakis

Move generic arg/param validation to `create_substs_for_generic_args` to resolve various const generics issues

This changes some diagnostics, but I think they're around as helpful as the previous ones, and occur infrequently regardless.

Fixes rust-lang#68257.
Fixes rust-lang#68398.

r? @eddyb

Author: bors

src/librustc/ty/mod.rs

@@ -931,7 +931,17 @@ pub enum GenericParamDefKind {
     Const,
 }
 
-#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)]
+impl GenericParamDefKind {
+    pub fn descr(&self) -> &'static str {
+        match self {
+            GenericParamDefKind::Lifetime => "lifetime",
+            GenericParamDefKind::Type { .. } => "type",
+            GenericParamDefKind::Const => "constant",
+        }
+    }
+}
+
+#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
 pub struct GenericParamDef {
     pub name: Symbol,
     pub def_id: DefId,

src/librustc/ty/structural_impls.rs

@@ -17,17 +17,6 @@ use std::fmt;
 use std::rc::Rc;
 use std::sync::Arc;
 
-impl fmt::Debug for ty::GenericParamDef {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let type_name = match self.kind {
-            ty::GenericParamDefKind::Lifetime => "Lifetime",
-            ty::GenericParamDefKind::Type { .. } => "Type",
-            ty::GenericParamDefKind::Const => "Const",
-        };
-        write!(f, "{}({}, {:?}, {})", type_name, self.name, self.def_id, self.index)
-    }
-}
-
 impl fmt::Debug for ty::TraitDef {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         ty::tls::with(|tcx| {

src/librustc_ast_passes/ast_validation.rs

@@ -8,7 +8,7 @@
 
 use rustc_ast_pretty::pprust;
 use rustc_data_structures::fx::FxHashMap;
-use rustc_errors::{error_code, struct_span_err, Applicability, FatalError};
+use rustc_errors::{error_code, struct_span_err, Applicability};
 use rustc_parse::validate_attr;
 use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY;
 use rustc_session::lint::LintBuffer;
@@ -596,23 +596,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 {
@@ -626,11 +618,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();
@@ -653,42 +640,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> {
@@ -1016,24 +987,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`.
@@ -1070,7 +1023,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| {
@@ -1085,7 +1038,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,20 @@
+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
+```
+
+The argument order should be changed to match the parameter declaration
+order, as in the following.
+
+```
+struct S<'a, T>(&'a T);
+
+type X = S<'static, ()>; // ok
+```

src/librustc_hir/hir.rs

@@ -298,6 +298,14 @@ impl GenericArg<'_> {
             _ => false,
         }
     }
+
+    pub fn descr(&self) -> &'static str {
+        match self {
+            GenericArg::Lifetime(_) => "lifetime",
+            GenericArg::Type(_) => "type",
+            GenericArg::Const(_) => "constant",
+        }
+    }
 }
 
 #[derive(RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]