From 03c901fd35b67ee0e67c13c350bc4e1a60e1d91d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Nov 2023 21:29:29 +0000 Subject: [PATCH 1/8] Add redundant_lifetime_args lint --- compiler/rustc_errors/src/diagnostic_impls.rs | 1 + compiler/rustc_lint/messages.ftl | 2 + compiler/rustc_lint/src/lib.rs | 3 + .../rustc_lint/src/redundant_lifetime_args.rs | 160 ++++++++++++++++++ compiler/rustc_middle/src/ty/diagnostics.rs | 15 +- 5 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 compiler/rustc_lint/src/redundant_lifetime_args.rs diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index f90190797aee9..6c0551848d688 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -46,6 +46,7 @@ impl<'a, T: Clone + IntoDiagArg> IntoDiagArg for &'a T { } } +#[macro_export] macro_rules! into_diag_arg_using_display { ($( $ty:ty ),+ $(,)?) => { $( diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 82b90e1660a95..ad0a5a2b6242c 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -534,6 +534,8 @@ 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 = lifetime `{$victim}` is required to be equal to `{$candidate}`, and is redundant and can be removed + lint_redundant_semicolons = unnecessary trailing {$multiple -> [true] semicolons diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 31c80c4d75bb1..d13c954947614 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -78,6 +78,7 @@ 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; @@ -113,6 +114,7 @@ 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::*; @@ -233,6 +235,7 @@ late_lint_methods!( MissingDoc: MissingDoc, AsyncFnInTrait: AsyncFnInTrait, NonLocalDefinitions: NonLocalDefinitions::default(), + RedundantLifetimeArgs: RedundantLifetimeArgs, ] ] ); diff --git a/compiler/rustc_lint/src/redundant_lifetime_args.rs b/compiler/rustc_lint/src/redundant_lifetime_args.rs new file mode 100644 index 0000000000000..7a7c5387369ea --- /dev/null +++ b/compiler/rustc_lint/src/redundant_lifetime_args.rs @@ -0,0 +1,160 @@ +#![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_span::DUMMY_SP; +use rustc_trait_selection::traits::{outlives_bounds::InferCtxtExt, ObligationCtxt}; + +use crate::{LateContext, LateLintPass}; + +declare_lint! { + /// Docs + pub REDUNDANT_LIFETIME_ARGS, + Allow, + "do something" +} + +declare_lint_pass!(RedundantLifetimeArgs => [REDUNDANT_LIFETIME_ARGS]); + +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); + 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); + + 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 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. + let mut shadowed = FxHashSet::default(); + + for (idx, &candidate) in lifetimes.iter().enumerate() { + if shadowed.contains(&candidate) { + // Don't suggest removing a lifetime twice. + continue; + } + + if !candidate.has_name() { + // Can't rename a named lifetime with `'_` without ambiguity. + continue; + } + + for &victim in &lifetimes[(idx + 1)..] { + let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) + | ty::ReLateParam(ty::LateParamRegion { + bound_region: ty::BoundRegionKind::BrNamed(def_id, _), + .. + })) = victim.kind() + else { + continue; + }; + + if tcx.parent(def_id) != owner_id.to_def_id() { + // Do not rename generics not local to this item since + // they'll overlap with this lint running on the parent. + continue; + } + + let infcx = infcx.fork(); + infcx.sub_regions(SubregionOrigin::RelateRegionParamBound(DUMMY_SP), candidate, victim); + infcx.sub_regions(SubregionOrigin::RelateRegionParamBound(DUMMY_SP), victim, candidate); + if infcx.resolve_regions(outlives_env).is_empty() { + shadowed.insert(victim); + tcx.emit_spanned_lint( + REDUNDANT_LIFETIME_ARGS, + tcx.local_def_id_to_hir_id(def_id.expect_local()), + tcx.def_span(def_id), + RedundantLifetimeArgsList { candidate, victim }, + ); + } + } + } +} + +#[derive(LintDiagnostic)] +#[diag(lint_redundant_lifetime_args)] +struct RedundantLifetimeArgsList<'tcx> { + candidate: ty::Region<'tcx>, + victim: ty::Region<'tcx>, +} diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index ee18647cdd8fd..cc1d6e50f6d9a 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -5,13 +5,13 @@ use std::fmt::Write; use std::ops::ControlFlow; use crate::ty::{ - AliasTy, Const, ConstKind, FallibleTypeFolder, InferConst, InferTy, Opaque, PolyTraitPredicate, - Projection, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, - TypeVisitor, + self, AliasTy, Const, ConstKind, FallibleTypeFolder, InferConst, InferTy, Opaque, + PolyTraitPredicate, Projection, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable, + TypeSuperVisitable, TypeVisitable, TypeVisitor, }; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{Applicability, Diag, DiagArgValue, IntoDiagArg}; +use rustc_errors::{into_diag_arg_using_display, Applicability, Diag, DiagArgValue, IntoDiagArg}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; @@ -19,10 +19,9 @@ use rustc_hir::{PredicateOrigin, WherePredicate}; use rustc_span::{BytePos, Span}; use rustc_type_ir::TyKind::*; -impl<'tcx> IntoDiagArg for Ty<'tcx> { - fn into_diag_arg(self) -> DiagArgValue { - self.to_string().into_diag_arg() - } +into_diag_arg_using_display! { + Ty<'_>, + ty::Region<'_>, } impl<'tcx> Ty<'tcx> { From 89409494e3cf0ece89116b4838de609e07290ff1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Nov 2023 21:54:03 +0000 Subject: [PATCH 2/8] Actually, just reuse the UNUSED_LIFETIMES lint --- .../src/collect/resolve_bound_vars.rs | 26 ------------- compiler/rustc_lint/messages.ftl | 3 +- .../rustc_lint/src/redundant_lifetime_args.rs | 17 +++----- .../unsatisfied-item-lifetime-bound.rs | 3 -- .../unsatisfied-item-lifetime-bound.stderr | 31 +++++---------- ...on-outlives-static-outlives-free-region.rs | 4 +- ...utlives-static-outlives-free-region.stderr | 8 ++-- .../ui/regions/regions-static-bound-rpass.rs | 7 ++-- .../regions/regions-static-bound-rpass.stderr | 34 +++++++++------- tests/ui/regions/regions-static-bound.rs | 5 --- tests/ui/regions/regions-static-bound.stderr | 38 ++---------------- .../transitively-redundant-lifetimes.rs | 16 ++++++++ .../transitively-redundant-lifetimes.stderr | 39 +++++++++++++++++++ 13 files changed, 107 insertions(+), 124 deletions(-) create mode 100644 tests/ui/regions/transitively-redundant-lifetimes.rs create mode 100644 tests/ui/regions/transitively-redundant-lifetimes.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index 0b8ac9926e419..affd678fc6cc7 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -20,7 +20,6 @@ use rustc_middle::hir::nested_filter; use rustc_middle::middle::resolve_bound_vars::*; use rustc_middle::query::Providers; use rustc_middle::ty::{self, TyCtxt, TypeSuperVisitable, TypeVisitor}; -use rustc_session::lint; use rustc_span::def_id::DefId; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -867,31 +866,6 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> { }) => { self.visit_lifetime(lifetime); walk_list!(self, visit_param_bound, bounds); - - if lifetime.res != hir::LifetimeName::Static { - for bound in bounds { - let hir::GenericBound::Outlives(lt) = bound else { - continue; - }; - if lt.res != hir::LifetimeName::Static { - continue; - } - self.insert_lifetime(lt, ResolvedArg::StaticLifetime); - self.tcx.node_span_lint( - lint::builtin::UNUSED_LIFETIMES, - lifetime.hir_id, - lifetime.ident.span, - format!("unnecessary lifetime parameter `{}`", lifetime.ident), - |lint| { - let help = format!( - "you can use the `'static` lifetime directly, in place of `{}`", - lifetime.ident, - ); - lint.help(help); - }, - ); - } - } } &hir::WherePredicate::EqPredicate(hir::WhereEqPredicate { lhs_ty, rhs_ty, .. }) => { self.visit_ty(lhs_ty); diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index ad0a5a2b6242c..cc7a87544bce1 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -534,7 +534,8 @@ 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 = lifetime `{$victim}` is required to be equal to `{$candidate}`, and is redundant and can be removed +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 -> diff --git a/compiler/rustc_lint/src/redundant_lifetime_args.rs b/compiler/rustc_lint/src/redundant_lifetime_args.rs index 7a7c5387369ea..59d2edba124af 100644 --- a/compiler/rustc_lint/src/redundant_lifetime_args.rs +++ b/compiler/rustc_lint/src/redundant_lifetime_args.rs @@ -8,19 +8,13 @@ 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! { - /// Docs - pub REDUNDANT_LIFETIME_ARGS, - Allow, - "do something" -} - -declare_lint_pass!(RedundantLifetimeArgs => [REDUNDANT_LIFETIME_ARGS]); +declare_lint_pass!(RedundantLifetimeArgs => []); impl<'tcx> LateLintPass<'tcx> for RedundantLifetimeArgs { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { @@ -142,10 +136,10 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: if infcx.resolve_regions(outlives_env).is_empty() { shadowed.insert(victim); tcx.emit_spanned_lint( - REDUNDANT_LIFETIME_ARGS, + UNUSED_LIFETIMES, tcx.local_def_id_to_hir_id(def_id.expect_local()), tcx.def_span(def_id), - RedundantLifetimeArgsList { candidate, victim }, + RedundantLifetimeArgsLint { candidate, victim }, ); } } @@ -154,7 +148,8 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: #[derive(LintDiagnostic)] #[diag(lint_redundant_lifetime_args)] -struct RedundantLifetimeArgsList<'tcx> { +#[note] +struct RedundantLifetimeArgsLint<'tcx> { candidate: ty::Region<'tcx>, victim: ty::Region<'tcx>, } diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs index a3f3b1a6d4d9a..1d427e89c21d8 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs @@ -1,8 +1,5 @@ -#![warn(unused_lifetimes)] - pub trait X { type Y<'a: 'static>; - //~^ WARNING unnecessary lifetime parameter } impl X for () { diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr index 8d21b9172c87a..cd2422b4f2d76 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr @@ -1,18 +1,5 @@ -warning: unnecessary lifetime parameter `'a` - --> $DIR/unsatisfied-item-lifetime-bound.rs:4:12 - | -LL | type Y<'a: 'static>; - | ^^ - | - = help: 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[E0478]: lifetime bound not satisfied - --> $DIR/unsatisfied-item-lifetime-bound.rs:9:18 + --> $DIR/unsatisfied-item-lifetime-bound.rs:6:18 | LL | type Y<'a: 'static>; | ------------------- definition of `Y` from trait @@ -21,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:9:12 + --> $DIR/unsatisfied-item-lifetime-bound.rs:6:12 | LL | type Y<'a> = &'a (); | ^^ @@ -32,44 +19,44 @@ LL | type Y<'a> = &'a () where 'a: 'static; | +++++++++++++++++ error[E0478]: lifetime bound not satisfied - --> $DIR/unsatisfied-item-lifetime-bound.rs:14:8 + --> $DIR/unsatisfied-item-lifetime-bound.rs:11:8 | LL | f: ::Y<'a>, | ^^^^^^^^^^^^^^^ | note: lifetime parameter instantiated with the lifetime `'a` as defined here - --> $DIR/unsatisfied-item-lifetime-bound.rs:13:10 + --> $DIR/unsatisfied-item-lifetime-bound.rs:10:10 | LL | struct B<'a, T: for<'r> X = &'r ()>> { | ^^ = note: but lifetime parameter must outlive the static lifetime error[E0478]: lifetime bound not satisfied - --> $DIR/unsatisfied-item-lifetime-bound.rs:19:8 + --> $DIR/unsatisfied-item-lifetime-bound.rs:16:8 | LL | f: ::Y<'a>, | ^^^^^^^^^^^^^^^ | note: lifetime parameter instantiated with the lifetime `'a` as defined here - --> $DIR/unsatisfied-item-lifetime-bound.rs:18:10 + --> $DIR/unsatisfied-item-lifetime-bound.rs:15: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:24:8 + --> $DIR/unsatisfied-item-lifetime-bound.rs:21:8 | LL | f: <() as X>::Y<'a>, | ^^^^^^^^^^^^^^^^ | note: lifetime parameter instantiated with the lifetime `'a` as defined here - --> $DIR/unsatisfied-item-lifetime-bound.rs:23:10 + --> $DIR/unsatisfied-item-lifetime-bound.rs:20:10 | LL | struct D<'a> { | ^^ = note: but lifetime parameter must outlive the static lifetime -error: aborting due to 4 previous errors; 1 warning emitted +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0478`. diff --git a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs index f6a628e97f56d..5778466f92eff 100644 --- a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs +++ b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs @@ -10,8 +10,8 @@ #![warn(unused_lifetimes)] -fn test<'a,'b>(x: &'a i32) -> &'b i32 - where 'a: 'static //~ WARN unnecessary lifetime parameter `'a` +fn test<'a,'b>(x: &'a i32) -> &'b i32 //~ WARN unnecessary lifetime parameter `'a` + where 'a: 'static { x } diff --git a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr index 9f03a6553ba83..69f409f436e9e 100644 --- a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr +++ b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr @@ -1,10 +1,10 @@ warning: unnecessary lifetime parameter `'a` - --> $DIR/regions-free-region-outlives-static-outlives-free-region.rs:14:11 + --> $DIR/regions-free-region-outlives-static-outlives-free-region.rs:13:9 | -LL | where 'a: 'static - | ^^ +LL | fn test<'a,'b>(x: &'a i32) -> &'b i32 + | ^^ | - = help: you can use the `'static` lifetime directly, in place of `'a` + = note: you can use the `'static` lifetime directly, in place of `'a` note: the lint level is defined here --> $DIR/regions-free-region-outlives-static-outlives-free-region.rs:11:9 | diff --git a/tests/ui/regions/regions-static-bound-rpass.rs b/tests/ui/regions/regions-static-bound-rpass.rs index 27da42882f3a6..028df1ac598d1 100644 --- a/tests/ui/regions/regions-static-bound-rpass.rs +++ b/tests/ui/regions/regions-static-bound-rpass.rs @@ -3,16 +3,17 @@ #![warn(unused_lifetimes)] fn invariant_id<'a,'b>(t: &'b mut &'static ()) -> &'b mut &'a () - where 'a: 'static { t } //~^ WARN unnecessary lifetime parameter `'a` + where 'a: 'static { t } fn static_id<'a>(t: &'a ()) -> &'static () - where 'a: 'static { t } //~^ WARN unnecessary lifetime parameter `'a` + where 'a: 'static { t } 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 } -//~^ WARN unnecessary lifetime parameter `'b` fn ref_id<'a>(t: &'a ()) -> &'a () where 'static: 'a { t } diff --git a/tests/ui/regions/regions-static-bound-rpass.stderr b/tests/ui/regions/regions-static-bound-rpass.stderr index f0f3a4c5261aa..d197266380a5a 100644 --- a/tests/ui/regions/regions-static-bound-rpass.stderr +++ b/tests/ui/regions/regions-static-bound-rpass.stderr @@ -1,10 +1,10 @@ warning: unnecessary lifetime parameter `'a` - --> $DIR/regions-static-bound-rpass.rs:6:11 + --> $DIR/regions-static-bound-rpass.rs:5:17 | -LL | where 'a: 'static { t } - | ^^ +LL | fn invariant_id<'a,'b>(t: &'b mut &'static ()) -> &'b mut &'a () + | ^^ | - = help: you can use the `'static` lifetime directly, in place of `'a` + = note: you can use the `'static` lifetime directly, in place of `'a` note: the lint level is defined here --> $DIR/regions-static-bound-rpass.rs:3:9 | @@ -12,20 +12,28 @@ LL | #![warn(unused_lifetimes)] | ^^^^^^^^^^^^^^^^ warning: unnecessary lifetime parameter `'a` - --> $DIR/regions-static-bound-rpass.rs:10:11 + --> $DIR/regions-static-bound-rpass.rs:9:14 | -LL | where 'a: 'static { t } - | ^^ +LL | fn static_id<'a>(t: &'a ()) -> &'static () + | ^^ | - = help: you can use the `'static` lifetime directly, in place of `'a` + = note: you can use the `'static` lifetime directly, in place of `'a` + +warning: unnecessary lifetime parameter `'a` + --> $DIR/regions-static-bound-rpass.rs:13: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-rpass.rs:14:19 + --> $DIR/regions-static-bound-rpass.rs:13:26 | -LL | where 'a: 'b, 'b: 'static { t } - | ^^ +LL | fn static_id_indirect<'a,'b>(t: &'a ()) -> &'static () + | ^^ | - = help: you can use the `'static` lifetime directly, in place of `'b` + = note: you can use the `'static` lifetime directly, in place of `'b` -warning: 3 warnings emitted +warning: 4 warnings emitted diff --git a/tests/ui/regions/regions-static-bound.rs b/tests/ui/regions/regions-static-bound.rs index e7aa8795f01a6..af6425971bd12 100644 --- a/tests/ui/regions/regions-static-bound.rs +++ b/tests/ui/regions/regions-static-bound.rs @@ -1,12 +1,7 @@ -#![warn(unused_lifetimes)] - fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t } -//~^ WARN lifetime parameter `'b` never used -//~| WARN unnecessary lifetime parameter `'a` fn static_id_indirect<'a,'b>(t: &'a ()) -> &'static () where 'a: 'b, 'b: 'static { t } -//~^ WARN unnecessary lifetime parameter `'b` fn static_id_wrong_way<'a>(t: &'a ()) -> &'static () where 'static: 'a { t diff --git a/tests/ui/regions/regions-static-bound.stderr b/tests/ui/regions/regions-static-bound.stderr index b314e9fe85d4e..f7b48349e4a3f 100644 --- a/tests/ui/regions/regions-static-bound.stderr +++ b/tests/ui/regions/regions-static-bound.stderr @@ -1,35 +1,5 @@ -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:53 - | -LL | fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t } - | ^^ - | - = help: you can use the `'static` lifetime directly, in place of `'a` - -warning: unnecessary lifetime parameter `'b` - --> $DIR/regions-static-bound.rs:8:19 - | -LL | where 'a: 'b, 'b: 'static { t } - | ^^ - | - = help: you can use the `'static` lifetime directly, in place of `'b` - error: lifetime may not live long enough - --> $DIR/regions-static-bound.rs:12:5 + --> $DIR/regions-static-bound.rs:7:5 | LL | fn static_id_wrong_way<'a>(t: &'a ()) -> &'static () where 'static: 'a { | -- lifetime `'a` defined here @@ -37,7 +7,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:17:5 + --> $DIR/regions-static-bound.rs:12:5 | LL | fn error(u: &(), v: &()) { | - - let's call the lifetime of this reference `'1` @@ -50,7 +20,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:19:5 + --> $DIR/regions-static-bound.rs:14:5 | LL | fn error(u: &(), v: &()) { | - - let's call the lifetime of this reference `'2` @@ -63,6 +33,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; 3 warnings emitted +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0521`. diff --git a/tests/ui/regions/transitively-redundant-lifetimes.rs b/tests/ui/regions/transitively-redundant-lifetimes.rs new file mode 100644 index 0000000000000..3baf2a88abb40 --- /dev/null +++ b/tests/ui/regions/transitively-redundant-lifetimes.rs @@ -0,0 +1,16 @@ +#![allow(unused)] +#![deny(unused_lifetimes)] + +fn a<'a, 'b>(x: &'a &'b &'a ()) {} //~ ERROR unnecessary lifetime parameter `'b` + +fn b<'a: 'b, 'b: 'a>() {} //~ ERROR unnecessary lifetime parameter `'b` + +struct Foo(T); +fn c<'a>(_: Foo<&'a ()>) {} //~ ERROR unnecessary lifetime parameter `'a` + +struct Bar<'a>(&'a ()); +impl<'a> Bar<'a> { + fn d<'b: 'a>(&'b self) {} //~ ERROR unnecessary lifetime parameter `'b` +} + +fn main() {} diff --git a/tests/ui/regions/transitively-redundant-lifetimes.stderr b/tests/ui/regions/transitively-redundant-lifetimes.stderr new file mode 100644 index 0000000000000..22a27115f7ae2 --- /dev/null +++ b/tests/ui/regions/transitively-redundant-lifetimes.stderr @@ -0,0 +1,39 @@ +error: unnecessary lifetime parameter `'b` + --> $DIR/transitively-redundant-lifetimes.rs:4:10 + | +LL | fn a<'a, 'b>(x: &'a &'b &'a ()) {} + | ^^ + | + = note: you can use the `'a` lifetime directly, in place of `'b` +note: the lint level is defined here + --> $DIR/transitively-redundant-lifetimes.rs:2:9 + | +LL | #![deny(unused_lifetimes)] + | ^^^^^^^^^^^^^^^^ + +error: unnecessary lifetime parameter `'b` + --> $DIR/transitively-redundant-lifetimes.rs:6:14 + | +LL | fn b<'a: 'b, 'b: 'a>() {} + | ^^ + | + = note: you can use the `'a` lifetime directly, in place of `'b` + +error: unnecessary lifetime parameter `'a` + --> $DIR/transitively-redundant-lifetimes.rs:9:6 + | +LL | fn c<'a>(_: Foo<&'a ()>) {} + | ^^ + | + = note: you can use the `'static` lifetime directly, in place of `'a` + +error: unnecessary lifetime parameter `'b` + --> $DIR/transitively-redundant-lifetimes.rs:13:10 + | +LL | fn d<'b: 'a>(&'b self) {} + | ^^ + | + = note: you can use the `'a` lifetime directly, in place of `'b` + +error: aborting due to 4 previous errors + From 535151ed03ba629393abcc6f927f38c1919ed70e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Nov 2023 22:05:46 +0000 Subject: [PATCH 3/8] Add comments --- .../rustc_lint/src/redundant_lifetime_args.rs | 34 +++++++++++++++---- compiler/rustc_lint_defs/src/builtin.rs | 6 ++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_lint/src/redundant_lifetime_args.rs b/compiler/rustc_lint/src/redundant_lifetime_args.rs index 59d2edba124af..34ff5e9dfe98b 100644 --- a/compiler/rustc_lint/src/redundant_lifetime_args.rs +++ b/compiler/rustc_lint/src/redundant_lifetime_args.rs @@ -82,17 +82,28 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: 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 }; @@ -101,20 +112,23 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: } // 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) { - // Don't suggest removing a lifetime twice. continue; } + // Can't rename a named lifetime named `'_` without ambiguity. if !candidate.has_name() { - // Can't rename a named lifetime with `'_` without ambiguity. 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, _), @@ -124,15 +138,21 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: 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() { - // Do not rename generics not local to this item since - // they'll overlap with this lint running on the parent. 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( @@ -150,6 +170,8 @@ fn check<'tcx>(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, owner_id: hir:: #[diag(lint_redundant_lifetime_args)] #[note] struct RedundantLifetimeArgsLint<'tcx> { - candidate: ty::Region<'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>, } diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 53b5273803c6f..c2df3e1015faf 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1694,6 +1694,12 @@ declare_lint! { /// #[deny(unused_lifetimes)] /// /// pub fn foo<'a>() {} + /// + /// // `'a = 'static`, so all usages of `'a` can be replaced with `'static` + /// pub fn bar<'a: 'static>() {} + /// + /// // `'a = 'b`, so all usages of `'b` can be replaced with `'a` + /// pub fn bar<'a: 'b, 'b: 'a>() {} /// ``` /// /// {{produces}} From ceff692a023ac37a5a8049be0b38551ee36093c7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Nov 2023 22:40:54 +0000 Subject: [PATCH 4/8] Fix stage 2 --- compiler/rustc_middle/src/ty/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 0daf83162dbbf..6275c5d2a1194 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -757,7 +757,7 @@ pub struct GlobalCtxt<'tcx> { impl<'tcx> GlobalCtxt<'tcx> { /// Installs `self` in a `TyCtxt` and `ImplicitCtxt` for the duration of /// `f`. - pub fn enter<'a: 'tcx, F, R>(&'a self, f: F) -> R + pub fn enter(&'tcx self, f: F) -> R where F: FnOnce(TyCtxt<'tcx>) -> R, { From 2d813547bfec8b2829bf411794811df17de52df5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 29 Nov 2023 02:41:31 +0000 Subject: [PATCH 5/8] Move check to wfcheck --- compiler/rustc_hir_analysis/messages.ftl | 3 + .../rustc_hir_analysis/src/check/wfcheck.rs | 128 +++++++++++++ compiler/rustc_lint/messages.ftl | 3 - compiler/rustc_lint/src/lib.rs | 3 - .../rustc_lint/src/redundant_lifetime_args.rs | 177 ------------------ .../unsatisfied-item-lifetime-bound.rs | 4 +- .../unsatisfied-item-lifetime-bound.stderr | 31 ++- tests/ui/regions/regions-static-bound.rs | 6 + tests/ui/regions/regions-static-bound.stderr | 46 ++++- .../transitively-redundant-lifetimes.rs | 2 + 10 files changed, 206 insertions(+), 197 deletions(-) delete mode 100644 compiler/rustc_lint/src/redundant_lifetime_args.rs diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index e66a834ab9e88..86b8b6d6b2b6b 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -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 = diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 4fd7c870fc730..cc7c41cb387b5 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -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 }; } diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index cc7a87544bce1..82b90e1660a95 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -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 diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index d13c954947614..31c80c4d75bb1 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -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, ] ] ); diff --git a/compiler/rustc_lint/src/redundant_lifetime_args.rs b/compiler/rustc_lint/src/redundant_lifetime_args.rs deleted file mode 100644 index 34ff5e9dfe98b..0000000000000 --- a/compiler/rustc_lint/src/redundant_lifetime_args.rs +++ /dev/null @@ -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>, -} diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs index 1d427e89c21d8..381d094d62621 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs @@ -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 () { diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr index cd2422b4f2d76..f1a7511c634b5 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr @@ -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: ::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 = &'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: ::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`. diff --git a/tests/ui/regions/regions-static-bound.rs b/tests/ui/regions/regions-static-bound.rs index af6425971bd12..3849dfa122f49 100644 --- a/tests/ui/regions/regions-static-bound.rs +++ b/tests/ui/regions/regions-static-bound.rs @@ -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 { diff --git a/tests/ui/regions/regions-static-bound.stderr b/tests/ui/regions/regions-static-bound.stderr index f7b48349e4a3f..4638ca9e35078 100644 --- a/tests/ui/regions/regions-static-bound.stderr +++ b/tests/ui/regions/regions-static-bound.stderr @@ -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`. diff --git a/tests/ui/regions/transitively-redundant-lifetimes.rs b/tests/ui/regions/transitively-redundant-lifetimes.rs index 3baf2a88abb40..af1195fe94f58 100644 --- a/tests/ui/regions/transitively-redundant-lifetimes.rs +++ b/tests/ui/regions/transitively-redundant-lifetimes.rs @@ -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() {} From ee78eab62be926b69236c9a344fbcec2b3355bbd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 5 Dec 2023 16:32:47 +0000 Subject: [PATCH 6/8] Lint redundant lifetimes in impl header --- compiler/rustc_hir_analysis/src/check/wfcheck.rs | 11 ++++++----- tests/ui/regions/transitively-redundant-lifetimes.rs | 3 +++ .../regions/transitively-redundant-lifetimes.stderr | 10 +++++++++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index cc7c41cb387b5..01f87ecdebb5e 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -2028,19 +2028,20 @@ fn lint_redundant_lifetimes<'tcx>( | DefKind::TraitAlias | DefKind::Fn | DefKind::Const - | DefKind::Impl { of_trait: false } => { + | DefKind::Impl { of_trait: _ } => { // 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. + // Don't check for redundant lifetimes for associated items of trait + // implementations, since the signature is required to be compatible + // with the trait, even if the implementation implies some lifetimes + // are redundant. return; } } - DefKind::Impl { of_trait: true } - | DefKind::Mod + DefKind::Mod | DefKind::Variant | DefKind::TyAlias | DefKind::ForeignTy diff --git a/tests/ui/regions/transitively-redundant-lifetimes.rs b/tests/ui/regions/transitively-redundant-lifetimes.rs index af1195fe94f58..9d375550de3e0 100644 --- a/tests/ui/regions/transitively-redundant-lifetimes.rs +++ b/tests/ui/regions/transitively-redundant-lifetimes.rs @@ -15,4 +15,7 @@ impl<'a> Bar<'a> { fn ok(x: &'static &()) {} +trait Tr<'a> {} +impl<'a: 'static> Tr<'a> for () {} //~ ERROR unnecessary lifetime parameter `'a` + fn main() {} diff --git a/tests/ui/regions/transitively-redundant-lifetimes.stderr b/tests/ui/regions/transitively-redundant-lifetimes.stderr index 22a27115f7ae2..a35942ca9801b 100644 --- a/tests/ui/regions/transitively-redundant-lifetimes.stderr +++ b/tests/ui/regions/transitively-redundant-lifetimes.stderr @@ -27,6 +27,14 @@ LL | fn c<'a>(_: Foo<&'a ()>) {} | = note: you can use the `'static` lifetime directly, in place of `'a` +error: unnecessary lifetime parameter `'a` + --> $DIR/transitively-redundant-lifetimes.rs:19:6 + | +LL | impl<'a: 'static> Tr<'a> for () {} + | ^^ + | + = note: you can use the `'static` lifetime directly, in place of `'a` + error: unnecessary lifetime parameter `'b` --> $DIR/transitively-redundant-lifetimes.rs:13:10 | @@ -35,5 +43,5 @@ LL | fn d<'b: 'a>(&'b self) {} | = note: you can use the `'a` lifetime directly, in place of `'b` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors From a9e262a32dbef558bbe40731ddfe272cb2f11948 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 16 Dec 2023 01:58:26 +0000 Subject: [PATCH 7/8] Split back out unused_lifetimes -> redundant_lifetimes --- .../rustc_hir_analysis/src/check/wfcheck.rs | 4 +-- compiler/rustc_lint_defs/src/builtin.rs | 26 +++++++++++++++++-- .../unsatisfied-item-lifetime-bound.rs | 2 +- .../unsatisfied-item-lifetime-bound.stderr | 6 ++--- ...on-outlives-static-outlives-free-region.rs | 2 +- ...utlives-static-outlives-free-region.stderr | 4 +-- .../ui/regions/regions-static-bound-rpass.rs | 2 +- .../regions/regions-static-bound-rpass.stderr | 4 +-- tests/ui/regions/regions-static-bound.rs | 2 +- tests/ui/regions/regions-static-bound.stderr | 7 ++++- .../transitively-redundant-lifetimes.rs | 3 +-- .../transitively-redundant-lifetimes.stderr | 16 ++++++------ 12 files changed, 52 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 01f87ecdebb5e..cc54db18750db 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -2118,8 +2118,8 @@ fn lint_redundant_lifetimes<'tcx>( && 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.emit_node_span_lint( + rustc_lint_defs::builtin::REDUNDANT_LIFETIMES, tcx.local_def_id_to_hir_id(def_id.expect_local()), tcx.def_span(def_id), RedundantLifetimeArgsLint { candidate, victim }, diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index c2df3e1015faf..2713690f8120a 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -79,6 +79,7 @@ declare_lint_pass! { PROC_MACRO_BACK_COMPAT, PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, PUB_USE_OF_PRIVATE_EXTERN_CRATE, + REDUNDANT_LIFETIMES, REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE, RENAMED_AND_REMOVED_LINTS, @@ -1694,6 +1695,27 @@ declare_lint! { /// #[deny(unused_lifetimes)] /// /// pub fn foo<'a>() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Unused lifetime parameters may signal a mistake or unfinished code. + /// Consider removing the parameter. + pub UNUSED_LIFETIMES, + Allow, + "detects lifetime parameters that are never used" +} + +declare_lint! { + /// The `redundant_lifetimes` lint detects lifetime parameters that are + /// redundant because they are equal to another named lifetime. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #[deny(redundant_lifetimes)] /// /// // `'a = 'static`, so all usages of `'a` can be replaced with `'static` /// pub fn bar<'a: 'static>() {} @@ -1708,9 +1730,9 @@ declare_lint! { /// /// Unused lifetime parameters may signal a mistake or unfinished code. /// Consider removing the parameter. - pub UNUSED_LIFETIMES, + pub REDUNDANT_LIFETIMES, Allow, - "detects lifetime parameters that are never used" + "detects lifetime parameters that are redundant because they are equal to some other named lifetime" } declare_lint! { diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs index 381d094d62621..e06341ddf3185 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.rs @@ -1,4 +1,4 @@ -#![warn(unused_lifetimes)] +#![warn(unused_lifetimes, redundant_lifetimes)] pub trait X { type Y<'a: 'static>; //~ WARN unnecessary lifetime parameter `'a` diff --git a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr index f1a7511c634b5..4f41ec025fc7f 100644 --- a/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr +++ b/tests/ui/generic-associated-types/unsatisfied-item-lifetime-bound.stderr @@ -65,10 +65,10 @@ 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 + --> $DIR/unsatisfied-item-lifetime-bound.rs:1:27 | -LL | #![warn(unused_lifetimes)] - | ^^^^^^^^^^^^^^^^ +LL | #![warn(unused_lifetimes, redundant_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^ error: aborting due to 4 previous errors; 1 warning emitted diff --git a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs index 5778466f92eff..bef0d70c77659 100644 --- a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs +++ b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.rs @@ -8,7 +8,7 @@ // // 'a : 'b -#![warn(unused_lifetimes)] +#![warn(redundant_lifetimes)] fn test<'a,'b>(x: &'a i32) -> &'b i32 //~ WARN unnecessary lifetime parameter `'a` where 'a: 'static diff --git a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr index 69f409f436e9e..d97cfd59f2ba4 100644 --- a/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr +++ b/tests/ui/regions/regions-free-region-outlives-static-outlives-free-region.stderr @@ -8,8 +8,8 @@ LL | fn test<'a,'b>(x: &'a i32) -> &'b i32 note: the lint level is defined here --> $DIR/regions-free-region-outlives-static-outlives-free-region.rs:11:9 | -LL | #![warn(unused_lifetimes)] - | ^^^^^^^^^^^^^^^^ +LL | #![warn(redundant_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^ warning: 1 warning emitted diff --git a/tests/ui/regions/regions-static-bound-rpass.rs b/tests/ui/regions/regions-static-bound-rpass.rs index 028df1ac598d1..f4177f835b15c 100644 --- a/tests/ui/regions/regions-static-bound-rpass.rs +++ b/tests/ui/regions/regions-static-bound-rpass.rs @@ -1,6 +1,6 @@ //@ run-pass -#![warn(unused_lifetimes)] +#![warn(redundant_lifetimes)] fn invariant_id<'a,'b>(t: &'b mut &'static ()) -> &'b mut &'a () //~^ WARN unnecessary lifetime parameter `'a` diff --git a/tests/ui/regions/regions-static-bound-rpass.stderr b/tests/ui/regions/regions-static-bound-rpass.stderr index d197266380a5a..4199ac7bb3d38 100644 --- a/tests/ui/regions/regions-static-bound-rpass.stderr +++ b/tests/ui/regions/regions-static-bound-rpass.stderr @@ -8,8 +8,8 @@ LL | fn invariant_id<'a,'b>(t: &'b mut &'static ()) -> &'b mut &'a () note: the lint level is defined here --> $DIR/regions-static-bound-rpass.rs:3:9 | -LL | #![warn(unused_lifetimes)] - | ^^^^^^^^^^^^^^^^ +LL | #![warn(redundant_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^ warning: unnecessary lifetime parameter `'a` --> $DIR/regions-static-bound-rpass.rs:9:14 diff --git a/tests/ui/regions/regions-static-bound.rs b/tests/ui/regions/regions-static-bound.rs index 3849dfa122f49..32fa2536533cc 100644 --- a/tests/ui/regions/regions-static-bound.rs +++ b/tests/ui/regions/regions-static-bound.rs @@ -1,4 +1,4 @@ -#![warn(unused_lifetimes)] +#![warn(unused_lifetimes, redundant_lifetimes)] fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t } //~^ WARN unnecessary lifetime parameter `'a` diff --git a/tests/ui/regions/regions-static-bound.stderr b/tests/ui/regions/regions-static-bound.stderr index 4638ca9e35078..48aa8f323291d 100644 --- a/tests/ui/regions/regions-static-bound.stderr +++ b/tests/ui/regions/regions-static-bound.stderr @@ -9,7 +9,7 @@ LL | fn static_id<'a,'b>(t: &'a ()) -> &'static () where 'a: 'static { t } note: the lint level is defined here --> $DIR/regions-static-bound.rs:1:9 | -LL | #![warn(unused_lifetimes)] +LL | #![warn(unused_lifetimes, redundant_lifetimes)] | ^^^^^^^^^^^^^^^^ warning: unnecessary lifetime parameter `'a` @@ -19,6 +19,11 @@ 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` +note: the lint level is defined here + --> $DIR/regions-static-bound.rs:1:27 + | +LL | #![warn(unused_lifetimes, redundant_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^ warning: unnecessary lifetime parameter `'a` --> $DIR/regions-static-bound.rs:7:23 diff --git a/tests/ui/regions/transitively-redundant-lifetimes.rs b/tests/ui/regions/transitively-redundant-lifetimes.rs index 9d375550de3e0..9c29f66e54c5a 100644 --- a/tests/ui/regions/transitively-redundant-lifetimes.rs +++ b/tests/ui/regions/transitively-redundant-lifetimes.rs @@ -1,5 +1,4 @@ -#![allow(unused)] -#![deny(unused_lifetimes)] +#![deny(redundant_lifetimes)] fn a<'a, 'b>(x: &'a &'b &'a ()) {} //~ ERROR unnecessary lifetime parameter `'b` diff --git a/tests/ui/regions/transitively-redundant-lifetimes.stderr b/tests/ui/regions/transitively-redundant-lifetimes.stderr index a35942ca9801b..2d8fc433b241f 100644 --- a/tests/ui/regions/transitively-redundant-lifetimes.stderr +++ b/tests/ui/regions/transitively-redundant-lifetimes.stderr @@ -1,18 +1,18 @@ error: unnecessary lifetime parameter `'b` - --> $DIR/transitively-redundant-lifetimes.rs:4:10 + --> $DIR/transitively-redundant-lifetimes.rs:3:10 | LL | fn a<'a, 'b>(x: &'a &'b &'a ()) {} | ^^ | = note: you can use the `'a` lifetime directly, in place of `'b` note: the lint level is defined here - --> $DIR/transitively-redundant-lifetimes.rs:2:9 + --> $DIR/transitively-redundant-lifetimes.rs:1:9 | -LL | #![deny(unused_lifetimes)] - | ^^^^^^^^^^^^^^^^ +LL | #![deny(redundant_lifetimes)] + | ^^^^^^^^^^^^^^^^^^^ error: unnecessary lifetime parameter `'b` - --> $DIR/transitively-redundant-lifetimes.rs:6:14 + --> $DIR/transitively-redundant-lifetimes.rs:5:14 | LL | fn b<'a: 'b, 'b: 'a>() {} | ^^ @@ -20,7 +20,7 @@ LL | fn b<'a: 'b, 'b: 'a>() {} = note: you can use the `'a` lifetime directly, in place of `'b` error: unnecessary lifetime parameter `'a` - --> $DIR/transitively-redundant-lifetimes.rs:9:6 + --> $DIR/transitively-redundant-lifetimes.rs:8:6 | LL | fn c<'a>(_: Foo<&'a ()>) {} | ^^ @@ -28,7 +28,7 @@ LL | fn c<'a>(_: Foo<&'a ()>) {} = note: you can use the `'static` lifetime directly, in place of `'a` error: unnecessary lifetime parameter `'a` - --> $DIR/transitively-redundant-lifetimes.rs:19:6 + --> $DIR/transitively-redundant-lifetimes.rs:18:6 | LL | impl<'a: 'static> Tr<'a> for () {} | ^^ @@ -36,7 +36,7 @@ LL | impl<'a: 'static> Tr<'a> for () {} = note: you can use the `'static` lifetime directly, in place of `'a` error: unnecessary lifetime parameter `'b` - --> $DIR/transitively-redundant-lifetimes.rs:13:10 + --> $DIR/transitively-redundant-lifetimes.rs:12:10 | LL | fn d<'b: 'a>(&'b self) {} | ^^ From da2b714ba122d7befe1f10c808e82a536c9efcf0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 27 Mar 2024 16:02:13 -0400 Subject: [PATCH 8/8] Clarifying comment --- compiler/rustc_hir_analysis/src/check/wfcheck.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index cc54db18750db..c26f982fa473c 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -2089,13 +2089,19 @@ fn lint_redundant_lifetimes<'tcx>( let mut shadowed = FxHashSet::default(); for (idx, &candidate) in lifetimes.iter().enumerate() { - // Don't suggest removing a lifetime twice. + // Don't suggest removing a lifetime twice. We only need to check this + // here and not up in the `victim` loop because equality is transitive, + // so if A = C and B = C, then A must = B, so it'll be shadowed too in + // A's victim loop. 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. + // We should only have late-bound lifetimes of the `BrNamed` variety, + // since we get these signatures straight from `hir_lowering`. And any + // other regions (ReError/ReStatic/etc.) shouldn't matter, since we + // can't really suggest to remove them. let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) | ty::ReLateParam(ty::LateParamRegion { bound_region: ty::BoundRegionKind::BrNamed(def_id, _), @@ -2113,7 +2119,7 @@ fn lint_redundant_lifetimes<'tcx>( continue; } - // If there are no lifetime errors, then we have proven that `'candidate = 'victim`! + // If `candidate <: victim` and `victim <: candidate`, then they're equal. if outlives_env.free_region_map().sub_free_regions(tcx, candidate, victim) && outlives_env.free_region_map().sub_free_regions(tcx, victim, candidate) {