Rollup merge of rust-lang#120556 - fmease:improve-unused-generic-param-diags, r=oli-obk

Improve the diagnostics for unused generic parameters

* Don't emit two errors (namely E0091 *and* E0392) for unused type parameters on *lazy* type aliases
* Fix the diagnostic help message of E0392 for *lazy* type aliases: Don't talk about the “fields” of lazy type aliases (use the term “body” instead) and don't suggest `PhantomData` for them, it doesn't make much sense
* Consolidate the diagnostics for E0091 (unused type parameters in type aliases) and E0392 (unused generic parameters due to bivariance) and make it translatable
  * Still keep the error codes distinct (for now)
  * Naturally leads to better diagnostics for E0091

r? ``@oli-obk`` (to ballast your review load :P) or compiler

Author: matthiaskrgr

compiler/rustc_error_codes/src/error_codes/E0091.md

@@ -1,11 +1,11 @@
-An unnecessary type or const parameter was given in a type alias.
+An unnecessary type parameter was given in a type alias.
 
 Erroneous code example:
 
 ```compile_fail,E0091
-type Foo<T> = u32; // error: type parameter `T` is unused
+type Foo<T> = u32; // error: type parameter `T` is never used
 // or:
-type Foo<A,B> = Box<A>; // error: type parameter `B` is unused
+type Foo<A, B> = Box<A>; // error: type parameter `B` is never used
 ```
 
 Please check you didn't write too many parameters. Example:

compiler/rustc_hir_analysis/messages.ftl

@@ -434,6 +434,17 @@ hir_analysis_unused_associated_type_bounds =
     .note = this associated type has a `where Self: Sized` bound. Thus, while the associated type can be specified, it cannot be used in any way, because trait objects are not `Sized`.
     .suggestion = remove this bound
 
+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 =
+    consider removing `{$param_name}` or referring to it in a field
+hir_analysis_unused_generic_parameter_ty_alias_help =
+    consider removing `{$param_name}` or referring to it in the body of the type alias
+
 hir_analysis_value_of_associated_struct_already_specified =
     the value of the associated type `{$item_name}` in trait `{$def_path}` is already specified
     .label = re-bound here

compiler/rustc_hir_analysis/src/check/check.rs

@@ -31,6 +31,7 @@ use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
 use rustc_trait_selection::traits::{self, TraitEngine, TraitEngineExt as _};
 use rustc_type_ir::fold::TypeFoldable;
 
+use std::cell::LazyCell;
 use std::ops::ControlFlow;
 
 pub fn check_abi(tcx: TyCtxt<'_>, hir_id: hir::HirId, span: Span, abi: Abi) {
@@ -520,9 +521,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
             }
         }
         DefKind::TyAlias => {
-            let pty_ty = tcx.type_of(def_id).instantiate_identity();
-            let generics = tcx.generics_of(def_id);
-            check_type_params_are_used(tcx, generics, pty_ty);
+            check_type_alias_type_params_are_used(tcx, def_id);
         }
         DefKind::ForeignMod => {
             let it = tcx.hir().expect_item(def_id);
@@ -1269,28 +1268,51 @@ fn detect_discriminant_duplicate<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>)
     }
 }
 
