Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take type defaults into account in suggestions to reorder generic parameters #80519

Merged
merged 1 commit into from
Jan 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 31 additions & 25 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 += ", ";
}
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
max-heller marked this conversation as resolved.
Show resolved Hide resolved

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#![crate_type = "lib"]

struct S<T = (), 'a>(&'a T);
//~^ ERROR lifetime parameters must be declared prior to type parameters
Original file line number Diff line number Diff line change
@@ -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