Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rollup merge of #84299 - lcnr:const-generics-defaults-name-res, r=varkor
Browse files Browse the repository at this point in the history
various const parameter defaults improvements

Actually resolve names in const parameter defaults, fixing `struct Foo<const N: usize = { usize::MAX }>`.

---
Split generic parameter ban rib for types and consts, allowing
```rust
#![feature(const_generics_defaults)]
struct Q;
struct Foo<T = Q, const Q: usize = 3>(T);
```

---
Remove the type/const ordering restriction if `const_generics_defaults` is active, even if `const_generics` is not. allowing us to stabilize and test const param defaults separately.

---
Check well formedness of const parameter defaults, eagerly emitting an error for `struct Foo<const N: usize = { 0 - 1 }>`

---
Do not forbid const parameters in param defaults, allowing `struct Foo<const N: usize, T = [u8; N]>(T)` and `struct Foo<const N: usize, const M: usize = N>`. Note that this should not change anything which is stabilized, as on stable, type parameters must be in front of const parameters, which means that type parameter defaults are only allowed if no const parameters exist.

We still forbid generic parameters inside of const param types.

r? ``@varkor`` ``@petrochenkov``
Dylan-DPC authored Apr 22, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents cee9014 + d3e0d2f commit faa7dca
Showing 45 changed files with 386 additions and 218 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
@@ -754,7 +754,7 @@ fn validate_generic_param_order(
GenericParamKind::Type { default: _ } => (ParamKindOrd::Type, ident),
GenericParamKind::Const { ref ty, kw_span: _, default: _ } => {
let ty = pprust::ty_to_string(ty);
let unordered = sess.features_untracked().const_generics;
let unordered = sess.features_untracked().unordered_const_ty_params();
(ParamKindOrd::Const { unordered }, Some(format!("const {}: {}", param.ident, ty)))
}
};
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
@@ -63,6 +63,10 @@ macro_rules! declare_features {
_ => panic!("`{}` was not listed in `declare_features`", feature),
}
}