-pub(super) fn check_type_params_are_used<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    generics: &ty::Generics,
-    ty: Ty<'tcx>,
-) {
-    debug!("check_type_params_are_used(generics={:?}, ty={:?})", generics, ty);
-
-    assert_eq!(generics.parent, None);
+fn check_type_alias_type_params_are_used<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) {
+    if tcx.type_alias_is_lazy(def_id) {
+        // Since we compute the variances for lazy type aliases and already reject bivariant
+        // parameters as unused, we can and should skip this check for lazy type aliases.
+        return;
+    }
 
+    let generics = tcx.generics_of(def_id);
     if generics.own_counts().types == 0 {
         return;
     }
 
-    let mut params_used = BitSet::new_empty(generics.params.len());
-
+    let ty = tcx.type_of(def_id).instantiate_identity();
     if ty.references_error() {
-        // If there is already another error, do not emit
-        // an error for not using a type parameter.
+        // If there is already another error, do not emit an error for not using a type parameter.
         assert!(tcx.dcx().has_errors().is_some());
         return;
     }
 
+    // Lazily calculated because it is only needed in case of an error.
+    let bounded_params = LazyCell::new(|| {
+        tcx.explicit_predicates_of(def_id)
+            .predicates
+            .iter()
+            .filter_map(|(predicate, span)| {
+                let bounded_ty = match predicate.kind().skip_binder() {
+                    ty::ClauseKind::Trait(pred) => pred.trait_ref.self_ty(),
+                    ty::ClauseKind::TypeOutlives(pred) => pred.0,
+                    _ => return None,
+                };
+                if let ty::Param(param) = bounded_ty.kind() {
+                    Some((param.index, span))
+                } else {
+                    None
+                }
+            })
+            // FIXME: This assumes that elaborated `Sized` bounds come first (which does hold at the
+            // time of writing). This is a bit fragile since we later use the span to detect elaborated
+            // `Sized` bounds. If they came last for example, this would break `Trait + /*elab*/Sized`
+            // since it would overwrite the span of the user-written bound. This could be fixed by
+            // folding the spans with `Span::to` which requires a bit of effort I think.
+            .collect::<FxHashMap<_, _>>()
+    });
+
+    let mut params_used = BitSet::new_empty(generics.params.len());
     for leaf in ty.walk() {
         if let GenericArgKind::Type(leaf_ty) = leaf.unpack()
             && let ty::Param(param) = leaf_ty.kind()
@@ -1305,15 +1327,24 @@ pub(super) fn check_type_params_are_used<'tcx>(
             && let ty::GenericParamDefKind::Type { .. } = param.kind
         {
             let span = tcx.def_span(param.def_id);
-            struct_span_code_err!(
-                tcx.dcx(),
+            let param_name = Ident::new(param.name, span);
+
+            // The corresponding predicates are post-`Sized`-elaboration. Therefore we
+            // * check for emptiness to detect lone user-written `?Sized` bounds
+            // * compare the param span to the pred span to detect lone user-written `Sized` bounds
+            let has_explicit_bounds = bounded_params.is_empty()
+                || (*bounded_params).get(&param.index).is_some_and(|&&pred_sp| pred_sp != span);
+            let const_param_help = (!has_explicit_bounds).then_some(());
+
+            let mut diag = tcx.dcx().create_err(errors::UnusedGenericParameter {
                 span,
-                E0091,
-                "type parameter `{}` is unused",
-                param.name,
-            )
-            .with_span_label(span, "unused type parameter")
-            .emit();
+                param_name,
+                param_def_kind: tcx.def_descr(param.def_id),
+                help: errors::UnusedGenericParameterHelp::TyAlias { param_name },
+                const_param_help,
+            });
+            diag.code(E0091);
+            diag.emit();
         }
     }
 }

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -1,11 +1,10 @@
 use crate::autoderef::Autoderef;
 use crate::constrained_generic_params::{identify_constrained_generic_params, Parameter};
+use crate::errors;
 
 use rustc_ast as ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
-use rustc_errors::{
-    codes::*, pluralize, struct_span_code_err, Applicability, DiagnosticBuilder, ErrorGuaranteed,
-};
+use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
 use rustc_hir::lang_items::LangItem;
@@ -21,7 +20,7 @@ use rustc_middle::ty::{
 };
 use rustc_middle::ty::{GenericArgKind, GenericArgs};
 use rustc_session::parse::feature_err;
-use rustc_span::symbol::{sym, Ident, Symbol};
+use rustc_span::symbol::{sym, Ident};
 use rustc_span::{Span, DUMMY_SP};
 use rustc_target::spec::abi::Abi;
 use rustc_trait_selection::regions::InferCtxtRegionExt;
@@ -1869,7 +1868,7 @@ fn check_variances_for_type_defn<'tcx>(
             hir::ParamName::Error => {}
             _ => {
                 let has_explicit_bounds = explicitly_bounded_params.contains(&parameter);
-                report_bivariance(tcx, hir_param, has_explicit_bounds);
+                report_bivariance(tcx, hir_param, has_explicit_bounds, item.kind);
             }
         }
     }
