diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 24c5377a3b125..b43559f422533 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -382,6 +382,10 @@ hir_analysis_placeholder_not_allowed_item_signatures = the placeholder `_` is no hir_analysis_precise_capture_self_alias = `Self` can't be captured in `use<...>` precise captures list, since it is an alias .label = `Self` is not a generic argument, but an alias to the type of the {$what} +hir_analysis_recursive_generic_parameter = {$param_def_kind} `{$param_name}` is only used recursively + .label = {$param_def_kind} must be used non-recursively in the definition + .note = all type parameters must be used in a non-recursive way in order to constrain their variance + hir_analysis_redundant_lifetime_args = unnecessary lifetime parameter `{$victim}` .note = you can use the `{$candidate}` lifetime directly, in place of `{$victim}` @@ -549,6 +553,8 @@ hir_analysis_unused_generic_parameter = {$param_def_kind} `{$param_name}` is never used .label = unused {$param_def_kind} .const_param_help = if you intended `{$param_name}` to be a const parameter, use `const {$param_name}: /* Type */` instead + .usage_spans = `{$param_name}` is named here, but is likely unused in the containing type + hir_analysis_unused_generic_parameter_adt_help = consider removing `{$param_name}`, referring to it in a field, or using a marker such as `{$phantom_data}` hir_analysis_unused_generic_parameter_adt_no_phantom_data_help = diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index bf8ef18c04fcc..dbc265ad3ff32 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1572,6 +1572,7 @@ fn check_type_alias_type_params_are_used<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalD param_name, param_def_kind: tcx.def_descr(param.def_id), help: errors::UnusedGenericParameterHelp::TyAlias { param_name }, + usage_spans: vec![], const_param_help, }); diag.code(E0091); diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index b2ef07d65c5b4..809427f86eef1 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -4,12 +4,12 @@ use crate::constrained_generic_params::{identify_constrained_generic_params, Par use crate::errors; use crate::fluent_generated as fluent; -use hir::intravisit::Visitor; +use hir::intravisit::{self, Visitor}; use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, ErrorGuaranteed}; use rustc_hir as hir; -use rustc_hir::def::DefKind; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId}; use rustc_hir::lang_items::LangItem; use rustc_hir::ItemKind; @@ -1799,7 +1799,7 @@ fn receiver_is_implemented<'tcx>( fn check_variances_for_type_defn<'tcx>( tcx: TyCtxt<'tcx>, - item: &hir::Item<'tcx>, + item: &'tcx hir::Item<'tcx>, hir_generics: &hir::Generics<'tcx>, ) { let identity_args = ty::GenericArgs::identity_for_item(tcx, item.owner_id); @@ -1886,21 +1886,21 @@ fn check_variances_for_type_defn<'tcx>( hir::ParamName::Error => {} _ => { let has_explicit_bounds = explicitly_bounded_params.contains(¶meter); - report_bivariance(tcx, hir_param, has_explicit_bounds, item.kind); + report_bivariance(tcx, hir_param, has_explicit_bounds, item); } } } } -fn report_bivariance( - tcx: TyCtxt<'_>, - param: &rustc_hir::GenericParam<'_>, +fn report_bivariance<'tcx>( + tcx: TyCtxt<'tcx>, + param: &'tcx hir::GenericParam<'tcx>, has_explicit_bounds: bool, - item_kind: ItemKind<'_>, + item: &'tcx hir::Item<'tcx>, ) -> ErrorGuaranteed { let param_name = param.name.ident(); - let help = match item_kind { + let help = match item.kind { ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Union(..) => { if let Some(def_id) = tcx.lang_items().phantom_data() { errors::UnusedGenericParameterHelp::Adt { @@ -1915,6 +1915,49 @@ fn report_bivariance( item_kind => bug!("report_bivariance: unexpected item kind: {item_kind:?}"), }; + let mut usage_spans = vec![]; + intravisit::walk_item( + &mut CollectUsageSpans { spans: &mut usage_spans, param_def_id: param.def_id.to_def_id() }, + item, + ); + + 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. + 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... + // + // 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 { + 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(); + } + } + let const_param_help = matches!(param.kind, hir::GenericParamKind::Type { .. } if !has_explicit_bounds) .then_some(()); @@ -1923,6 +1966,7 @@ fn report_bivariance( span: param.span, param_name, param_def_kind: tcx.def_descr(param.def_id.to_def_id()), + usage_spans, help, const_param_help, }); @@ -1930,6 +1974,77 @@ fn report_bivariance( 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 +/// cyclical. +/// +/// Notably, we don't consider substitutions here, so this may have false positives. +struct IsProbablyCyclical<'tcx> { + tcx: TyCtxt<'tcx>, + adt_def_id: DefId, + seen: FxHashSet, +} + +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)?; + } + + ControlFlow::Continue(()) + } +} + +impl<'tcx> TypeVisitor> 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 { + return ControlFlow::Break(()); + } + + if self.seen.insert(adt_def.did()) { + self.visit_all_fields(adt_def)?; + } + } + + t.super_visit_with(self) + } +} + +/// Collect usages of the `param_def_id` and `Res::SelfTyAlias` in the HIR. +/// +/// This is used to report places where the user has used parameters in a +/// non-variance-constraining way for better bivariance errors. +struct CollectUsageSpans<'a> { + spans: &'a mut Vec, + param_def_id: DefId, +} + +impl<'tcx> Visitor<'tcx> for CollectUsageSpans<'_> { + type Result = (); + + fn visit_generics(&mut self, _g: &'tcx rustc_hir::Generics<'tcx>) -> Self::Result { + // Skip the generics. We only care about fields, not where clause/param bounds. + } + + fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) -> Self::Result { + if let hir::TyKind::Path(hir::QPath::Resolved(None, qpath)) = t.kind { + if let Res::Def(DefKind::TyParam, def_id) = qpath.res + && def_id == self.param_def_id + { + self.spans.push(t.span); + return; + } else if let Res::SelfTyAlias { .. } = qpath.res { + self.spans.push(t.span); + return; + } + } + intravisit::walk_ty(self, t); + } +} + impl<'tcx> WfCheckingCtxt<'_, 'tcx> { /// Feature gates RFC 2056 -- trivial bounds, checking for global bounds that /// aren't true. diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 0ee87a13e9e37..cec295e1c151a 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1597,12 +1597,29 @@ pub(crate) struct UnusedGenericParameter { pub span: Span, pub param_name: Ident, pub param_def_kind: &'static str, + #[label(hir_analysis_usage_spans)] + pub usage_spans: Vec, #[subdiagnostic] pub help: UnusedGenericParameterHelp, #[help(hir_analysis_const_param_help)] pub const_param_help: Option<()>, } +#[derive(Diagnostic)] +#[diag(hir_analysis_recursive_generic_parameter)] +pub(crate) struct RecursiveGenericParameter { + #[primary_span] + pub spans: Vec, + #[label] + pub param_span: Span, + pub param_name: Ident, + pub param_def_kind: &'static str, + #[subdiagnostic] + pub help: UnusedGenericParameterHelp, + #[note] + pub note: (), +} + #[derive(Subdiagnostic)] pub(crate) enum UnusedGenericParameterHelp { #[help(hir_analysis_unused_generic_parameter_adt_help)] diff --git a/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr b/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr index 944fe8aa8e596..48e7da9661210 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr @@ -8,7 +8,9 @@ error[E0392]: type parameter `T` is never used --> $DIR/inherent-impls-overflow.rs:14:12 | LL | type Poly0 = Poly1<(T,)>; - | ^ unused type parameter + | ^ - `T` is named here, but is likely unused in the containing type + | | + | unused type parameter | = 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 @@ -17,7 +19,9 @@ error[E0392]: type parameter `T` is never used --> $DIR/inherent-impls-overflow.rs:17:12 | LL | type Poly1 = Poly0<(T,)>; - | ^ unused type parameter + | ^ - `T` is named here, but is likely unused in the containing type + | | + | unused type parameter | = 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 diff --git a/tests/ui/traits/issue-105231.rs b/tests/ui/traits/issue-105231.rs index 89b2da4452a5c..7338642beefa6 100644 --- a/tests/ui/traits/issue-105231.rs +++ b/tests/ui/traits/issue-105231.rs @@ -1,9 +1,9 @@ //~ ERROR overflow evaluating the requirement `A>>>>>>: Send` struct A(B); //~^ ERROR recursive types `A` and `B` have infinite size -//~| ERROR `T` is never used +//~| ERROR `T` is only used recursively struct B(A>); -//~^ ERROR `T` is never used +//~^ ERROR `T` is only used recursively trait Foo {} impl Foo for T where T: Send {} impl Foo for B {} diff --git a/tests/ui/traits/issue-105231.stderr b/tests/ui/traits/issue-105231.stderr index 6467a438375c9..d3014a79ad605 100644 --- a/tests/ui/traits/issue-105231.stderr +++ b/tests/ui/traits/issue-105231.stderr @@ -15,23 +15,27 @@ LL | LL ~ struct B(Box>>); | -error[E0392]: type parameter `T` is never used - --> $DIR/issue-105231.rs:2:10 +error: type parameter `T` is only used recursively + --> $DIR/issue-105231.rs:2:15 | LL | struct A(B); - | ^ unused type parameter + | - ^ + | | + | type parameter must be used non-recursively in the definition | = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` - = 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/issue-105231.rs:5:10 +error: type parameter `T` is only used recursively + --> $DIR/issue-105231.rs:5:17 | LL | struct B(A>); - | ^ unused type parameter + | - ^ + | | + | type parameter must be used non-recursively in the definition | = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` - = 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 `A>>>>>>: Send` | @@ -44,5 +48,5 @@ LL | struct B(A>); error: aborting due to 4 previous errors -Some errors have detailed explanations: E0072, E0275, E0392. +Some errors have detailed explanations: E0072, E0275. For more information about an error, try `rustc --explain E0072`. diff --git a/tests/ui/variance/variance-unused-type-param.rs b/tests/ui/variance/variance-unused-type-param.rs index d111406436478..ef3c41ca5560c 100644 --- a/tests/ui/variance/variance-unused-type-param.rs +++ b/tests/ui/variance/variance-unused-type-param.rs @@ -11,11 +11,14 @@ enum SomeEnum { Nothing } // Here T might *appear* used, but in fact it isn't. enum ListCell { -//~^ ERROR parameter `T` is never used Cons(Box>), + //~^ ERROR parameter `T` is only used recursively Nil } +struct SelfTyAlias(Box); +//~^ ERROR parameter `T` is only used recursively + struct WithBounds {} //~^ ERROR parameter `T` is never used @@ -25,4 +28,9 @@ struct WithWhereBounds where T: Sized {} struct WithOutlivesBounds {} //~^ ERROR parameter `T` is never used +struct DoubleNothing { +//~^ ERROR parameter `T` is never used + s: SomeStruct, +} + fn main() {} diff --git a/tests/ui/variance/variance-unused-type-param.stderr b/tests/ui/variance/variance-unused-type-param.stderr index 3011b7bd18fa7..c747532e62836 100644 --- a/tests/ui/variance/variance-unused-type-param.stderr +++ b/tests/ui/variance/variance-unused-type-param.stderr @@ -16,17 +16,30 @@ LL | enum SomeEnum { Nothing } = help: consider removing `A`, referring to it in a field, or using a marker such as `PhantomData` = help: if you intended `A` to be a const parameter, use `const A: /* Type */` instead -error[E0392]: type parameter `T` is never used - --> $DIR/variance-unused-type-param.rs:13:15 +error: type parameter `T` is only used recursively + --> $DIR/variance-unused-type-param.rs:14:23 | LL | enum ListCell { - | ^ unused type parameter + | - type parameter must be used non-recursively in the definition +LL | Cons(Box>), + | ^ | = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` - = 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: type parameter `T` is only used recursively + --> $DIR/variance-unused-type-param.rs:19:27 + | +LL | struct SelfTyAlias(Box); + | - ^^^^ + | | + | type parameter must be used non-recursively in the definition + | + = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` + = 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/variance-unused-type-param.rs:19:19 + --> $DIR/variance-unused-type-param.rs:22:19 | LL | struct WithBounds {} | ^ unused type parameter @@ -34,7 +47,7 @@ LL | struct WithBounds {} = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` error[E0392]: type parameter `T` is never used - --> $DIR/variance-unused-type-param.rs:22:24 + --> $DIR/variance-unused-type-param.rs:25:24 | LL | struct WithWhereBounds where T: Sized {} | ^ unused type parameter @@ -42,13 +55,25 @@ LL | struct WithWhereBounds where T: Sized {} = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` error[E0392]: type parameter `T` is never used - --> $DIR/variance-unused-type-param.rs:25:27 + --> $DIR/variance-unused-type-param.rs:28:27 | LL | struct WithOutlivesBounds {} | ^ unused type parameter | = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` -error: aborting due to 6 previous errors +error[E0392]: type parameter `T` is never used + --> $DIR/variance-unused-type-param.rs:31:22 + | +LL | struct DoubleNothing { + | ^ unused type parameter +LL | +LL | s: SomeStruct, + | - `T` is named here, but is likely unused in the containing type + | + = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` + = help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0392`.