From 3829746ef9356be7ab766efcbc328aeb1d5a555f Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 30 Mar 2019 20:30:36 +0100 Subject: [PATCH 1/2] Include bounds in generic reordering diagnostic. This commit extends the existing generic re-ordering diagnostic to include any bounds on the generic parameter, thus producing correct suggestions. --- src/librustc_passes/ast_validation.rs | 37 ++++++++++++++++++--------- src/test/ui/issue-59508.fixed | 16 ++++++++++++ src/test/ui/issue-59508.rs | 16 ++++++++++++ src/test/ui/issue-59508.stderr | 8 ++++++ 4 files changed, 65 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/issue-59508.fixed create mode 100644 src/test/ui/issue-59508.rs create mode 100644 src/test/ui/issue-59508.stderr diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 4e2aefe623167..917564b17dfd0 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -361,7 +361,14 @@ enum GenericPosition { fn validate_generics_order<'a>( handler: &errors::Handler, - generics: impl Iterator)>, + generics: impl Iterator< + Item = ( + ParamKindOrd, + Option<&'a [GenericBound]>, + Span, + Option + ), + >, pos: GenericPosition, span: Span, ) { @@ -369,9 +376,9 @@ fn validate_generics_order<'a>( let mut out_of_order = FxHashMap::default(); let mut param_idents = vec![]; - for (kind, span, ident) in generics { + for (kind, bounds, span, ident) in generics { if let Some(ident) = ident { - param_idents.push((kind, param_idents.len(), ident)); + param_idents.push((kind, bounds, param_idents.len(), ident)); } let max_param = &mut max_param; match max_param { @@ -385,13 +392,19 @@ fn validate_generics_order<'a>( 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 (_, _, ident) in param_idents { + for (_, bounds, _, ident) in param_idents { if !first { ordered_params += ", "; } ordered_params += &ident; + if let Some(bounds) = bounds { + if !bounds.is_empty() { + ordered_params += ": "; + ordered_params += &pprust::bounds_to_string(&bounds); + } + } first = false; } } @@ -701,7 +714,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { GenericArg::Lifetime(..) => ParamKindOrd::Lifetime, GenericArg::Type(..) => ParamKindOrd::Type, GenericArg::Const(..) => ParamKindOrd::Const, - }, arg.span(), None) + }, None, arg.span(), None) }), GenericPosition::Arg, generic_args.span()); // Type bindings such as `Item=impl Debug` in `Iterator` @@ -736,16 +749,16 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } validate_generics_order(self.err_handler(), generics.params.iter().map(|param| { - let span = param.ident.span; let ident = Some(param.ident.to_string()); - match ¶m.kind { - GenericParamKind::Lifetime { .. } => (ParamKindOrd::Lifetime, span, ident), - GenericParamKind::Type { .. } => (ParamKindOrd::Type, span, ident), + let (kind, ident) = match ¶m.kind { + GenericParamKind::Lifetime { .. } => (ParamKindOrd::Lifetime, ident), + GenericParamKind::Type { .. } => (ParamKindOrd::Type, ident), GenericParamKind::Const { ref ty } => { let ty = pprust::ty_to_string(ty); - (ParamKindOrd::Const, span, Some(format!("const {}: {}", param.ident, ty))) + (ParamKindOrd::Const, Some(format!("const {}: {}", param.ident, ty))) } - } + }; + (kind, Some(&*param.bounds), param.ident.span, ident) }), GenericPosition::Param, generics.span); for predicate in &generics.where_clause.predicates { diff --git a/src/test/ui/issue-59508.fixed b/src/test/ui/issue-59508.fixed new file mode 100644 index 0000000000000..b5c60a1626f53 --- /dev/null +++ b/src/test/ui/issue-59508.fixed @@ -0,0 +1,16 @@ +// run-rustfix + +#![allow(dead_code)] + +// This test checks that generic parameter re-ordering diagnostic suggestions contain bounds. + +struct A; + +impl A { + pub fn do_things<'a, 'b: 'a, T>() { + //~^ ERROR lifetime parameters must be declared prior to type parameters + println!("panic"); + } +} + +fn main() {} diff --git a/src/test/ui/issue-59508.rs b/src/test/ui/issue-59508.rs new file mode 100644 index 0000000000000..0b39c5d8f2aec --- /dev/null +++ b/src/test/ui/issue-59508.rs @@ -0,0 +1,16 @@ +// run-rustfix + +#![allow(dead_code)] + +// This test checks that generic parameter re-ordering diagnostic suggestions contain bounds. + +struct A; + +impl A { + pub fn do_things() { + //~^ ERROR lifetime parameters must be declared prior to type parameters + println!("panic"); + } +} + +fn main() {} diff --git a/src/test/ui/issue-59508.stderr b/src/test/ui/issue-59508.stderr new file mode 100644 index 0000000000000..33e967cebffcc --- /dev/null +++ b/src/test/ui/issue-59508.stderr @@ -0,0 +1,8 @@ +error: lifetime parameters must be declared prior to type parameters + --> $DIR/issue-59508.rs:10:25 + | +LL | pub fn do_things() { + | ----^^--^^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b: 'a, T>` + +error: aborting due to previous error + From 0270d565d9f6287bce6a7e64e55aac245288541e Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 30 Mar 2019 20:57:04 +0100 Subject: [PATCH 2/2] Only mention const generics if enabled. This commit updates the generic parameter re-ordering diagnostic to only mention const generics if the feature is enabled. --- src/librustc_passes/ast_validation.rs | 57 ++++++++++++------- src/test/ui/issue-59508-1.rs | 18 ++++++ src/test/ui/issue-59508-1.stderr | 14 +++++ src/test/ui/issue-59508.stderr | 2 +- .../ui/lifetime-before-type-params.stderr | 8 +-- src/test/ui/parser/issue-14303-enum.stderr | 2 +- src/test/ui/parser/issue-14303-fn-def.stderr | 2 +- src/test/ui/parser/issue-14303-impl.stderr | 2 +- src/test/ui/parser/issue-14303-struct.stderr | 2 +- src/test/ui/parser/issue-14303-trait.stderr | 2 +- .../suggestions/suggest-move-lifetimes.stderr | 8 +-- 11 files changed, 83 insertions(+), 34 deletions(-) create mode 100644 src/test/ui/issue-59508-1.rs create mode 100644 src/test/ui/issue-59508-1.stderr diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 917564b17dfd0..5c325c55b619d 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -360,6 +360,7 @@ enum GenericPosition { } fn validate_generics_order<'a>( + sess: &Session, handler: &errors::Handler, generics: impl Iterator< Item = ( @@ -426,7 +427,11 @@ fn validate_generics_order<'a>( if let GenericPosition::Param = pos { err.span_suggestion( span, - &format!("reorder the {}s: lifetimes, then types, then consts", pos_str), + &format!( + "reorder the {}s: lifetimes, then types{}", + pos_str, + if sess.features_untracked().const_generics { ", then consts" } else { "" }, + ), ordered_params.clone(), Applicability::MachineApplicable, ); @@ -709,13 +714,19 @@ 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.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()); + 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` // are allowed to contain nested `impl Trait`. @@ -748,18 +759,24 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } } - validate_generics_order(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 { .. } => (ParamKindOrd::Type, ident), - GenericParamKind::Const { ref ty } => { - let ty = pprust::ty_to_string(ty); - (ParamKindOrd::Const, Some(format!("const {}: {}", param.ident, ty))) - } - }; - (kind, Some(&*param.bounds), param.ident.span, ident) - }), GenericPosition::Param, generics.span); + validate_generics_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 { .. } => (ParamKindOrd::Type, ident), + GenericParamKind::Const { ref ty } => { + let ty = pprust::ty_to_string(ty); + (ParamKindOrd::Const, Some(format!("const {}: {}", param.ident, ty))) + } + }; + (kind, Some(&*param.bounds), param.ident.span, ident) + }), + GenericPosition::Param, + generics.span, + ); for predicate in &generics.where_clause.predicates { if let WherePredicate::EqPredicate(ref predicate) = *predicate { diff --git a/src/test/ui/issue-59508-1.rs b/src/test/ui/issue-59508-1.rs new file mode 100644 index 0000000000000..4fbed9b08f215 --- /dev/null +++ b/src/test/ui/issue-59508-1.rs @@ -0,0 +1,18 @@ +#![allow(dead_code)] +#![feature(const_generics)] +//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash + +// This test checks that generic parameter re-ordering diagnostic suggestions mention that +// consts come after types and lifetimes when the `const_generics` feature is enabled. +// We cannot run rustfix on this test because of the above const generics warning. + +struct A; + +impl A { + pub fn do_things() { + //~^ ERROR lifetime parameters must be declared prior to type parameters + println!("panic"); + } +} + +fn main() {} diff --git a/src/test/ui/issue-59508-1.stderr b/src/test/ui/issue-59508-1.stderr new file mode 100644 index 0000000000000..8fb7d7c3c84dc --- /dev/null +++ b/src/test/ui/issue-59508-1.stderr @@ -0,0 +1,14 @@ +warning: the feature `const_generics` is incomplete and may cause the compiler to crash + --> $DIR/issue-59508-1.rs:2:12 + | +LL | #![feature(const_generics)] + | ^^^^^^^^^^^^^^ + +error: lifetime parameters must be declared prior to type parameters + --> $DIR/issue-59508-1.rs:12:25 + | +LL | pub fn do_things() { + | ----^^--^^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b: 'a, T>` + +error: aborting due to previous error + diff --git a/src/test/ui/issue-59508.stderr b/src/test/ui/issue-59508.stderr index 33e967cebffcc..c0fdb2ef34ac4 100644 --- a/src/test/ui/issue-59508.stderr +++ b/src/test/ui/issue-59508.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-59508.rs:10:25 | LL | pub fn do_things() { - | ----^^--^^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b: 'a, T>` + | ----^^--^^----- help: reorder the parameters: lifetimes, then types: `<'a, 'b: 'a, T>` error: aborting due to previous error diff --git a/src/test/ui/lifetime-before-type-params.stderr b/src/test/ui/lifetime-before-type-params.stderr index 3cef5db66c66f..ffc6784bafed8 100644 --- a/src/test/ui/lifetime-before-type-params.stderr +++ b/src/test/ui/lifetime-before-type-params.stderr @@ -2,25 +2,25 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/lifetime-before-type-params.rs:2:13 | LL | fn first() {} - | ----^^--^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | ----^^--^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: lifetime parameters must be declared prior to type parameters --> $DIR/lifetime-before-type-params.rs:4:18 | LL | fn second<'a, T, 'b>() {} - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: lifetime parameters must be declared prior to type parameters --> $DIR/lifetime-before-type-params.rs:6:16 | LL | fn third() {} - | -------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, U>` + | -------^^- help: reorder the parameters: lifetimes, then types: `<'a, T, U>` error: lifetime parameters must be declared prior to type parameters --> $DIR/lifetime-before-type-params.rs:8:18 | LL | fn fourth<'a, T, 'b, U, 'c, V>() {} - | --------^^-----^^---- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, 'c, T, U, V>` + | --------^^-----^^---- help: reorder the parameters: lifetimes, then types: `<'a, 'b, 'c, T, U, V>` error[E0601]: `main` function not found in crate `lifetime_before_type_params` | diff --git a/src/test/ui/parser/issue-14303-enum.stderr b/src/test/ui/parser/issue-14303-enum.stderr index bcecd75b1abba..46f16ea0cc41c 100644 --- a/src/test/ui/parser/issue-14303-enum.stderr +++ b/src/test/ui/parser/issue-14303-enum.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-14303-enum.rs:1:15 | LL | enum X<'a, T, 'b> { - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-fn-def.stderr b/src/test/ui/parser/issue-14303-fn-def.stderr index 082c37e0be795..8cbab4b9653a0 100644 --- a/src/test/ui/parser/issue-14303-fn-def.stderr +++ b/src/test/ui/parser/issue-14303-fn-def.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-14303-fn-def.rs:1:15 | LL | fn foo<'a, T, 'b>(x: &'a T) {} - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-impl.stderr b/src/test/ui/parser/issue-14303-impl.stderr index 3b5615d2a9eca..56cd4fb381038 100644 --- a/src/test/ui/parser/issue-14303-impl.stderr +++ b/src/test/ui/parser/issue-14303-impl.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-14303-impl.rs:3:13 | LL | impl<'a, T, 'b> X {} - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-struct.stderr b/src/test/ui/parser/issue-14303-struct.stderr index dbd0b987dd190..f31cb92ad66ce 100644 --- a/src/test/ui/parser/issue-14303-struct.stderr +++ b/src/test/ui/parser/issue-14303-struct.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-14303-struct.rs:1:17 | LL | struct X<'a, T, 'b> { - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-trait.stderr b/src/test/ui/parser/issue-14303-trait.stderr index 7dfa62d823fd8..0e7399102bf17 100644 --- a/src/test/ui/parser/issue-14303-trait.stderr +++ b/src/test/ui/parser/issue-14303-trait.stderr @@ -2,7 +2,7 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/issue-14303-trait.rs:1:18 | LL | trait Foo<'a, T, 'b> {} - | --------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T>` + | --------^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, T>` error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.stderr b/src/test/ui/suggestions/suggest-move-lifetimes.stderr index 657914d1c8c0c..1851c8deaa8b4 100644 --- a/src/test/ui/suggestions/suggest-move-lifetimes.stderr +++ b/src/test/ui/suggestions/suggest-move-lifetimes.stderr @@ -2,25 +2,25 @@ error: lifetime parameters must be declared prior to type parameters --> $DIR/suggest-move-lifetimes.rs:1:13 | LL | struct A { - | ----^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T>` + | ----^^- help: reorder the parameters: lifetimes, then types: `<'a, T>` error: lifetime parameters must be declared prior to type parameters --> $DIR/suggest-move-lifetimes.rs:5:13 | LL | struct B { - | ----^^---- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, U>` + | ----^^---- help: reorder the parameters: lifetimes, then types: `<'a, T, U>` error: lifetime parameters must be declared prior to type parameters --> $DIR/suggest-move-lifetimes.rs:10:16 | LL | struct C { - | -------^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, T, U>` + | -------^^- help: reorder the parameters: lifetimes, then types: `<'a, T, U>` error: lifetime parameters must be declared prior to type parameters --> $DIR/suggest-move-lifetimes.rs:15:16 | LL | struct D { - | -------^^--^^-----^^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, 'c, T, U, V>` + | -------^^--^^-----^^- help: reorder the parameters: lifetimes, then types: `<'a, 'b, 'c, T, U, V>` error: aborting due to 4 previous errors