From 947b279bc6cc32d298b16f22b76bfd272b73b6bb Mon Sep 17 00:00:00 2001 From: max-heller Date: Wed, 30 Dec 2020 11:43:30 -0500 Subject: [PATCH] Take type defaults into account in suggestions to reorder generic parameters --- .../rustc_ast_passes/src/ast_validation.rs | 56 ++++++++++--------- .../defaults/complex-unord-param.min.stderr | 2 +- .../defaults/intermixed-lifetime.full.stderr | 4 +- .../defaults/intermixed-lifetime.min.stderr | 8 +-- .../defaults/needs-feature.min.stderr | 2 +- .../defaults/needs-feature.none.stderr | 2 +- .../defaults/simple-defaults.min.stderr | 2 +- ...ue-80512-param-reordering-with-defaults.rs | 4 ++ ...0512-param-reordering-with-defaults.stderr | 8 +++ 9 files changed, 53 insertions(+), 35 deletions(-) create mode 100644 src/test/ui/issues/issue-80512-param-reordering-with-defaults.rs create mode 100644 src/test/ui/issues/issue-80512-param-reordering-with-defaults.stderr diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 686300c7c5f..0fcffda309b 100644 --- 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, Span, Option)>, + generics: &[GenericParam], span: Span, ) { let mut max_param: Option = 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) = (¶m.kind, Some(&*param.bounds), param.ident.span); + let (ord_kind, ident) = match ¶m.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 ¶m.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 index cec56d7038a..8e8d26a0004 100644 --- 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 index 98352addaef..c4a666a829d 100644 --- 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(&'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(&'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 index 532f6d700b2..69a490978d1 100644 --- 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(&'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(&'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(&'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(&'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 index 86d6173fa02..a4006203e4a 100644 --- 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(T); - | -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `` + | -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `` 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 index 86d6173fa02..a4006203e4a 100644 --- 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(T); - | -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `` + | -----------------^----- help: reorder the parameters: lifetimes, then types, then consts: `` 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 index 01fb4210dd6..0746c64ac8c 100644 --- 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 new file mode 100644 index 00000000000..fe3e4fbc7e0 --- /dev/null +++ b/src/test/ui/issues/issue-80512-param-reordering-with-defaults.rs @@ -0,0 +1,4 @@ +#![crate_type = "lib"] + +struct S(&'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 new file mode 100644 index 00000000000..a1e9a903f81 --- /dev/null +++ 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(&'a T); + | ---------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T = ()>` + +error: aborting due to previous error +