Skip to content

Commit

Permalink
Auto merge of rust-lang#74676 - lcnr:generics-no-sort, r=varkor
Browse files Browse the repository at this point in the history
correctly deal with unsorted generic parameters

We now stop sorting generic params and instead correctly handle unsorted params in the rest of the compiler.

We still restrict const params to come after type params though, so this PR does not change anything which
is visible to users.

This might slightly influence perf, so let's prevent any unintentional rollups. @bors rollup=never

r? @varkor
  • Loading branch information
bors committed Jul 24, 2020
2 parents 9008693 + 5f1eea9 commit cfb6114
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 41 deletions.
15 changes: 1 addition & 14 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -936,20 +936,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
})
});

let mut lowered_params: Vec<_> =
lowered_generics.params.into_iter().chain(in_band_defs).collect();

// FIXME(const_generics): the compiler doesn't always cope with
// unsorted generic parameters at the moment, so we make sure
// that they're ordered correctly here for now. (When we chain
// the `in_band_defs`, we might make the order unsorted.)
lowered_params.sort_by_key(|param| match param.kind {
hir::GenericParamKind::Lifetime { .. } => ParamKindOrd::Lifetime,
hir::GenericParamKind::Type { .. } => ParamKindOrd::Type,
hir::GenericParamKind::Const { .. } => ParamKindOrd::Const,
});

lowered_generics.params = lowered_params.into();
lowered_generics.params.extend(in_band_defs);

let lowered_generics = lowered_generics.into_generics(self.arena);
(lowered_generics, res)
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_resolve/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,10 @@ fn object_lifetime_defaults_for_item(
}
GenericParamKind::Const { .. } => {
// Generic consts don't impose any constraints.
None
//
// We still store a dummy value here to allow generic parameters
// in an arbitrary order.
Some(Set1::Empty)
}
})
.collect()
Expand Down
47 changes: 23 additions & 24 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1362,13 +1362,9 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
let type_start = own_start - has_self as u32 + params.len() as u32;
let mut i = 0;

// FIXME(const_generics): a few places in the compiler expect generic params
// to be in the order lifetimes, then type params, then const params.
//
// To prevent internal errors in case const parameters are supplied before
// type parameters we first add all type params, then all const params.
params.extend(ast_generics.params.iter().filter_map(|param| {
if let GenericParamKind::Type { ref default, synthetic, .. } = param.kind {
params.extend(ast_generics.params.iter().filter_map(|param| match param.kind {
GenericParamKind::Lifetime { .. } => None,
GenericParamKind::Type { ref default, synthetic, .. } => {
if !allow_defaults && default.is_some() {
if !tcx.features().default_type_parameter_fallback {
tcx.struct_span_lint_hir(
Expand All @@ -1378,7 +1374,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
|lint| {
lint.build(
"defaults for type parameters are only allowed in \
`struct`, `enum`, `type`, or `trait` definitions.",
`struct`, `enum`, `type`, or `trait` definitions.",
)
.emit();
},
Expand All @@ -1403,13 +1399,8 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
};
i += 1;
Some(param_def)
} else {
None
}
}));

params.extend(ast_generics.params.iter().filter_map(|param| {
if let GenericParamKind::Const { .. } = param.kind {
GenericParamKind::Const { .. } => {
let param_def = ty::GenericParamDef {
index: type_start + i as u32,
name: param.name.ident().name,
Expand All @@ -1419,8 +1410,6 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
};
i += 1;
Some(param_def)
} else {
None
}
}));

Expand Down Expand Up @@ -1899,14 +1888,24 @@ fn explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericPredicat
// Collect the predicates that were written inline by the user on each
// type parameter (e.g., `<T: Foo>`).
for param in ast_generics.params {
if let GenericParamKind::Type { .. } = param.kind {
let name = param.name.ident().name;
let param_ty = ty::ParamTy::new(index, name).to_ty(tcx);
index += 1;

let sized = SizedByDefault::Yes;
let bounds = AstConv::compute_bounds(&icx, param_ty, &param.bounds, sized, param.span);
predicates.extend(bounds.predicates(tcx, param_ty));
match param.kind {
// We already dealt with early bound lifetimes above.
GenericParamKind::Lifetime { .. } => (),
GenericParamKind::Type { .. } => {
let name = param.name.ident().name;
let param_ty = ty::ParamTy::new(index, name).to_ty(tcx);
index += 1;

let sized = SizedByDefault::Yes;
let bounds =
AstConv::compute_bounds(&icx, param_ty, &param.bounds, sized, param.span);
predicates.extend(bounds.predicates(tcx, param_ty));
}
GenericParamKind::Const { .. } => {
// Bounds on const parameters are currently not possible.
debug_assert!(param.bounds.is_empty());
index += 1;
}
}
}

Expand Down
12 changes: 11 additions & 1 deletion src/test/ui/const-generics/argument_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,14 @@ struct Bad<const N: usize, T> { //~ ERROR type parameters must be declared prior
another: T,
}

fn main() { }
struct AlsoBad<const N: usize, 'a, T, 'b, const M: usize, U> {
//~^ ERROR type parameters must be declared prior
//~| ERROR lifetime parameters must be declared prior
a: &'a T,
b: &'b U,
}

fn main() {
let _: AlsoBad<7, 'static, u32, 'static, 17, u16>;
//~^ ERROR lifetime provided when a type was expected
}
24 changes: 23 additions & 1 deletion src/test/ui/const-generics/argument_order.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@ error: type parameters must be declared prior to const parameters
LL | struct Bad<const N: usize, T> {
| -----------------^- help: reorder the parameters: lifetimes, then types, then consts: `<T, const N: usize>`

error: lifetime parameters must be declared prior to const parameters
--> $DIR/argument_order.rs:9:32
|
LL | struct AlsoBad<const N: usize, 'a, T, 'b, const M: usize, U> {
| -----------------^^-----^^-------------------- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T, U, const N: usize, const M: usize>`

error: type parameters must be declared prior to const parameters
--> $DIR/argument_order.rs:9:36
|
LL | struct AlsoBad<const N: usize, 'a, T, 'b, const M: usize, U> {
| ---------------------^----------------------^- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b, T, U, const N: usize, const M: usize>`

warning: the feature `const_generics` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/argument_order.rs:1:12
|
Expand All @@ -13,5 +25,15 @@ LL | #![feature(const_generics)]
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #44580 <https://github.com/rust-lang/rust/issues/44580> for more information

error: aborting due to previous error; 1 warning emitted
error[E0747]: lifetime provided when a type was expected
--> $DIR/argument_order.rs:17:23
|
LL | let _: AlsoBad<7, 'static, u32, 'static, 17, u16>;
| ^^^^^^^
|
= note: lifetime arguments must be provided before type arguments
= help: reorder the arguments: lifetimes, then types, then consts: `<'a, 'b, T, U, N, M>`

error: aborting due to 4 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0747`.

0 comments on commit cfb6114

Please sign in to comment.