1
Fork 0

Move check to wfcheck

This commit is contained in:
Michael Goulet 2023-11-29 02:41:31 +00:00
parent ceff692a02
commit 2d813547bf
10 changed files with 206 additions and 197 deletions

View file

@ -355,6 +355,9 @@ hir_analysis_pattern_type_wild_pat = "wildcard patterns are not permitted for pa
hir_analysis_placeholder_not_allowed_item_signatures = the placeholder `_` is not allowed within types on item signatures for {$kind}
.label = not allowed in type signatures
hir_analysis_redundant_lifetime_args = unnecessary lifetime parameter `{$victim}`
.note = you can use the `{$candidate}` lifetime directly, in place of `{$victim}`
hir_analysis_requires_note = the `{$trait_name}` impl for `{$ty}` requires that `{$error_predicate}`
hir_analysis_return_type_notation_equality_bound =

View file

@ -8,11 +8,13 @@ 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_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::lang_items::LangItem;
use rustc_hir::ItemKind;
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
use rustc_macros::LintDiagnostic;
use rustc_middle::query::Providers;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::trait_def::TraitSpecializationKind;
@ -136,6 +138,8 @@ where
infcx.implied_bounds_tys_compat(param_env, body_def_id, &assumed_wf_types, false);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
lint_redundant_lifetimes(tcx, body_def_id, &outlives_env);
let errors = infcx.resolve_regions(&outlives_env);
if errors.is_empty() {
return Ok(());
@ -2010,6 +2014,130 @@ fn check_mod_type_wf(tcx: TyCtxt<'_>, module: LocalModDefId) -> Result<(), Error
res
}
fn lint_redundant_lifetimes<'tcx>(
tcx: TyCtxt<'tcx>,
owner_id: LocalDefId,
outlives_env: &OutlivesEnvironment<'tcx>,
) {
let def_kind = tcx.def_kind(owner_id);
match def_kind {
DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Trait
| DefKind::TraitAlias
| DefKind::Fn
| DefKind::Const
| DefKind::Impl { of_trait: false } => {
// Proceed
}
DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => {
let parent_def_id = tcx.local_parent(owner_id);
if matches!(tcx.def_kind(parent_def_id), DefKind::Impl { of_trait: true }) {
// Don't check for redundant lifetimes for trait implementations,
// since the signature is required to be compatible with the trait.
return;
}
}
DefKind::Impl { of_trait: true }
| DefKind::Mod
| DefKind::Variant
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TyParam
| DefKind::ConstParam
| DefKind::Static { .. }
| DefKind::Ctor(_, _)
| DefKind::Macro(_)
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure => return,
}
// The ordering of this lifetime map is a bit subtle.
//
// Specifically, we want to find a "candidate" lifetime that precedes a "victim" lifetime,
// where we can prove that `'candidate = 'victim`.
//
// `'static` must come first in this list because we can never replace `'static` with
// something else, but if we find some lifetime `'a` where `'a = 'static`, we want to
// suggest replacing `'a` with `'static`.
let mut lifetimes = vec![tcx.lifetimes.re_static];
lifetimes.extend(
ty::GenericArgs::identity_for_item(tcx, owner_id).iter().filter_map(|arg| arg.as_region()),
);
// If we are in a function, add its late-bound lifetimes too.
if matches!(def_kind, DefKind::Fn | DefKind::AssocFn) {
for var in tcx.fn_sig(owner_id).instantiate_identity().bound_vars() {
let ty::BoundVariableKind::Region(kind) = var else { continue };
lifetimes.push(ty::Region::new_late_param(tcx, owner_id.to_def_id(), kind));
}
}
lifetimes.retain(|candidate| candidate.has_name());
// Keep track of lifetimes which have already been replaced with other lifetimes.
// This makes sure that if `'a = 'b = 'c`, we don't say `'c` should be replaced by
// both `'a` and `'b`.
let mut shadowed = FxHashSet::default();
for (idx, &candidate) in lifetimes.iter().enumerate() {
// Don't suggest removing a lifetime twice.
if shadowed.contains(&candidate) {
continue;
}
for &victim in &lifetimes[(idx + 1)..] {
// We only care about lifetimes that are "real", i.e. that have a def-id.
let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. })
| ty::ReLateParam(ty::LateParamRegion {
bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
..
})) = victim.kind()
else {
continue;
};
// Do not rename lifetimes not local to this item since they'll overlap
// with the lint running on the parent. We still want to consider parent
// lifetimes which make child lifetimes redundant, otherwise we would
// have truncated the `identity_for_item` args above.
if tcx.parent(def_id) != owner_id.to_def_id() {
continue;
}
// If there are no lifetime errors, then we have proven that `'candidate = 'victim`!
if outlives_env.free_region_map().sub_free_regions(tcx, candidate, victim)
&& outlives_env.free_region_map().sub_free_regions(tcx, victim, candidate)
{
shadowed.insert(victim);
tcx.emit_spanned_lint(
rustc_lint_defs::builtin::UNUSED_LIFETIMES,
tcx.local_def_id_to_hir_id(def_id.expect_local()),
tcx.def_span(def_id),
RedundantLifetimeArgsLint { candidate, victim },
);
}
}
}
}
#[derive(LintDiagnostic)]
#[diag(hir_analysis_redundant_lifetime_args)]
#[note]
struct RedundantLifetimeArgsLint<'tcx> {
/// The lifetime we have found to be redundant.
victim: ty::Region<'tcx>,
// The lifetime we can replace the victim with.
candidate: ty::Region<'tcx>,
}
pub fn provide(providers: &mut Providers) {
*providers = Providers { check_mod_type_wf, check_well_formed, ..*providers };
}

