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

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

Merged
merged 12 commits into from
Feb 27, 2020
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
12 changes: 11 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 0 additions & 11 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
82 changes: 17 additions & 65 deletions src/librustc_ast_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -594,23 +594,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 {
Expand All @@ -624,11 +616,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();
Expand All @@ -651,42 +638,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> {
Expand Down Expand Up @@ -1000,24 +971,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`.
Expand Down Expand Up @@ -1054,7 +1007,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| {
Expand All @@ -1069,7 +1022,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
};
(kind, Some(&*param.bounds), param.ident.span, ident)
}),
GenericPosition::Param,
generics.span,
);

Expand Down
1 change: 1 addition & 0 deletions src/librustc_error_codes/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions src/librustc_error_codes/error_codes/E0747.md
Original file line number Diff line number Diff line change
@@ -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
```
varkor marked this conversation as resolved.
Show resolved Hide resolved

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
```
8 changes: 8 additions & 0 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Loading