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.

Sign up for GitHub

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

Jump to bottom

various const parameter defaults improvements #84299

Merged
merged 4 commits into from
Apr 25, 2021
Merged

various const parameter defaults improvements #84299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
259a368
fix name resolution for param defaults
lcnr Apr 18, 2021
7cb1dcd
loosen ordering restricts for `const_generics_defaults`
lcnr Apr 18, 2021
312b4fd
improve wf check for const param defaults
lcnr Apr 18, 2021
d3e0d2f
supply substs to anon consts in defaults
lcnr Apr 18, 2021
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
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
};
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/active.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
};
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
}
}
}
Expand Down
11 changes: 0 additions & 11 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
69 changes: 40 additions & 29 deletions compiler/rustc_resolve/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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));
}
}
}
Expand Down
58 changes: 8 additions & 50 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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 }`.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 }`.
Expand All @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_typeck/src/astconv/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
ParamKindOrd::Const {
unordered: tcx
.features()
.const_generics,
.unordered_const_ty_params(),
}
}
},
Expand All @@ -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!(
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_typeck/src/astconv/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
53 changes: 40 additions & 13 deletions compiler/rustc_typeck/src/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {}
}
}

Expand Down Expand Up @@ -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)
}
}
});
Expand Down
Loading