pub fn unordered_const_ty_params(&self) -> bool {
self.const_generics || self.const_generics_defaults
}
}
};
}
4 changes: 3 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
@@ -296,7 +296,9 @@ impl GenericArg<'_> {
match self {
GenericArg::Lifetime(_) => ast::ParamKindOrd::Lifetime,
GenericArg::Type(_) => ast::ParamKindOrd::Type,
GenericArg::Const(_) => ast::ParamKindOrd::Const { unordered: feats.const_generics },
GenericArg::Const(_) => {
ast::ParamKindOrd::Const { unordered: feats.unordered_const_ty_params() }
}
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@ impl GenericParamDefKind {
GenericParamDefKind::Lifetime => ast::ParamKindOrd::Lifetime,
GenericParamDefKind::Type { .. } => ast::ParamKindOrd::Type,
GenericParamDefKind::Const { .. } => {
ast::ParamKindOrd::Const { unordered: tcx.features().const_generics }
ast::ParamKindOrd::Const { unordered: tcx.features().unordered_const_ty_params() }
}
}
}
11 changes: 0 additions & 11 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -472,17 +472,6 @@ impl<'a> Resolver<'a> {
);
err
}
ResolutionError::ParamInAnonConstInTyDefault(name) => {
let mut err = self.session.struct_span_err(
span,
"constant values inside of type parameter defaults must not depend on generic parameters",
);
err.span_label(
span,
format!("the anonymous constant must not depend on the parameter `{}`", name),
);
err
}
ResolutionError::ParamInNonTrivialAnonConst { name, is_type } => {
let mut err = self.session.struct_span_err(
span,
69 changes: 40 additions & 29 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
@@ -555,18 +555,23 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
// provide previous type parameters as they're built. We
// put all the parameters on the ban list and then remove
// them one by one as they are processed and become available.
let mut default_ban_rib = Rib::new(ForwardGenericParamBanRibKind);
let mut found_default = false;
default_ban_rib.bindings.extend(generics.params.iter().filter_map(
|param| match param.kind {
GenericParamKind::Type { default: Some(_), .. }
| GenericParamKind::Const { default: Some(_), .. } => {
found_default = true;
Some((Ident::with_dummy_span(param.ident.name), Res::Err))
let mut forward_ty_ban_rib = Rib::new(ForwardGenericParamBanRibKind);
let mut forward_const_ban_rib = Rib::new(ForwardGenericParamBanRibKind);
for param in generics.params.iter() {
match param.kind {
GenericParamKind::Type { .. } => {
forward_ty_ban_rib
.bindings
.insert(Ident::with_dummy_span(param.ident.name), Res::Err);
}
_ => None,
},
));
GenericParamKind::Const { .. } => {
forward_const_ban_rib
.bindings
.insert(Ident::with_dummy_span(param.ident.name), Res::Err);
}
GenericParamKind::Lifetime => {}
}
}

// rust-lang/rust#61631: The type `Self` is essentially
// another type parameter. For ADTs, we consider it
@@ -579,7 +584,7 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
// such as in the case of `trait Add<Rhs = Self>`.)
if self.diagnostic_metadata.current_self_item.is_some() {
// (`Some` if + only if we are in ADT's generics.)
default_ban_rib.bindings.insert(Ident::with_dummy_span(kw::SelfUpper), Res::Err);
forward_ty_ban_rib.bindings.insert(Ident::with_dummy_span(kw::SelfUpper), Res::Err);
}

for param in &generics.params {
@@ -591,32 +596,38 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
}

if let Some(ref ty) = default {
self.ribs[TypeNS].push(default_ban_rib);
self.with_rib(ValueNS, ForwardGenericParamBanRibKind, |this| {
// HACK: We use an empty `ForwardGenericParamBanRibKind` here which
// is only used to forbid the use of const parameters inside of
// type defaults.
//
// While the rib name doesn't really fit here, it does allow us to use the same
// code for both const and type parameters.
this.visit_ty(ty);
});
default_ban_rib = self.ribs[TypeNS].pop().unwrap();
self.ribs[TypeNS].push(forward_ty_ban_rib);
self.ribs[ValueNS].push(forward_const_ban_rib);
self.visit_ty(ty);
forward_const_ban_rib = self.ribs[ValueNS].pop().unwrap();
forward_ty_ban_rib = self.ribs[TypeNS].pop().unwrap();
}

// Allow all following defaults to refer to this type parameter.
default_ban_rib.bindings.remove(&Ident::with_dummy_span(param.ident.name));
forward_ty_ban_rib.bindings.remove(&Ident::with_dummy_span(param.ident.name));
}
GenericParamKind::Const { ref ty, kw_span: _, default: _ } => {
// FIXME(const_generics_defaults): handle `default` value here
for bound in &param.bounds {
self.visit_param_bound(bound);
}
GenericParamKind::Const { ref ty, kw_span: _, ref default } => {
// Const parameters can't have param bounds.
assert!(param.bounds.is_empty());

self.ribs[TypeNS].push(Rib::new(ConstParamTyRibKind));
self.ribs[ValueNS].push(Rib::new(ConstParamTyRibKind));
self.visit_ty(ty);
self.ribs[TypeNS].pop().unwrap();
self.ribs[ValueNS].pop().unwrap();

if let Some(ref expr) = default {
self.ribs[TypeNS].push(forward_ty_ban_rib);
self.ribs[ValueNS].push(forward_const_ban_rib);
self.visit_anon_const(expr);
forward_const_ban_rib = self.ribs[ValueNS].pop().unwrap();
forward_ty_ban_rib = self.ribs[TypeNS].pop().unwrap();
}

// Allow all following defaults to refer to this const parameter.
forward_const_ban_rib
.bindings
.remove(&Ident::with_dummy_span(param.ident.name));
}
}
}
58 changes: 8 additions & 50 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
@@ -239,8 +239,6 @@ enum ResolutionError<'a> {
ForwardDeclaredTyParam, // FIXME(const_generics_defaults)
/// ERROR E0770: the type of const parameters must not depend on other generic parameters.
ParamInTyOfConstParam(Symbol),
/// constant values inside of type parameter defaults must not depend on generic parameters.
ParamInAnonConstInTyDefault(Symbol),
/// generic parameters must not be used inside const evaluations.
///
/// This error is only emitted when using `min_const_generics`.
@@ -2672,26 +2670,18 @@ impl<'a> Resolver<'a> {
}
}
Res::Def(DefKind::TyParam, _) | Res::SelfTy(..) => {
let mut in_ty_param_default = false;
for rib in ribs {
let has_generic_params = match rib.kind {
let has_generic_params: HasGenericParams = match rib.kind {
NormalRibKind
| ClosureOrAsyncRibKind
| AssocItemRibKind
| ModuleRibKind(..)
| MacroDefinition(..) => {
| MacroDefinition(..)
| ForwardGenericParamBanRibKind => {
// Nothing to do. Continue.
continue;
}

// We only forbid constant items if we are inside of type defaults,
// for example `struct Foo<T, U = [u8; std::mem::size_of::<T>()]>`
ForwardGenericParamBanRibKind => {
// FIXME(const_generic_defaults): we may need to distinguish between
// being in type parameter defaults and const parameter defaults
in_ty_param_default = true;
continue;
}
ConstantItemRibKind(trivial, _) => {
let features = self.session.features_untracked();
// HACK(min_const_generics): We currently only allow `N` or `{ N }`.
@@ -2720,19 +2710,7 @@ impl<'a> Resolver<'a> {
}
}

if in_ty_param_default {
if record_used {
self.report_error(
span,
ResolutionError::ParamInAnonConstInTyDefault(
rib_ident.name,
),
);
}
return Res::Err;
} else {
continue;
}
continue;
}

// This was an attempt to use a type parameter outside its scope.
@@ -2770,23 +2748,15 @@ impl<'a> Resolver<'a> {
ribs.next();
}

let mut in_ty_param_default = false;
for rib in ribs {
let has_generic_params = match rib.kind {
NormalRibKind
| ClosureOrAsyncRibKind
| AssocItemRibKind
| ModuleRibKind(..)
| MacroDefinition(..) => continue,

// We only forbid constant items if we are inside of type defaults,
// for example `struct Foo<T, U = [u8; std::mem::size_of::<T>()]>`
ForwardGenericParamBanRibKind => {
// FIXME(const_generic_defaults): we may need to distinguish between
// being in type parameter defaults and const parameter defaults
in_ty_param_default = true;
continue;
}
| MacroDefinition(..)
| ForwardGenericParamBanRibKind => continue,

ConstantItemRibKind(trivial, _) => {
let features = self.session.features_untracked();
// HACK(min_const_generics): We currently only allow `N` or `{ N }`.
@@ -2808,19 +2778,7 @@ impl<'a> Resolver<'a> {
return Res::Err;
}

if in_ty_param_default {
if record_used {
self.report_error(
span,
ResolutionError::ParamInAnonConstInTyDefault(
rib_ident.name,
),
);
}
return Res::Err;
} else {
continue;
}
continue;
}

ItemRibKind(has_generic_params) => has_generic_params,
6 changes: 4 additions & 2 deletions compiler/rustc_typeck/src/astconv/generics.rs
Original file line number Diff line number Diff line change
@@ -286,7 +286,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
ParamKindOrd::Const {
unordered: tcx
.features()
.const_generics,
.unordered_const_ty_params(),
}
}
},
@@ -309,7 +309,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
GenericArg::Lifetime(_) => ParamKindOrd::Lifetime,
GenericArg::Type(_) => ParamKindOrd::Type,
GenericArg::Const(_) => ParamKindOrd::Const {
unordered: tcx.features().const_generics,
unordered: tcx
.features()
.unordered_const_ty_params(),
},
}),
Some(&format!(
4 changes: 3 additions & 1 deletion compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
@@ -513,7 +513,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
GenericParamDefKind::Const { has_default } => {
let ty = tcx.at(self.span).type_of(param.def_id);
if !infer_args && has_default {
tcx.const_param_default(param.def_id).into()
tcx.const_param_default(param.def_id)
.subst_spanned(tcx, substs.unwrap(), Some(self.span))
.into()
} else {
if infer_args {
self.astconv.ct_infer(ty, Some(param), self.span).into()
4 changes: 3 additions & 1 deletion compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -1446,7 +1446,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
GenericParamDefKind::Const { has_default, .. } => {
if !infer_args && has_default {
tcx.const_param_default(param.def_id).into()
tcx.const_param_default(param.def_id)
.subst_spanned(tcx, substs.unwrap(), Some(self.span))
.into()
} else {
self.fcx.var_for_def(self.span, param)
}
53 changes: 40 additions & 13 deletions compiler/rustc_typeck/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -728,20 +728,36 @@ fn check_where_clauses<'tcx, 'fcx>(
//
// Here, the default `Vec<[u32]>` is not WF because `[u32]: Sized` does not hold.
for param in &generics.params {
if let GenericParamDefKind::Type { .. } = param.kind {
if is_our_default(&param) {
let ty = fcx.tcx.type_of(param.def_id);
// Ignore dependent defaults -- that is, where the default of one type
// parameter includes another (e.g., `<T, U = T>`). In those cases, we can't
// be sure if it will error or not as user might always specify the other.
if !ty.needs_subst() {
match param.kind {
GenericParamDefKind::Type { .. } => {
if is_our_default(&param) {
let ty = fcx.tcx.type_of(param.def_id);
// Ignore dependent defaults -- that is, where the default of one type
// parameter includes another (e.g., `<T, U = T>`). In those cases, we can't
// be sure if it will error or not as user might always specify the other.
if !ty.needs_subst() {
fcx.register_wf_obligation(
ty.into(),
fcx.tcx.def_span(param.def_id),
ObligationCauseCode::MiscObligation,
);
}
}
}
GenericParamDefKind::Const { .. } => {
// FIXME(const_generics_defaults): Figure out if this
// is the behavior we want, see the comment further below.
if is_our_default(&param) {
let default_ct = tcx.const_param_default(param.def_id);
fcx.register_wf_obligation(
ty.into(),
default_ct.into(),
fcx.tcx.def_span(param.def_id),
ObligationCauseCode::MiscObligation,
);
}
}
// Doesn't have defaults.
GenericParamDefKind::Lifetime => {}
}
}

@@ -774,14 +790,25 @@ fn check_where_clauses<'tcx, 'fcx>(
fcx.tcx.mk_param_from_def(param)
}
GenericParamDefKind::Const { .. } => {
// FIXME(const_generics_defaults): I(@lcnr) feel like always
// using the const parameter is the right choice here, even
// if it needs substs.
//
// Before stabilizing this we probably want to get some tests
// where this makes a difference and figure out what's the exact
// behavior we want here.

// If the param has a default, ...
if is_our_default(param) {
let default_ct = tcx.const_param_default(param.def_id);
// Const params currently have to be concrete.
assert!(!default_ct.needs_subst());
default_ct.into()
} else {
fcx.tcx.mk_param_from_def(param)
// ... and it's not a dependent default, ...
if !default_ct.needs_subst() {
// ... then substitute it with the default.
return default_ct.into();
}
}

fcx.tcx.mk_param_from_def(param)
}
}
});
Loading

0 comments on commit faa7dca

Please sign in to comment.