Take type defaults into account in suggestions to reorder generic par…

…ameters
rust-lang · Dec 30, 2020 · 947b279 · 947b279
1 parent bbcaed0
commit 947b279
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 35 deletions.
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -717,35 +717,46 @@ impl<'a> AstValidator<'a> {
 
 /// Checks that generic parameters are in the correct order,
 /// which is lifetimes, then types and then consts. (`<'a, T, const N: usize>`)
-fn validate_generic_param_order<'a>(
+fn validate_generic_param_order(
     sess: &Session,
     handler: &rustc_errors::Handler,
-    generics: impl Iterator<Item = (ParamKindOrd, Option<&'a [GenericBound]>, Span, Option<String>)>,
+    generics: &[GenericParam],
     span: Span,
 ) {
     let mut max_param: Option<ParamKindOrd> = None;
     let mut out_of_order = FxHashMap::default();
     let mut param_idents = vec![];
 
-    for (kind, bounds, span, ident) in generics {
+    for param in generics {
+        let ident = Some(param.ident.to_string());
+        let (kind, bounds, span) = (&param.kind, Some(&*param.bounds), param.ident.span);
+        let (ord_kind, ident) = match &param.kind {
+            GenericParamKind::Lifetime => (ParamKindOrd::Lifetime, ident),
+            GenericParamKind::Type { default: _ } => (ParamKindOrd::Type, ident),
+            GenericParamKind::Const { ref ty, kw_span: _ } => {
+                let ty = pprust::ty_to_string(ty);
+                let unordered = sess.features_untracked().const_generics;
+                (ParamKindOrd::Const { unordered }, Some(format!("const {}: {}", param.ident, ty)))
+            }
+        };
         if let Some(ident) = ident {
-            param_idents.push((kind, bounds, param_idents.len(), ident));
+            param_idents.push((kind, ord_kind, bounds, param_idents.len(), ident));
         }
         let max_param = &mut max_param;
         match max_param {
-            Some(max_param) if *max_param > kind => {
-                let entry = out_of_order.entry(kind).or_insert((*max_param, vec![]));
+            Some(max_param) if *max_param > ord_kind => {
+                let entry = out_of_order.entry(ord_kind).or_insert((*max_param, vec![]));
                 entry.1.push(span);
             }
-            Some(_) | None => *max_param = Some(kind),
+            Some(_) | None => *max_param = Some(ord_kind),
         };
     }
 
     let mut ordered_params = "<".to_string();
     if !out_of_order.is_empty() {
-        param_idents.sort_by_key(|&(po, _, i, _)| (po, i));
+        param_idents.sort_by_key(|&(_, po, _, i, _)| (po, i));
         let mut first = true;
-        for (_, bounds, _, ident) in param_idents {
+        for (kind, _, bounds, _, ident) in param_idents {
             if !first {
                 ordered_params += ", ";
             }
@@ -756,6 +767,16 @@ fn validate_generic_param_order<'a>(
                     ordered_params += &pprust::bounds_to_string(&bounds);
                 }
             }
+            match kind {
+                GenericParamKind::Type { default: Some(default) } => {
+                    ordered_params += " = ";
+                    ordered_params += &pprust::ty_to_string(default);
+                }
+                GenericParamKind::Type { default: None } => (),
+                GenericParamKind::Lifetime => (),
+                // FIXME(const_generics:defaults)
+                GenericParamKind::Const { ty: _, kw_span: _ } => (),
+            }
             first = false;
         }
     }
@@ -1150,22 +1171,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         validate_generic_param_order(
             self.session,
             self.err_handler(),
-            generics.params.iter().map(|param| {
-                let ident = Some(param.ident.to_string());
-                let (kind, ident) = match &param.kind {
-                    GenericParamKind::Lifetime => (ParamKindOrd::Lifetime, ident),
-                    GenericParamKind::Type { default: _ } => (ParamKindOrd::Type, ident),
-                    GenericParamKind::Const { ref ty, kw_span: _ } => {
-                        let ty = pprust::ty_to_string(ty);
-                        let unordered = self.session.features_untracked().const_generics;
-                        (
-                            ParamKindOrd::Const { unordered },
-                            Some(format!("const {}: {}", param.ident, ty)),
-                        )
-                    }
-                };
-                (kind, Some(&*param.bounds), param.ident.span, ident)
-            }),
+            &generics.params,
             generics.span,
         );
 

diff --git a/src/test/ui/const-generics/defaults/complex-unord-param.min.stderr b/src/test/ui/const-generics/defaults/complex-unord-param.min.stderr
@@ -2,7 +2,7 @@ error: type parameters must be declared prior to const parameters
   --> $DIR/complex-unord-param.rs:8:41
    |
 LL | struct NestedArrays<'a, const N: usize, A: 'a, const M: usize, T:'a =u32> {
-   |                    ---------------------^----------------------^--------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, A: 'a, T: 'a, const N: usize, const M: usize>`
+   |                    ---------------------^----------------------^--------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, A: 'a, T: 'a = u32, const N: usize, const M: usize>`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/const-generics/defaults/intermixed-lifetime.full.stderr b/src/test/ui/const-generics/defaults/intermixed-lifetime.full.stderr
@@ -2,13 +2,13 @@ error: lifetime parameters must be declared prior to const parameters
   --> $DIR/intermixed-lifetime.rs:6:28
    |
 LL | struct Foo<const N: usize, 'a, T = u32>(&'a (), T);
-   |           -----------------^^---------- help: reorder the parameters: lifetimes, then consts and types: `<'a, const N: usize, T>`
+   |           -----------------^^---------- help: reorder the parameters: lifetimes, then consts and types: `<'a, const N: usize, T = u32>`
 
 error: lifetime parameters must be declared prior to type parameters
   --> $DIR/intermixed-lifetime.rs:10:37
    |
 LL | struct Bar<const N: usize, T = u32, 'a>(&'a (), T);
-   |           --------------------------^^- help: reorder the parameters: lifetimes, then consts and types: `<'a, const N: usize, T>`
+   |           --------------------------^^- help: reorder the parameters: lifetimes, then consts and types: `<'a, const N: usize, T = u32>`
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/const-generics/defaults/intermixed-lifetime.min.stderr b/src/test/ui/const-generics/defaults/intermixed-lifetime.min.stderr
@@ -2,25 +2,25 @@ error: lifetime parameters must be declared prior to const parameters
   --> $DIR/intermixed-lifetime.rs:6:28
    |
 LL | struct Foo<const N: usize, 'a, T = u32>(&'a (), T);
-   |           -----------------^^---------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, const N: usize>`
+   |           -----------------^^---------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = u32, const N: usize>`
 
 error: type parameters must be declared prior to const parameters
   --> $DIR/intermixed-lifetime.rs:6:32
    |
 LL | struct Foo<const N: usize, 'a, T = u32>(&'a (), T);
