Skip to content

Commit

Permalink
Auto merge of #68434 - varkor:astconv-mismatch-error, r=nikomatsakis
Browse files Browse the repository at this point in the history
Move generic arg/param validation to `create_substs_for_generic_args` to resolve various const generics issues

This changes some diagnostics, but I think they're around as helpful as the previous ones, and occur infrequently regardless.

Fixes #68257.
Fixes #68398.

r? @eddyb
  • Loading branch information
bors committed Feb 27, 2020
2 parents 49c68bd + bead79e commit 6d69cab
Show file tree
Hide file tree
Showing 25 changed files with 349 additions and 233 deletions.
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 @@ -596,23 +596,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 @@ -626,11 +618,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 @@ -653,42 +640,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 @@ -1016,24 +987,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 @@ -1070,7 +1023,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 @@ -1085,7 +1038,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
```

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

0 comments on commit 6d69cab

Please sign in to comment.