From 3716a3fd3104915b9663a335fa92222ae3e179de Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 17 Jul 2024 15:56:16 -0400 Subject: [PATCH 1/3] Mention that type parameters are used recursively --- compiler/rustc_hir_analysis/messages.ftl | 5 ++ .../rustc_hir_analysis/src/check/wfcheck.rs | 83 ++++++++++++++----- compiler/rustc_hir_analysis/src/errors.rs | 15 ++++ .../inherent-impls-overflow.next.stderr | 23 ++--- .../inherent-impls-overflow.rs | 4 +- tests/ui/traits/issue-105231.rs | 4 +- tests/ui/traits/issue-105231.stderr | 22 +++-- .../ui/variance/variance-unused-type-param.rs | 2 +- .../variance-unused-type-param.stderr | 10 ++- 9 files changed, 119 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 24c5377a3b1..b37c19ed8a8 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 its 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,7 @@ 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 + 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/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index b2ef07d65c5..a4bff9272af 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,19 +1915,60 @@ fn report_bivariance( item_kind => bug!("report_bivariance: unexpected item kind: {item_kind:?}"), }; - let const_param_help = - matches!(param.kind, hir::GenericParamKind::Type { .. } if !has_explicit_bounds) - .then_some(()); + let mut usage_spans = vec![]; + intravisit::walk_item( + &mut CollectUsageSpans { spans: &mut usage_spans, param_def_id: param.def_id.to_def_id() }, + item, + ); - let mut diag = tcx.dcx().create_err(errors::UnusedGenericParameter { - span: param.span, - param_name, - param_def_kind: tcx.def_descr(param.def_id.to_def_id()), - help, - const_param_help, - }); - diag.code(E0392); - diag.emit() + if usage_spans.is_empty() { + let const_param_help = + matches!(param.kind, hir::GenericParamKind::Type { .. } if !has_explicit_bounds) + .then_some(()); + + let mut diag = tcx.dcx().create_err(errors::UnusedGenericParameter { + span: param.span, + param_name, + param_def_kind: tcx.def_descr(param.def_id.to_def_id()), + help, + const_param_help, + }); + diag.code(E0392); + diag.emit() + } else { + 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: (), + }); + diag.emit() + } +} + +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 + && let Res::Def(DefKind::TyParam, def_id) = qpath.res + && def_id == self.param_def_id + { + self.spans.push(t.span); + } + intravisit::walk_ty(self, t); + } } impl<'tcx> WfCheckingCtxt<'_, 'tcx> { diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 0ee87a13e9e..2edd8f54adb 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1603,6 +1603,21 @@ pub(crate) struct UnusedGenericParameter { 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 944fe8aa8e5..b60fdea004e 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr @@ -4,23 +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 = Poly1<(T,)>; - | ^ 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 its 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 = Poly0<(T,)>; - | ^ 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 its variance error[E0275]: overflow evaluating the requirement `Poly0<()> == _` --> $DIR/inherent-impls-overflow.rs:21:6 @@ -32,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`. diff --git a/tests/ui/lazy-type-alias/inherent-impls-overflow.rs b/tests/ui/lazy-type-alias/inherent-impls-overflow.rs index 98f0d811a47..1397695a3fe 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.rs +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.rs @@ -13,10 +13,10 @@ impl Loop {} type Poly0 = 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 = 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<(((((((...,),),),),),),)>` diff --git a/tests/ui/traits/issue-105231.rs b/tests/ui/traits/issue-105231.rs index 89b2da4452a..7338642beef 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 6467a438375..6fd73b29f21 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 its 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 its 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 d1114064364..7e35f59fd84 100644 --- a/tests/ui/variance/variance-unused-type-param.rs +++ b/tests/ui/variance/variance-unused-type-param.rs @@ -11,8 +11,8 @@ 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 } diff --git a/tests/ui/variance/variance-unused-type-param.stderr b/tests/ui/variance/variance-unused-type-param.stderr index 3011b7bd18f..212db564ac4 100644 --- a/tests/ui/variance/variance-unused-type-param.stderr +++ b/tests/ui/variance/variance-unused-type-param.stderr @@ -16,14 +16,16 @@ 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 its variance error[E0392]: type parameter `T` is never used --> $DIR/variance-unused-type-param.rs:19:19 From a0a251a6882b410a72069feb67ab7342d1b0e409 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 17 Jul 2024 16:36:21 -0400 Subject: [PATCH 2/3] Account for self ty alias --- compiler/rustc_hir_analysis/messages.ftl | 2 +- .../rustc_hir_analysis/src/check/wfcheck.rs | 15 ++++++++----- .../inherent-impls-overflow.next.stderr | 4 ++-- tests/ui/traits/issue-105231.stderr | 4 ++-- .../ui/variance/variance-unused-type-param.rs | 3 +++ .../variance-unused-type-param.stderr | 21 ++++++++++++++----- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index b37c19ed8a8..ac46f981d98 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -384,7 +384,7 @@ hir_analysis_precise_capture_self_alias = `Self` can't be captured in `use<...>` 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 its variance + .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}` diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index a4bff9272af..ffe3e61d9cf 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -1961,11 +1961,16 @@ impl<'tcx> Visitor<'tcx> for CollectUsageSpans<'_> { } fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) -> Self::Result { - if let hir::TyKind::Path(hir::QPath::Resolved(None, qpath)) = t.kind - && let Res::Def(DefKind::TyParam, def_id) = qpath.res - && def_id == self.param_def_id - { - self.spans.push(t.span); + 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); } 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 b60fdea004e..192b5eebdaa 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr @@ -13,7 +13,7 @@ LL | type Poly0 = Poly1<(T,)>; | 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 - = note: all type parameters must be used in a non-recursive way in order to constrain its variance + = 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/inherent-impls-overflow.rs:17:24 @@ -24,7 +24,7 @@ LL | type Poly1 = Poly0<(T,)>; | 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 - = note: all type parameters must be used in a non-recursive way in order to constrain its variance + = 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 diff --git a/tests/ui/traits/issue-105231.stderr b/tests/ui/traits/issue-105231.stderr index 6fd73b29f21..d3014a79ad6 100644 --- a/tests/ui/traits/issue-105231.stderr +++ b/tests/ui/traits/issue-105231.stderr @@ -24,7 +24,7 @@ LL | struct A(B); | 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 its variance + = 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/issue-105231.rs:5:17 @@ -35,7 +35,7 @@ LL | struct B(A>); | 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 its variance + = 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` | diff --git a/tests/ui/variance/variance-unused-type-param.rs b/tests/ui/variance/variance-unused-type-param.rs index 7e35f59fd84..ada57ab0d09 100644 --- a/tests/ui/variance/variance-unused-type-param.rs +++ b/tests/ui/variance/variance-unused-type-param.rs @@ -16,6 +16,9 @@ enum ListCell { Nil } +struct SelfTyAlias(Box); +//~^ ERROR parameter `T` is only used recursively + struct WithBounds {} //~^ ERROR parameter `T` is never used diff --git a/tests/ui/variance/variance-unused-type-param.stderr b/tests/ui/variance/variance-unused-type-param.stderr index 212db564ac4..1a45bcba45a 100644 --- a/tests/ui/variance/variance-unused-type-param.stderr +++ b/tests/ui/variance/variance-unused-type-param.stderr @@ -25,10 +25,21 @@ LL | Cons(Box>), | ^ | = 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 its variance + = 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 @@ -36,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 @@ -44,13 +55,13 @@ 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: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0392`. From c02d0de871a99b08d013bd79941889fca53ae3c4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 17 Jul 2024 18:05:21 -0400 Subject: [PATCH 3/3] Account for structs that have unused params in nested types in fields --- compiler/rustc_hir_analysis/messages.ftl | 1 + .../rustc_hir_analysis/src/check/check.rs | 1 + .../rustc_hir_analysis/src/check/wfcheck.rs | 115 ++++++++++++++---- compiler/rustc_hir_analysis/src/errors.rs | 2 + .../inherent-impls-overflow.next.stderr | 23 ++-- .../inherent-impls-overflow.rs | 4 +- .../ui/variance/variance-unused-type-param.rs | 5 + .../variance-unused-type-param.stderr | 14 ++- 8 files changed, 128 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index ac46f981d98..b43559f4225 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -553,6 +553,7 @@ 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}` diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index bf8ef18c04f..dbc265ad3ff 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 ffe3e61d9cf..809427f86ee 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -1921,33 +1921,102 @@ fn report_bivariance<'tcx>( item, ); - if usage_spans.is_empty() { - let const_param_help = - matches!(param.kind, hir::GenericParamKind::Type { .. } if !has_explicit_bounds) - .then_some(()); + 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 mut diag = tcx.dcx().create_err(errors::UnusedGenericParameter { - span: param.span, - param_name, - param_def_kind: tcx.def_descr(param.def_id.to_def_id()), - help, - const_param_help, - }); - diag.code(E0392); - diag.emit() - } else { - 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: (), - }); - diag.emit() + let const_param_help = + matches!(param.kind, hir::GenericParamKind::Type { .. } if !has_explicit_bounds) + .then_some(()); + + let mut diag = tcx.dcx().create_err(errors::UnusedGenericParameter { + span: param.span, + param_name, + param_def_kind: tcx.def_descr(param.def_id.to_def_id()), + usage_spans, + help, + const_param_help, + }); + diag.code(E0392); + 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, diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 2edd8f54adb..cec295e1c15 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1597,6 +1597,8 @@ 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)] 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 192b5eebdaa..48e7da96612 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr @@ -4,27 +4,27 @@ error[E0275]: overflow evaluating the requirement `Loop == _` LL | impl Loop {} | ^^^^ -error: type parameter `T` is only used recursively - --> $DIR/inherent-impls-overflow.rs:14:24 +error[E0392]: type parameter `T` is never used + --> $DIR/inherent-impls-overflow.rs:14:12 | LL | type Poly0 = Poly1<(T,)>; - | - ^ + | ^ - `T` is named here, but is likely unused in the containing type | | - | type parameter must be used non-recursively in the definition + | unused type parameter | = help: consider removing `T` or referring to it in the body of the type alias - = note: all type parameters must be used in a non-recursive way in order to constrain their variance + = help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead -error: type parameter `T` is only used recursively - --> $DIR/inherent-impls-overflow.rs:17:24 +error[E0392]: type parameter `T` is never used + --> $DIR/inherent-impls-overflow.rs:17:12 | LL | type Poly1 = Poly0<(T,)>; - | - ^ + | ^ - `T` is named here, but is likely unused in the containing type | | - | type parameter must be used non-recursively in the definition + | unused type parameter | = help: consider removing `T` or referring to it in the body of the type alias - = note: all type parameters must be used in a non-recursive way in order to constrain their variance + = help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead error[E0275]: overflow evaluating the requirement `Poly0<()> == _` --> $DIR/inherent-impls-overflow.rs:21:6 @@ -36,4 +36,5 @@ LL | impl Poly0<()> {} error: aborting due to 4 previous errors -For more information about this error, try `rustc --explain E0275`. +Some errors have detailed explanations: E0275, E0392. +For more information about an error, try `rustc --explain E0275`. diff --git a/tests/ui/lazy-type-alias/inherent-impls-overflow.rs b/tests/ui/lazy-type-alias/inherent-impls-overflow.rs index 1397695a3fe..98f0d811a47 100644 --- a/tests/ui/lazy-type-alias/inherent-impls-overflow.rs +++ b/tests/ui/lazy-type-alias/inherent-impls-overflow.rs @@ -13,10 +13,10 @@ impl Loop {} type Poly0 = Poly1<(T,)>; //[current]~^ ERROR overflow normalizing the type alias `Poly0<(((((((...,),),),),),),)>` -//[next]~^^ ERROR type parameter `T` is only used recursively +//[next]~^^ ERROR type parameter `T` is never used type Poly1 = Poly0<(T,)>; //[current]~^ ERROR overflow normalizing the type alias `Poly1<(((((((...,),),),),),),)>` -//[next]~^^ ERROR type parameter `T` is only used recursively +//[next]~^^ ERROR type parameter `T` is never used impl Poly0<()> {} //[current]~^ ERROR overflow normalizing the type alias `Poly1<(((((((...,),),),),),),)>` diff --git a/tests/ui/variance/variance-unused-type-param.rs b/tests/ui/variance/variance-unused-type-param.rs index ada57ab0d09..ef3c41ca556 100644 --- a/tests/ui/variance/variance-unused-type-param.rs +++ b/tests/ui/variance/variance-unused-type-param.rs @@ -28,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 1a45bcba45a..c747532e628 100644 --- a/tests/ui/variance/variance-unused-type-param.stderr +++ b/tests/ui/variance/variance-unused-type-param.stderr @@ -62,6 +62,18 @@ LL | struct WithOutlivesBounds {} | = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` -error: aborting due to 7 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`.