@@ -1879,30 +1878,38 @@ fn report_bivariance(
     tcx: TyCtxt<'_>,
     param: &rustc_hir::GenericParam<'_>,
     has_explicit_bounds: bool,
+    item_kind: ItemKind<'_>,
 ) -> ErrorGuaranteed {
-    let span = param.span;
-    let param_name = param.name.ident().name;
-    let mut err = error_392(tcx, span, param_name);
-
-    let suggested_marker_id = tcx.lang_items().phantom_data();
-    // Help is available only in presence of lang items.
-    let msg = if let Some(def_id) = suggested_marker_id {
-        format!(
-            "consider removing `{}`, referring to it in a field, or using a marker such as `{}`",
-            param_name,
-            tcx.def_path_str(def_id),
-        )
-    } else {
-        format!("consider removing `{param_name}` or referring to it in a field")
+    let param_name = param.name.ident();
+
+    let help = match item_kind {
+        ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Union(..) => {
+            if let Some(def_id) = tcx.lang_items().phantom_data() {
+                errors::UnusedGenericParameterHelp::Adt {
+                    param_name,
+                    phantom_data: tcx.def_path_str(def_id),
+                }
+            } else {
+                errors::UnusedGenericParameterHelp::AdtNoPhantomData { param_name }
+            }
+        }
+        ItemKind::TyAlias(..) => errors::UnusedGenericParameterHelp::TyAlias { param_name },
+        item_kind => bug!("report_bivariance: unexpected item kind: {item_kind:?}"),
     };
-    err.help(msg);
 
-    if matches!(param.kind, hir::GenericParamKind::Type { .. }) && !has_explicit_bounds {
-        err.help(format!(
-            "if you intended `{param_name}` to be a const parameter, use `const {param_name}: usize` instead"
-        ));
-    }
-    err.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()),
+        help,
+        const_param_help,
+    });
+    diag.code(E0392);
+    diag.emit()
 }
 
 impl<'tcx> WfCheckingCtxt<'_, 'tcx> {
@@ -1967,11 +1974,6 @@ fn check_mod_type_wf(tcx: TyCtxt<'_>, module: LocalModDefId) -> Result<(), Error
     res
 }
 
-fn error_392(tcx: TyCtxt<'_>, span: Span, param_name: Symbol) -> DiagnosticBuilder<'_> {
-    struct_span_code_err!(tcx.dcx(), span, E0392, "parameter `{param_name}` is never used")
-        .with_span_label(span, "unused parameter")
-}
-
 pub fn provide(providers: &mut Providers) {
     *providers = Providers { check_mod_type_wf, check_well_formed, ..*providers };
 }

compiler/rustc_hir_analysis/src/errors.rs

@@ -1511,3 +1511,27 @@ pub struct NotSupportedDelegation<'a> {
     #[label]
     pub callee_span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag(hir_analysis_unused_generic_parameter)]
+pub(crate) struct UnusedGenericParameter {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    pub param_name: Ident,
+    pub param_def_kind: &'static str,
+    #[subdiagnostic]
+    pub help: UnusedGenericParameterHelp,
+    #[help(hir_analysis_const_param_help)]
+    pub const_param_help: Option<()>,
+}
+
+#[derive(Subdiagnostic)]
+pub(crate) enum UnusedGenericParameterHelp {
+    #[help(hir_analysis_unused_generic_parameter_adt_help)]
+    Adt { param_name: Ident, phantom_data: String },
+    #[help(hir_analysis_unused_generic_parameter_adt_no_phantom_data_help)]
+    AdtNoPhantomData { param_name: Ident },
+    #[help(hir_analysis_unused_generic_parameter_ty_alias_help)]
+    TyAlias { param_name: Ident },
+}

tests/ui/const-generics/generic_arg_infer/infer-arg-test.stderr

@@ -10,14 +10,14 @@ error: expected identifier, found reserved identifier `_`
 LL | fn bad_infer_fn<_>() {}
    |                 ^ expected identifier, found reserved identifier
 
-error[E0392]: parameter `_` is never used
+error[E0392]: type parameter `_` is never used
   --> $DIR/infer-arg-test.rs:7:17
    |
 LL | struct BadInfer<_>;