View file

@ -534,9 +534,6 @@ lint_reason_must_be_string_literal = reason must be a string literal
lint_reason_must_come_last = reason in lint attribute must come last
lint_redundant_lifetime_args = unnecessary lifetime parameter `{$victim}`
.note = you can use the `{$candidate}` lifetime directly, in place of `{$victim}`
lint_redundant_semicolons =
unnecessary trailing {$multiple ->
[true] semicolons

View file

@ -78,7 +78,6 @@ mod opaque_hidden_inferred_bound;
mod pass_by_value;
mod passes;
mod ptr_nulls;
mod redundant_lifetime_args;
mod redundant_semicolon;
mod reference_casting;
mod traits;
@ -114,7 +113,6 @@ use noop_method_call::*;
use opaque_hidden_inferred_bound::*;
use pass_by_value::*;
use ptr_nulls::*;
use redundant_lifetime_args::RedundantLifetimeArgs;
use redundant_semicolon::*;
use reference_casting::*;
use traits::*;
@ -235,7 +233,6 @@ late_lint_methods!(
MissingDoc: MissingDoc,
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
RedundantLifetimeArgs: RedundantLifetimeArgs,
]
]
);

View file

@ -1,177 +0,0 @@
#![allow(rustc::diagnostic_outside_of_impl)]
#![allow(rustc::untranslatable_diagnostic)]
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::{SubregionOrigin, TyCtxtInferExt};
use rustc_macros::LintDiagnostic;
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::lint::builtin::UNUSED_LIFETIMES;
use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits::{outlives_bounds::InferCtxtExt, ObligationCtxt};
use crate::{LateContext, LateLintPass};
declare_lint_pass!(RedundantLifetimeArgs => []);
impl<'tcx> LateLintPass<'tcx> for RedundantLifetimeArgs {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
check(cx.tcx, cx.param_env, item.owner_id);
}
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'tcx>) {
check(cx.tcx, cx.param_env, item.owner_id);
}
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'tcx>) {
if cx
.tcx
.hir()
.expect_item(cx.tcx.local_parent(item.owner_id.def_id))
.expect_impl()
.of_trait
.is_some()
{
// Don't check for redundant lifetimes for trait implementations,
// since the signature is required to be compatible with the trait.
return;
}
check(cx.tcx, cx.param_env, item.owner_id);
}
}
fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir::OwnerId) {
let def_kind = tcx.def_kind(owner_id);
match def_kind {
DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Trait
| DefKind::TraitAlias
| DefKind::AssocTy
| DefKind::Fn
| DefKind::Const
| DefKind::AssocFn
| DefKind::AssocConst
| DefKind::Impl { of_trait: _ } => {
// Proceed
}
DefKind::Mod
| DefKind::Variant
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TyParam
| DefKind::ConstParam
| DefKind::Static(_)
| DefKind::Ctor(_, _)
| DefKind::Macro(_)
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure => return,
}
let infcx = &tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(infcx);
// Compute the implied outlives bounds for the item. This ensures that we treat
// a signature with an argument like `&'a &'b ()` as implicitly having `'b: 'a`.
let Ok(assumed_wf_types) = ocx.assumed_wf_types(param_env, owner_id.def_id) else {
return;
};
let implied_bounds = infcx.implied_bounds_tys(param_env, owner_id.def_id, assumed_wf_types);
let outlives_env = &OutlivesEnvironment::with_bounds(param_env, implied_bounds);
// The ordering of this lifetime map is a bit subtle.
//
// Specifically, we want to find a "candidate" lifetime that precedes a "victim" lifetime,
// where we can prove that `'candidate = 'victim`.
//
// `'static` must come first in this list because we can never replace `'static` with
// something else, but if we find some lifetime `'a` where `'a = 'static`, we want to
// suggest replacing `'a` with `'static`.
let mut lifetimes = vec![tcx.lifetimes.re_static];
lifetimes.extend(
ty::GenericArgs::identity_for_item(tcx, owner_id).iter().filter_map(|arg| arg.as_region()),
);
// If we are in a function, add its late-bound lifetimes too.
if matches!(def_kind, DefKind::Fn | DefKind::AssocFn) {
for var in tcx.fn_sig(owner_id).instantiate_identity().bound_vars() {
let ty::BoundVariableKind::Region(kind) = var else { continue };
lifetimes.push(ty::Region::new_late_param(tcx, owner_id.to_def_id(), kind));
}
}
// Keep track of lifetimes which have already been replaced with other lifetimes.
// This makes sure that if `'a = 'b = 'c`, we don't say `'c` should be replaced by
// both `'a` and `'b`.
let mut shadowed = FxHashSet::default();
for (idx, &candidate) in lifetimes.iter().enumerate() {
// Don't suggest removing a lifetime twice.
if shadowed.contains(&candidate) {
continue;
}
// Can't rename a named lifetime named `'_` without ambiguity.
if !candidate.has_name() {
continue;
}
for &victim in &lifetimes[(idx + 1)..] {
// We only care about lifetimes that are "real", i.e. that have a def-id.
let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. })
| ty::ReLateParam(ty::LateParamRegion {
bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
..
})) = victim.kind()
else {
continue;
};
// Do not rename lifetimes not local to this item since they'll overlap
// with the lint running on the parent. We still want to consider parent
// lifetimes which make child lifetimes redundant, otherwise we would
// have truncated the `identity_for_item` args above.
if tcx.parent(def_id) != owner_id.to_def_id() {
continue;
}
let infcx = infcx.fork();
// Require that `'candidate = 'victim`
infcx.sub_regions(SubregionOrigin::RelateRegionParamBound(DUMMY_SP), candidate, victim);
infcx.sub_regions(SubregionOrigin::RelateRegionParamBound(DUMMY_SP), victim, candidate);
// If there are no lifetime errors, then we have proven that `'candidate = 'victim`!
if infcx.resolve_regions(outlives_env).is_empty() {
shadowed.insert(victim);
tcx.emit_spanned_lint(
UNUSED_LIFETIMES,
tcx.local_def_id_to_hir_id(def_id.expect_local()),
tcx.def_span(def_id),
RedundantLifetimeArgsLint { candidate, victim },
);
}
}
}
}
#[derive(LintDiagnostic)]
#[diag(lint_redundant_lifetime_args)]
#[note]
struct RedundantLifetimeArgsLint<'tcx> {
/// The lifetime we have found to be redundant.
victim: ty::Region<'tcx>,
// The lifetime we can replace the victim with.
candidate: ty::Region<'tcx>,
}

