Skip to content

Simplify some places that deal with generic parameter defaults #132912

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

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Simplify some places that deal with generic parameter defaults #132912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
98 changes: 23 additions & 75 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1427,57 +1427,28 @@ fn check_where_clauses<'tcx>(wfcx: &WfCheckingCtxt<'_, 'tcx>, span: Span, def_id
let predicates = tcx.predicates_of(def_id.to_def_id());
let generics = tcx.generics_of(def_id);

let is_our_default = |def: &ty::GenericParamDef| match def.kind {
GenericParamDefKind::Type { has_default, .. }
| GenericParamDefKind::Const { has_default, .. } => {
has_default && def.index >= generics.parent_count as u32
}
GenericParamDefKind::Lifetime => {
span_bug!(tcx.def_span(def.def_id), "lifetime params can have no default")
}
};

// Check that concrete defaults are well-formed. See test `type-check-defaults.rs`.
// For example, this forbids the declaration:
//
// struct Foo<T = Vec<[u32]>> { .. }
//
// Here, the default `Vec<[u32]>` is not WF because `[u32]: Sized` does not hold.
for param in &generics.own_params {
match param.kind {
GenericParamDefKind::Type { .. } => {
if is_our_default(param) {
let ty = tcx.type_of(param.def_id).instantiate_identity();
// 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.has_param() {
wfcx.register_wf_obligation(
tcx.def_span(param.def_id),
Some(WellFormedLoc::Ty(param.def_id.expect_local())),
ty.into(),
);
}
}
}
GenericParamDefKind::Const { .. } => {
if is_our_default(param) {
// FIXME(const_generics_defaults): This
// is incorrect when dealing with unused args, for example
// for `struct Foo<const N: usize, const M: usize = { 1 - 2 }>`
// we should eagerly error.
let default_ct = tcx.const_param_default(param.def_id).instantiate_identity();
if !default_ct.has_param() {
wfcx.register_wf_obligation(
tcx.def_span(param.def_id),
None,
default_ct.into(),
);
}
}
if let Some(default) = param.default_value(tcx).map(ty::EarlyBinder::instantiate_identity) {
// 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.
// FIXME(generic_const_exprs): This is incorrect when dealing with unused const params.
// E.g: `struct Foo<const N: usize, const M: usize = { 1 - 2 }>;`. Here, we should
// eagerly error but we don't as we have `ConstKind::Unevaluated(.., [N, M])`.
if !default.has_param() {
wfcx.register_wf_obligation(
tcx.def_span(param.def_id),
matches!(param.kind, GenericParamDefKind::Type { .. })
.then(|| WellFormedLoc::Ty(param.def_id.expect_local())),
default,
);
}
// Doesn't have defaults.
GenericParamDefKind::Lifetime => {}
}
}

Expand All @@ -1490,39 +1461,16 @@ fn check_where_clauses<'tcx>(wfcx: &WfCheckingCtxt<'_, 'tcx>, span: Span, def_id
//
// First we build the defaulted generic parameters.
let args = GenericArgs::for_item(tcx, def_id.to_def_id(), |param, _| {
match param.kind {
GenericParamDefKind::Lifetime => {
// All regions are identity.
tcx.mk_param_from_def(param)
}

GenericParamDefKind::Type { .. } => {
// If the param has a default, ...
if is_our_default(param) {
let default_ty = tcx.type_of(param.def_id).instantiate_identity();
// ... and it's not a dependent default, ...
if !default_ty.has_param() {
// ... then instantiate it with the default.
return default_ty.into();
}
}

tcx.mk_param_from_def(param)
}
GenericParamDefKind::Const { .. } => {
// If the param has a default, ...
if is_our_default(param) {
let default_ct = tcx.const_param_default(param.def_id).instantiate_identity();
// ... and it's not a dependent default, ...
if !default_ct.has_param() {
// ... then instantiate it with the default.
return default_ct.into();
}
}

tcx.mk_param_from_def(param)
}
if param.index >= generics.parent_count as u32
// If the param has a default, ...
&& let Some(default) = param.default_value(tcx).map(ty::EarlyBinder::instantiate_identity)
// ... and it's not a dependent default, ...
&& !default.has_param()
{
// ... then instantiate it with the default.
return default;
}
tcx.mk_param_from_def(param)
});

// Now we build the instantiated predicates.
Expand Down
32 changes: 9 additions & 23 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1306,30 +1306,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rustc_hir_analysis::hir_ty_lowering::RegionInferReason::Param(param),
)
.into(),
GenericParamDefKind::Type { has_default, .. } => {
if !infer_args && has_default {
// If we have a default, then it doesn't matter that we're not
// inferring the type arguments: we provide the default where any
// is missing.
tcx.type_of(param.def_id).instantiate(tcx, preceding_args).into()
} else {
// If no type arguments were provided, we have to infer them.
// This case also occurs as a result of some malformed input, e.g.
// a lifetime argument being given instead of a type parameter.
// Using inference instead of `Error` gives better error messages.
self.fcx.var_for_def(self.span, param)
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
if !infer_args && let Some(default) = param.default_value(tcx) {
// If we have a default, then it doesn't matter that we're not inferring
// the type/const arguments: We provide the default where any is missing.
return default.instantiate(tcx, preceding_args);
}
}
GenericParamDefKind::Const { has_default, .. } => {
if has_default {
if !infer_args {
return tcx
.const_param_default(param.def_id)
.instantiate(tcx, preceding_args)
.into();
}
}

// If no type/const arguments were provided, we have to infer them.
// This case also occurs as a result of some malformed input, e.g.,
// a lifetime argument being given instead of a type/const parameter.
// Using inference instead of `Error` gives better error messages.
self.fcx.var_for_def(self.span, param)
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ impl GenericParamDef {
tcx: TyCtxt<'tcx>,
) -> Option<EarlyBinder<'tcx, ty::GenericArg<'tcx>>> {
match self.kind {
GenericParamDefKind::Type { has_default, .. } if has_default => {
GenericParamDefKind::Type { has_default: true, .. } => {
Some(tcx.type_of(self.def_id).map_bound(|t| t.into()))
}
GenericParamDefKind::Const { has_default, .. } if has_default => {
GenericParamDefKind::Const { has_default: true, .. } => {
Some(tcx.const_param_default(self.def_id).map_bound(|c| c.into()))
}
_ => None,
Expand Down
20 changes: 5 additions & 15 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,21 +782,11 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
impl ReachEverythingInTheInterfaceVisitor<'_, '_> {
fn generics(&mut self) -> &mut Self {
for param in &self.ev.tcx.generics_of(self.item_def_id).own_params {
match param.kind {
GenericParamDefKind::Lifetime => {}
GenericParamDefKind::Type { has_default, .. } => {
if has_default {
self.visit(self.ev.tcx.type_of(param.def_id).instantiate_identity());
}
}
GenericParamDefKind::Const { has_default, .. } => {
self.visit(self.ev.tcx.type_of(param.def_id).instantiate_identity());
if has_default {
self.visit(
self.ev.tcx.const_param_default(param.def_id).instantiate_identity(),
);
}
}
if let GenericParamDefKind::Const { .. } = param.kind {
self.visit(self.ev.tcx.type_of(param.def_id).instantiate_identity());
}
if let Some(default) = param.default_value(self.ev.tcx) {
Copy link
Contributor

@petrochenkov petrochenkov Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default_value naming is pretty misleading, it looks like it only returns something for constant parameters (because only they has values).
I thought the GenericParamDefKind::Type was lost here before I looked at the definition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about renaming it to default?

Copy link
Member Author

@fmease fmease Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, default_term (cc, {ast, hir}::Term) or default_arg?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think default_term would be fine and probably actually an improvement, but also I don't know if I share the same concern about the default_value name.

self.visit(default.instantiate_identity());
}
}
self
Expand Down
Loading