Account for structs that have unused params in nested types in fields

8 files changed, 129 insertions, 38 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(());
-
-        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()
+    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(());
+
+    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<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)?;
+        }
+
+        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 {
+                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<Span>,
     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<Span>,
     #[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<T> = 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<T> = 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<T> = 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<T> = 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<T> where T: Sized {}
 struct WithOutlivesBounds<T: 'static> {}
 //~^ ERROR parameter `T` is never used
 
+struct DoubleNothing<T> {
+//~^ ERROR parameter `T` is never used
+    s: SomeStruct<T>,
+}
+
 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<T: 'static> {}
    |
    = 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<T> {
+   |                      ^ unused type parameter
+LL |
+LL |     s: SomeStruct<T>,
+   |                   - `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`.