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.

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

Lazy type aliases: Diagnostics: Detect bivariant ty params that are only used recursively #127976

Merged
merged 1 commit into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 37 additions & 36 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1922,39 +1922,31 @@ fn report_bivariance<'tcx>(
);

if !usage_spans.is_empty() {
// First, check if the ADT is (probably) cyclical. We say probably here, since
// we're not actually looking into substitutions, just walking through fields.
// And we only recurse into the fields of ADTs, and not the hidden types of
// opaques or anything else fancy.
// First, check if the ADT/LTA is (probably) cyclical. We say probably here, since we're
// not actually looking into substitutions, just walking through fields / the "RHS".
// We don't recurse into the hidden types of opaques or anything else fancy.
let item_def_id = item.owner_id.to_def_id();
let is_probably_cyclical = if matches!(
tcx.def_kind(item_def_id),
DefKind::Struct | DefKind::Union | DefKind::Enum
) {
IsProbablyCyclical { tcx, adt_def_id: item_def_id, seen: Default::default() }
.visit_all_fields(tcx.adt_def(item_def_id))
.is_break()
} else {
false
};
// If the ADT is cyclical, then if at least one usage of the type parameter or
// the `Self` alias is present in the, then it's probably a cyclical struct, and
// we should call those parameter usages recursive rather than just saying they're
// unused...
let is_probably_cyclical =
IsProbablyCyclical { tcx, item_def_id, seen: Default::default() }
.visit_def(item_def_id)
.is_break();
// If the ADT/LTA is cyclical, then if at least one usage of the type parameter or
// the `Self` alias is present in the, then it's probably a cyclical struct/ type
// alias, and we should call those parameter usages recursive rather than just saying
// they're unused...
//
// We currently report *all* of the parameter usages, since computing the exact
// subset is very involved, and the fact we're mentioning recursion at all is
// likely to guide the user in the right direction.
if is_probably_cyclical {
let diag = tcx.dcx().create_err(errors::RecursiveGenericParameter {
return tcx.dcx().emit_err(errors::RecursiveGenericParameter {
spans: usage_spans,
param_span: param.span,
param_name,
param_def_kind: tcx.def_descr(param.def_id.to_def_id()),
help,
note: (),
});
return diag.emit();
}
}

Expand All @@ -1974,42 +1966,51 @@ fn report_bivariance<'tcx>(
diag.emit()
}

/// Detects cases where an ADT is trivially cyclical -- we want to detect this so
/// /we only mention that its parameters are used cyclically if the ADT is truly
/// Detects cases where an ADT/LTA is trivially cyclical -- we want to detect this so
/// we only mention that its parameters are used cyclically if the ADT/LTA is truly
/// cyclical.
///
/// Notably, we don't consider substitutions here, so this may have false positives.
struct IsProbablyCyclical<'tcx> {
tcx: TyCtxt<'tcx>,
adt_def_id: DefId,
item_def_id: DefId,
seen: FxHashSet<DefId>,
}

impl<'tcx> IsProbablyCyclical<'tcx> {
fn visit_all_fields(&mut self, adt_def: ty::AdtDef<'tcx>) -> ControlFlow<(), ()> {
for field in adt_def.all_fields() {
self.tcx.type_of(field.did).instantiate_identity().visit_with(self)?;
fn visit_def(&mut self, def_id: DefId) -> ControlFlow<(), ()> {
match self.tcx.def_kind(def_id) {
DefKind::Struct | DefKind::Enum | DefKind::Union => {
self.tcx.adt_def(def_id).all_fields().try_for_each(|field| {
self.tcx.type_of(field.did).instantiate_identity().visit_with(self)
})
}
DefKind::TyAlias if self.tcx.type_alias_is_lazy(def_id) => {
self.tcx.type_of(def_id).instantiate_identity().visit_with(self)
}
_ => ControlFlow::Continue(()),
}

ControlFlow::Continue(())
}
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for IsProbablyCyclical<'tcx> {
type Result = ControlFlow<(), ()>;

fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<(), ()> {
if let Some(adt_def) = t.ty_adt_def() {
if adt_def.did() == self.adt_def_id {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<(), ()> {
let def_id = match ty.kind() {
ty::Adt(adt_def, _) => Some(adt_def.did()),
ty::Alias(ty::Weak, alias_ty) => Some(alias_ty.def_id),
_ => None,
};
if let Some(def_id) = def_id {
if def_id == self.item_def_id {
return ControlFlow::Break(());
}

if self.seen.insert(adt_def.did()) {
self.visit_all_fields(adt_def)?;
if self.seen.insert(def_id) {
self.visit_def(def_id)?;
}
}

t.super_visit_with(self)
ty.super_visit_with(self)
}
}

Expand Down
23 changes: 11 additions & 12 deletions tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@ error[E0275]: overflow evaluating the requirement `Loop == _`
LL | impl Loop {}
| ^^^^

error[E0392]: type parameter `T` is never used
--> $DIR/inherent-impls-overflow.rs:14:12
error: type parameter `T` is only used recursively
--> $DIR/inherent-impls-overflow.rs:14:24
|
LL | type Poly0<T> = Poly1<(T,)>;
| ^ - `T` is named here, but is likely unused in the containing type
| - ^
| |
| unused type parameter
| type parameter must be used non-recursively in the definition
|
= help: consider removing `T` or referring to it in the body of the type alias
= help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead
= note: all type parameters must be used in a non-recursive way in order to constrain their variance

error[E0392]: type parameter `T` is never used
--> $DIR/inherent-impls-overflow.rs:17:12
error: type parameter `T` is only used recursively
--> $DIR/inherent-impls-overflow.rs:17:24
|
LL | type Poly1<T> = Poly0<(T,)>;
| ^ - `T` is named here, but is likely unused in the containing type
| - ^
| |
| unused type parameter
| type parameter must be used non-recursively in the definition
|
= help: consider removing `T` or referring to it in the body of the type alias
= help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead
= note: all type parameters must be used in a non-recursive way in order to constrain their variance

error[E0275]: overflow evaluating the requirement `Poly0<()> == _`
--> $DIR/inherent-impls-overflow.rs:21:6
Expand All @@ -36,5 +36,4 @@ LL | impl Poly0<()> {}

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0275, E0392.
For more information about an error, try `rustc --explain E0275`.
For more information about this error, try `rustc --explain E0275`.
4 changes: 2 additions & 2 deletions tests/ui/lazy-type-alias/inherent-impls-overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ impl Loop {}

type Poly0<T> = Poly1<(T,)>;
//[current]~^ ERROR overflow normalizing the type alias `Poly0<(((((((...,),),),),),),)>`
//[next]~^^ ERROR type parameter `T` is never used
//[next]~^^ ERROR type parameter `T` is only used recursively
type Poly1<T> = Poly0<(T,)>;
//[current]~^ ERROR overflow normalizing the type alias `Poly1<(((((((...,),),),),),),)>`
//[next]~^^ ERROR type parameter `T` is never used
//[next]~^^ ERROR type parameter `T` is only used recursively

impl Poly0<()> {}
//[current]~^ ERROR overflow normalizing the type alias `Poly1<(((((((...,),),),),),),)>`
Expand Down
Loading