Skip to content

Commit

Permalink
Improve the diagnostics for unused generic parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
fmease committed Feb 1, 2024
1 parent cb4d9a1 commit 02320b5
Show file tree
Hide file tree
Showing 43 changed files with 371 additions and 171 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_error_codes/src/error_codes/E0091.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
77 changes: 54 additions & 23 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()
Expand All @@ -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();
}
}
}
Expand Down
64 changes: 33 additions & 31 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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> {
Expand Down Expand Up @@ -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 };
}
24 changes: 24 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/const-generics/issue-46511.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/const-generics/issues/issue-67375.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/const-generics/issues/issue-67945-1.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/const-generics/issues/issue-67945-3.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/const-generics/issues/issue-67945-4.min.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit 02320b5

Please sign in to comment.