-   |                 ^ unused parameter
+   |                 ^ unused type parameter
    |
    = help: consider removing `_`, referring to it in a field, or using a marker such as `PhantomData`
-   = help: if you intended `_` to be a const parameter, use `const _: usize` instead
+   = help: if you intended `_` to be a const parameter, use `const _: /* Type */` instead
 
 error[E0107]: struct takes 2 generic arguments but 3 generic arguments were supplied
   --> $DIR/infer-arg-test.rs:18:10

tests/ui/const-generics/issue-46511.stderr

@@ -7,11 +7,11 @@ LL |     _a: [u8; std::mem::size_of::<&'a mut u8>()]
    = note: lifetime parameters may not be used in const expressions
    = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions
 
-error[E0392]: parameter `'a` is never used
+error[E0392]: lifetime parameter `'a` is never used
   --> $DIR/issue-46511.rs:3:12
    |
 LL | struct Foo<'a>
-   |            ^^ unused parameter
+   |            ^^ unused lifetime parameter
    |
    = help: consider removing `'a`, referring to it in a field, or using a marker such as `PhantomData`
 

tests/ui/const-generics/issues/issue-67375.min.stderr

@@ -7,14 +7,14 @@ LL |     inner: [(); { [|_: &T| {}; 0].len() }],
    = note: type parameters may not be used in const expressions
    = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions
 
-error[E0392]: parameter `T` is never used
+error[E0392]: type parameter `T` is never used
   --> $DIR/issue-67375.rs:5:12
    |
 LL | struct Bug<T> {
-   |            ^ unused parameter
+   |            ^ unused type parameter
    |
    = 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: usize` instead
+   = help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead
 
 error: aborting due to 2 previous errors
 

tests/ui/const-generics/issues/issue-67945-1.min.stderr

@@ -16,14 +16,14 @@ LL |         let b = &*(&x as *const _ as *const S);
    = note: type parameters may not be used in const expressions
    = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions
 
-error[E0392]: parameter `S` is never used
+error[E0392]: type parameter `S` is never used
   --> $DIR/issue-67945-1.rs:7:12
    |
 LL | struct Bug<S> {
-   |            ^ unused parameter
+   |            ^ unused type parameter
    |
    = help: consider removing `S`, referring to it in a field, or using a marker such as `PhantomData`
-   = help: if you intended `S` to be a const parameter, use `const S: usize` instead
+   = help: if you intended `S` to be a const parameter, use `const S: /* Type */` instead
 
 error: aborting due to 3 previous errors
 

tests/ui/const-generics/issues/issue-67945-3.min.stderr

@@ -7,14 +7,14 @@ LL |         let x: Option<S> = None;
    = note: type parameters may not be used in const expressions
    = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions
 
-error[E0392]: parameter `S` is never used
+error[E0392]: type parameter `S` is never used
   --> $DIR/issue-67945-3.rs:9:12
    |
 LL | struct Bug<S> {
-   |            ^ unused parameter
+   |            ^ unused type parameter
    |
    = help: consider removing `S`, referring to it in a field, or using a marker such as `PhantomData`
-   = help: if you intended `S` to be a const parameter, use `const S: usize` instead
+   = help: if you intended `S` to be a const parameter, use `const S: /* Type */` instead
 
 error: aborting due to 2 previous errors
 

tests/ui/const-generics/issues/issue-67945-4.min.stderr

@@ -7,14 +7,14 @@ LL |         let x: Option<Box<S>> = None;
    = note: type parameters may not be used in const expressions
    = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions
 
-error[E0392]: parameter `S` is never used
+error[E0392]: type parameter `S` is never used
   --> $DIR/issue-67945-4.rs:8:12
    |
 LL | struct Bug<S> {
-   |            ^ unused parameter
+   |            ^ unused type parameter
    |
    = help: consider removing `S`, referring to it in a field, or using a marker such as `PhantomData`
-   = help: if you intended `S` to be a const parameter, use `const S: usize` instead
+   = help: if you intended `S` to be a const parameter, use `const S: /* Type */` instead
 
 error: aborting due to 2 previous errors