View file

@ -1,5 +1,7 @@
#![warn(unused_lifetimes)]
pub trait X {
type Y<'a: 'static>;
type Y<'a: 'static>; //~ WARN unnecessary lifetime parameter `'a`
}
impl X for () {

View file

@ -1,5 +1,5 @@
error[E0478]: lifetime bound not satisfied
--> $DIR/unsatisfied-item-lifetime-bound.rs:6:18
--> $DIR/unsatisfied-item-lifetime-bound.rs:8:18
|
LL | type Y<'a: 'static>;
| ------------------- definition of `Y` from trait
@ -8,7 +8,7 @@ LL | type Y<'a> = &'a ();
| ^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'a` as defined here
--> $DIR/unsatisfied-item-lifetime-bound.rs:6:12
--> $DIR/unsatisfied-item-lifetime-bound.rs:8:12
|
LL | type Y<'a> = &'a ();
| ^^
@ -19,44 +19,57 @@ LL | type Y<'a> = &'a () where 'a: 'static;
| +++++++++++++++++
error[E0478]: lifetime bound not satisfied
--> $DIR/unsatisfied-item-lifetime-bound.rs:11:8
--> $DIR/unsatisfied-item-lifetime-bound.rs:13:8
|
LL | f: <T as X>::Y<'a>,
| ^^^^^^^^^^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'a` as defined here
--> $DIR/unsatisfied-item-lifetime-bound.rs:10:10
--> $DIR/unsatisfied-item-lifetime-bound.rs:12:10
|
LL | struct B<'a, T: for<'r> X<Y<'r> = &'r ()>> {
| ^^
= note: but lifetime parameter must outlive the static lifetime
error[E0478]: lifetime bound not satisfied
--> $DIR/unsatisfied-item-lifetime-bound.rs:16:8
--> $DIR/unsatisfied-item-lifetime-bound.rs:18:8
|
LL | f: <T as X>::Y<'a>,
| ^^^^^^^^^^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'a` as defined here
--> $DIR/unsatisfied-item-lifetime-bound.rs:15:10
--> $DIR/unsatisfied-item-lifetime-bound.rs:17:10
|
LL | struct C<'a, T: X> {
| ^^
= note: but lifetime parameter must outlive the static lifetime
error[E0478]: lifetime bound not satisfied
--> $DIR/unsatisfied-item-lifetime-bound.rs:21:8
--> $DIR/unsatisfied-item-lifetime-bound.rs:23:8
|
LL | f: <() as X>::Y<'a>,
| ^^^^^^^^^^^^^^^^
|
note: lifetime parameter instantiated with the lifetime `'a` as defined here
--> $DIR/unsatisfied-item-lifetime-bound.rs:20:10
--> $DIR/unsatisfied-item-lifetime-bound.rs:22:10
|
LL | struct D<'a> {
| ^^
= note: but lifetime parameter must outlive the static lifetime
error: aborting due to 4 previous errors
warning: unnecessary lifetime parameter `'a`
--> $DIR/unsatisfied-item-lifetime-bound.rs:4:12
|
LL | type Y<'a: 'static>;
| ^^
|
= note: you can use the `'static` lifetime directly, in place of `'a`
note: the lint level is defined here
--> $DIR/unsatisfied-item-lifetime-bound.rs:1:9
|
LL | #![warn(unused_lifetimes)]
| ^^^^^^^^^^^^^^^^
error: aborting due to 4 previous errors; 1 warning emitted
For more information about this error, try `rustc --explain E0478`.

View file

@ -1,6 +1,12 @@
#![warn(unused_lifetimes)]
fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t }
//~^ WARN unnecessary lifetime parameter `'a`
//~| WARN lifetime parameter `'b` never used
fn static_id_indirect<'a,'b>(t: &'a ()) -> &'static ()
//~^ WARN unnecessary lifetime parameter `'a`
//~| WARN unnecessary lifetime parameter `'b`
where 'a: 'b, 'b: 'static { t }
fn static_id_wrong_way<'a>(t: &'a ()) -> &'static () where 'static: 'a {

View file

@ -1,5 +1,43 @@
warning: lifetime parameter `'b` never used
--> $DIR/regions-static-bound.rs:3:17
|
LL | fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t }
| -^^
| |
| help: elide the unused lifetime
|
note: the lint level is defined here
--> $DIR/regions-static-bound.rs:1:9
|
LL | #![warn(unused_lifetimes)]
| ^^^^^^^^^^^^^^^^
warning: unnecessary lifetime parameter `'a`
--> $DIR/regions-static-bound.rs:3:14
|
LL | fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t }
| ^^
|
= note: you can use the `'static` lifetime directly, in place of `'a`
warning: unnecessary lifetime parameter `'a`
--> $DIR/regions-static-bound.rs:7:23
|
LL | fn static_id_indirect<'a,'b>(t: &'a ()) -> &'static ()
| ^^
|
= note: you can use the `'static` lifetime directly, in place of `'a`
warning: unnecessary lifetime parameter `'b`
--> $DIR/regions-static-bound.rs:7:26
|
LL | fn static_id_indirect<'a,'b>(t: &'a ()) -> &'static ()
| ^^
|
= note: you can use the `'static` lifetime directly, in place of `'b`
error: lifetime may not live long enough
--> $DIR/regions-static-bound.rs:7:5
--> $DIR/regions-static-bound.rs:13:5
|
LL | fn static_id_wrong_way<'a>(t: &'a ()) -> &'static () where 'static: 'a {
| -- lifetime `'a` defined here
@ -7,7 +45,7 @@ LL | t
| ^ returning this value requires that `'a` must outlive `'static`
error[E0521]: borrowed data escapes outside of function
--> $DIR/regions-static-bound.rs:12:5
--> $DIR/regions-static-bound.rs:18:5
|
LL | fn error(u: &(), v: &()) {
| - - let's call the lifetime of this reference `'1`
@ -20,7 +58,7 @@ LL | static_id(&u);
| argument requires that `'1` must outlive `'static`
error[E0521]: borrowed data escapes outside of function
--> $DIR/regions-static-bound.rs:14:5
--> $DIR/regions-static-bound.rs:20:5
|
LL | fn error(u: &(), v: &()) {
| - - let's call the lifetime of this reference `'2`
@ -33,6 +71,6 @@ LL | static_id_indirect(&v);
| `v` escapes the function body here
| argument requires that `'2` must outlive `'static`
error: aborting due to 3 previous errors
error: aborting due to 3 previous errors; 4 warnings emitted
For more information about this error, try `rustc --explain E0521`.

View file

@ -13,4 +13,6 @@ impl<'a> Bar<'a> {
fn d<'b: 'a>(&'b self) {} //~ ERROR unnecessary lifetime parameter `'b`
}
fn ok(x: &'static &()) {}
fn main() {}