-   |           ---------------------^------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, const N: usize>`
+   |           ---------------------^------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = u32, const N: usize>`
 
 error: lifetime parameters must be declared prior to const parameters
   --> $DIR/intermixed-lifetime.rs:10:37
    |
 LL | struct Bar<const N: usize, T = u32, 'a>(&'a (), T);
-   |           --------------------------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, const N: usize>`
+   |           --------------------------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = u32, const N: usize>`
 
 error: type parameters must be declared prior to const parameters
   --> $DIR/intermixed-lifetime.rs:10:28
    |
 LL | struct Bar<const N: usize, T = u32, 'a>(&'a (), T);
-   |           -----------------^----------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, const N: usize>`
+   |           -----------------^----------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = u32, const N: usize>`
 
 error: aborting due to 4 previous errors
 
diff --git a/src/test/ui/const-generics/defaults/needs-feature.min.stderr b/src/test/ui/const-generics/defaults/needs-feature.min.stderr
@@ -2,7 +2,7 @@ error: type parameters must be declared prior to const parameters
   --> $DIR/needs-feature.rs:9:26
    |
 LL | struct A<const N: usize, T=u32>(T);
-   |         -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<T, const N: usize>`
+   |         -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<T = u32, const N: usize>`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/const-generics/defaults/needs-feature.none.stderr b/src/test/ui/const-generics/defaults/needs-feature.none.stderr
@@ -2,7 +2,7 @@ error: type parameters must be declared prior to const parameters
   --> $DIR/needs-feature.rs:9:26
    |
 LL | struct A<const N: usize, T=u32>(T);
-   |         -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<T, const N: usize>`
+   |         -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<T = u32, const N: usize>`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/const-generics/defaults/simple-defaults.min.stderr b/src/test/ui/const-generics/defaults/simple-defaults.min.stderr
@@ -2,7 +2,7 @@ error: type parameters must be declared prior to const parameters
   --> $DIR/simple-defaults.rs:8:40
    |
 LL | struct FixedOutput<'a, const N: usize, T=u32> {
-   |                   ---------------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, const N: usize>`
+   |                   ---------------------^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = u32, const N: usize>`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-80512-param-reordering-with-defaults.rs b/src/test/ui/issues/issue-80512-param-reordering-with-defaults.rs
@@ -0,0 +1,4 @@
+#![crate_type = "lib"]
+
+struct S<T = (), 'a>(&'a T);
+//~^ ERROR lifetime parameters must be declared prior to type parameters
diff --git a/src/test/ui/issues/issue-80512-param-reordering-with-defaults.stderr b/src/test/ui/issues/issue-80512-param-reordering-with-defaults.stderr
@@ -0,0 +1,8 @@
+error: lifetime parameters must be declared prior to type parameters
+  --> $DIR/issue-80512-param-reordering-with-defaults.rs:3:18
+   |
+LL | struct S<T = (), 'a>(&'a T);
+   |         ---------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = ()>`
+
+error: aborting due to previous error
+