From 775b893b5ce80a33ae4ace718a13fb87f31b2e1d Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 22 Sep 2025 08:56:15 -0500 Subject: [PATCH 1/4] Use ty reference --- compiler/rustc_lint/src/lifetime_syntax.rs | 35 ++++++---------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_lint/src/lifetime_syntax.rs b/compiler/rustc_lint/src/lifetime_syntax.rs index 413525eb6e514..ed253c6a26d23 100644 --- a/compiler/rustc_lint/src/lifetime_syntax.rs +++ b/compiler/rustc_lint/src/lifetime_syntax.rs @@ -517,9 +517,8 @@ fn build_mismatch_suggestion( #[derive(Debug)] struct Info<'tcx> { - type_span: Span, - referenced_type_span: Option, lifetime: &'tcx hir::Lifetime, + ty: &'tcx hir::Ty<'tcx>, } impl<'tcx> Info<'tcx> { @@ -543,7 +542,7 @@ impl<'tcx> Info<'tcx> { /// to include the type. Otherwise we end up pointing at nothing, /// which is a bit confusing. fn reporting_span(&self) -> Span { - if self.lifetime.is_implicit() { self.type_span } else { self.lifetime.ident.span } + if self.lifetime.is_implicit() { self.ty.span } else { self.lifetime.ident.span } } /// When removing an explicit lifetime from a reference, @@ -561,11 +560,9 @@ impl<'tcx> Info<'tcx> { // FIXME: Ideally, we'd also remove the lifetime declaration. fn removing_span(&self) -> Span { let mut span = self.suggestion("'dummy").0; - - if let Some(referenced_type_span) = self.referenced_type_span { - span = span.until(referenced_type_span); + if let hir::TyKind::Ref(_, mut_ty) = self.ty.kind { + span = span.until(mut_ty.ty.span); } - span } @@ -577,14 +574,13 @@ impl<'tcx> Info<'tcx> { type LifetimeInfoMap<'tcx> = FxIndexMap<&'tcx hir::LifetimeKind, Vec>>; struct LifetimeInfoCollector<'a, 'tcx> { - type_span: Span, - referenced_type_span: Option, map: &'a mut LifetimeInfoMap<'tcx>, + ty: &'tcx hir::Ty<'tcx>, } impl<'a, 'tcx> LifetimeInfoCollector<'a, 'tcx> { fn collect(ty: &'tcx hir::Ty<'tcx>, map: &'a mut LifetimeInfoMap<'tcx>) { - let mut this = Self { type_span: ty.span, referenced_type_span: None, map }; + let mut this = Self { map, ty }; intravisit::walk_unambig_ty(&mut this, ty); } @@ -593,27 +589,14 @@ impl<'a, 'tcx> LifetimeInfoCollector<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for LifetimeInfoCollector<'a, 'tcx> { #[instrument(skip(self))] fn visit_lifetime(&mut self, lifetime: &'tcx hir::Lifetime) { - let type_span = self.type_span; - let referenced_type_span = self.referenced_type_span; - - let info = Info { type_span, referenced_type_span, lifetime }; - + let info = Info { lifetime, ty: self.ty }; self.map.entry(&lifetime.kind).or_default().push(info); } #[instrument(skip(self))] fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx, hir::AmbigArg>) -> Self::Result { - let old_type_span = self.type_span; - let old_referenced_type_span = self.referenced_type_span; - - self.type_span = ty.span; - if let hir::TyKind::Ref(_, ty) = ty.kind { - self.referenced_type_span = Some(ty.ty.span); - } - + let old_ty = std::mem::replace(&mut self.ty, ty.as_unambig_ty()); intravisit::walk_ty(self, ty); - - self.type_span = old_type_span; - self.referenced_type_span = old_referenced_type_span; + self.ty = old_ty; } } From c915c989f670590bf81e5b87bf745678c6b9a4d3 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 22 Sep 2025 09:07:54 -0500 Subject: [PATCH 2/4] Eagerly evaluate lifetime syntax category --- compiler/rustc_lint/src/lifetime_syntax.rs | 42 ++++++---------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_lint/src/lifetime_syntax.rs b/compiler/rustc_lint/src/lifetime_syntax.rs index ed253c6a26d23..5a8a324d846da 100644 --- a/compiler/rustc_lint/src/lifetime_syntax.rs +++ b/compiler/rustc_lint/src/lifetime_syntax.rs @@ -148,11 +148,11 @@ enum LifetimeSyntaxCategory { } impl LifetimeSyntaxCategory { - fn new(syntax_source: (hir::LifetimeSyntax, LifetimeSource)) -> Option { + fn new(lifetime: &hir::Lifetime) -> Option { use LifetimeSource::*; use hir::LifetimeSyntax::*; - match syntax_source { + match (lifetime.syntax, lifetime.source) { // E.g. `&T`. (Implicit, Reference) | // E.g. `&'_ T`. @@ -233,22 +233,8 @@ impl std::ops::Add for LifetimeSyntaxCategories { } fn lifetimes_use_matched_syntax(input_info: &[Info<'_>], output_info: &[Info<'_>]) -> bool { - let mut syntax_counts = LifetimeSyntaxCategories::::default(); - - for info in input_info.iter().chain(output_info) { - if let Some(category) = info.lifetime_syntax_category() { - *syntax_counts.select(category) += 1; - } - } - - tracing::debug!(?syntax_counts); - - matches!( - syntax_counts, - LifetimeSyntaxCategories { hidden: _, elided: 0, named: 0 } - | LifetimeSyntaxCategories { hidden: 0, elided: _, named: 0 } - | LifetimeSyntaxCategories { hidden: 0, elided: 0, named: _ } - ) + let (first, inputs) = input_info.split_first().unwrap(); + std::iter::chain(inputs, output_info).all(|info| info.syntax_category == first.syntax_category) } fn emit_mismatch_diagnostic<'tcx>( @@ -312,11 +298,6 @@ fn emit_mismatch_diagnostic<'tcx>( let syntax_source = info.syntax_source(); - if let (_, Other) = syntax_source { - // Ignore any other kind of lifetime. - continue; - } - if let (ExplicitBound, _) = syntax_source { bound_lifetime = Some(info); } @@ -393,9 +374,7 @@ fn emit_mismatch_diagnostic<'tcx>( let categorize = |infos: &[Info<'_>]| { let mut categories = LifetimeSyntaxCategories::>::default(); for info in infos { - if let Some(category) = info.lifetime_syntax_category() { - categories.select(category).push(info.reporting_span()); - } + categories.select(info.syntax_category).push(info.reporting_span()); } categories }; @@ -518,6 +497,7 @@ fn build_mismatch_suggestion( #[derive(Debug)] struct Info<'tcx> { lifetime: &'tcx hir::Lifetime, + syntax_category: LifetimeSyntaxCategory, ty: &'tcx hir::Ty<'tcx>, } @@ -526,10 +506,6 @@ impl<'tcx> Info<'tcx> { (self.lifetime.syntax, self.lifetime.source) } - fn lifetime_syntax_category(&self) -> Option { - LifetimeSyntaxCategory::new(self.syntax_source()) - } - fn lifetime_name(&self) -> &str { self.lifetime.ident.as_str() } @@ -589,8 +565,10 @@ impl<'a, 'tcx> LifetimeInfoCollector<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for LifetimeInfoCollector<'a, 'tcx> { #[instrument(skip(self))] fn visit_lifetime(&mut self, lifetime: &'tcx hir::Lifetime) { - let info = Info { lifetime, ty: self.ty }; - self.map.entry(&lifetime.kind).or_default().push(info); + if let Some(syntax_category) = LifetimeSyntaxCategory::new(lifetime) { + let info = Info { lifetime, syntax_category, ty: self.ty }; + self.map.entry(&lifetime.kind).or_default().push(info); + } } #[instrument(skip(self))] From ba5a42b48b0fe35a98f82ab982745e12ee87966c Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 22 Sep 2025 08:24:30 -0500 Subject: [PATCH 3/4] LifetimeSyntax tweak data structure --- compiler/rustc_lint/src/lifetime_syntax.rs | 74 +++++++++++++--------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_lint/src/lifetime_syntax.rs b/compiler/rustc_lint/src/lifetime_syntax.rs index 5a8a324d846da..bcf8175cdf2e1 100644 --- a/compiler/rustc_lint/src/lifetime_syntax.rs +++ b/compiler/rustc_lint/src/lifetime_syntax.rs @@ -111,35 +111,47 @@ impl<'tcx> LateLintPass<'tcx> for LifetimeSyntax { } fn check_fn_like<'tcx>(cx: &LateContext<'tcx>, fd: &'tcx hir::FnDecl<'tcx>) { - let mut input_map = Default::default(); - let mut output_map = Default::default(); - - for input in fd.inputs { - LifetimeInfoCollector::collect(input, &mut input_map); + if fd.inputs.is_empty() { + return; } + let hir::FnRetTy::Return(output) = fd.output else { + return; + }; - if let hir::FnRetTy::Return(output) = fd.output { - LifetimeInfoCollector::collect(output, &mut output_map); - } + let mut map: FxIndexMap> = FxIndexMap::default(); - report_mismatches(cx, &input_map, &output_map); -} + LifetimeInfoCollector::collect(output, |info| { + let group = map.entry(info.lifetime.kind).or_default(); + group.outputs.push(info); + }); + if map.is_empty() { + return; + } -#[instrument(skip_all)] -fn report_mismatches<'tcx>( - cx: &LateContext<'tcx>, - inputs: &LifetimeInfoMap<'tcx>, - outputs: &LifetimeInfoMap<'tcx>, -) { - for (resolved_lifetime, output_info) in outputs { - if let Some(input_info) = inputs.get(resolved_lifetime) { - if !lifetimes_use_matched_syntax(input_info, output_info) { - emit_mismatch_diagnostic(cx, input_info, output_info); + for input in fd.inputs { + LifetimeInfoCollector::collect(input, |info| { + if let Some(group) = map.get_mut(&info.lifetime.kind) { + group.inputs.push(info); } + }); + } + + for LifetimeGroup { ref inputs, ref outputs } in map.into_values() { + if inputs.is_empty() { + continue; + } + if !lifetimes_use_matched_syntax(inputs, outputs) { + emit_mismatch_diagnostic(cx, inputs, outputs); } } } +#[derive(Default)] +struct LifetimeGroup<'tcx> { + inputs: Vec>, + outputs: Vec>, +} + #[derive(Debug, Copy, Clone, PartialEq)] enum LifetimeSyntaxCategory { Hidden, @@ -547,27 +559,31 @@ impl<'tcx> Info<'tcx> { } } -type LifetimeInfoMap<'tcx> = FxIndexMap<&'tcx hir::LifetimeKind, Vec>>; - -struct LifetimeInfoCollector<'a, 'tcx> { - map: &'a mut LifetimeInfoMap<'tcx>, +struct LifetimeInfoCollector<'tcx, F> { + info_func: F, ty: &'tcx hir::Ty<'tcx>, } -impl<'a, 'tcx> LifetimeInfoCollector<'a, 'tcx> { - fn collect(ty: &'tcx hir::Ty<'tcx>, map: &'a mut LifetimeInfoMap<'tcx>) { - let mut this = Self { map, ty }; +impl<'tcx, F> LifetimeInfoCollector<'tcx, F> +where + F: FnMut(Info<'tcx>), +{ + fn collect(ty: &'tcx hir::Ty<'tcx>, info_func: F) { + let mut this = Self { info_func, ty }; intravisit::walk_unambig_ty(&mut this, ty); } } -impl<'a, 'tcx> Visitor<'tcx> for LifetimeInfoCollector<'a, 'tcx> { +impl<'tcx, F> Visitor<'tcx> for LifetimeInfoCollector<'tcx, F> +where + F: FnMut(Info<'tcx>), +{ #[instrument(skip(self))] fn visit_lifetime(&mut self, lifetime: &'tcx hir::Lifetime) { if let Some(syntax_category) = LifetimeSyntaxCategory::new(lifetime) { let info = Info { lifetime, syntax_category, ty: self.ty }; - self.map.entry(&lifetime.kind).or_default().push(info); + (self.info_func)(info); } } From f0eea4cc4f6f87b70bf6e29382a21ead33687b84 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 22 Sep 2025 08:37:01 -0500 Subject: [PATCH 4/4] LifetimeSyntax little things --- compiler/rustc_lint/src/lifetime_syntax.rs | 66 +++++++--------------- 1 file changed, 20 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_lint/src/lifetime_syntax.rs b/compiler/rustc_lint/src/lifetime_syntax.rs index bcf8175cdf2e1..0cac91c234080 100644 --- a/compiler/rustc_lint/src/lifetime_syntax.rs +++ b/compiler/rustc_lint/src/lifetime_syntax.rs @@ -3,6 +3,7 @@ use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{self as hir, LifetimeSource}; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::Span; +use rustc_span::def_id::LocalDefId; use tracing::instrument; use crate::{LateContext, LateLintPass, LintContext, lints}; @@ -78,11 +79,11 @@ impl<'tcx> LateLintPass<'tcx> for LifetimeSyntax { fn check_fn( &mut self, cx: &LateContext<'tcx>, - _: hir::intravisit::FnKind<'tcx>, + _: intravisit::FnKind<'tcx>, fd: &'tcx hir::FnDecl<'tcx>, _: &'tcx hir::Body<'tcx>, - _: rustc_span::Span, - _: rustc_span::def_id::LocalDefId, + _: Span, + _: LocalDefId, ) { check_fn_like(cx, fd); } @@ -97,11 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for LifetimeSyntax { } #[instrument(skip_all)] - fn check_foreign_item( - &mut self, - cx: &LateContext<'tcx>, - fi: &'tcx rustc_hir::ForeignItem<'tcx>, - ) { + fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, fi: &'tcx hir::ForeignItem<'tcx>) { match fi.kind { hir::ForeignItemKind::Fn(fn_sig, _idents, _generics) => check_fn_like(cx, fn_sig.decl), hir::ForeignItemKind::Static(..) => {} @@ -228,7 +225,7 @@ impl LifetimeSyntaxCategories> { pub fn iter_unnamed(&self) -> impl Iterator { let Self { hidden, elided, named: _ } = self; - [hidden.iter(), elided.iter()].into_iter().flatten() + std::iter::chain(hidden, elided) } } @@ -308,13 +305,13 @@ fn emit_mismatch_diagnostic<'tcx>( use LifetimeSource::*; use hir::LifetimeSyntax::*; - let syntax_source = info.syntax_source(); + let lifetime = info.lifetime; - if let (ExplicitBound, _) = syntax_source { + if lifetime.syntax == ExplicitBound { bound_lifetime = Some(info); } - match syntax_source { + match (lifetime.syntax, lifetime.source) { // E.g. `&T`. (Implicit, Reference) => { suggest_change_to_explicit_anonymous.push(info); @@ -334,8 +331,8 @@ fn emit_mismatch_diagnostic<'tcx>( suggest_change_to_explicit_bound.push(info); } - // E.g. `ContainsLifetime<'_>`. - (ExplicitAnonymous, Path { .. }) => { + // E.g. `ContainsLifetime<'_>`, `+ '_`, `+ use<'_>`. + (ExplicitAnonymous, Path { .. } | OutlivesBound | PreciseCapturing) => { suggest_change_to_explicit_bound.push(info); } @@ -346,8 +343,8 @@ fn emit_mismatch_diagnostic<'tcx>( suggest_change_to_explicit_anonymous.push(info); } - // E.g. `ContainsLifetime<'a>`. - (ExplicitBound, Path { .. }) => { + // E.g. `ContainsLifetime<'a>`, `+ 'a`, `+ use<'a>`. + (ExplicitBound, Path { .. } | OutlivesBound | PreciseCapturing) => { suggest_change_to_mixed_explicit_anonymous.push(info); suggest_change_to_explicit_anonymous.push(info); } @@ -356,29 +353,18 @@ fn emit_mismatch_diagnostic<'tcx>( panic!("This syntax / source combination is not possible"); } - // E.g. `+ '_`, `+ use<'_>`. - (ExplicitAnonymous, OutlivesBound | PreciseCapturing) => { - suggest_change_to_explicit_bound.push(info); - } - - // E.g. `+ 'a`, `+ use<'a>`. - (ExplicitBound, OutlivesBound | PreciseCapturing) => { - suggest_change_to_mixed_explicit_anonymous.push(info); - suggest_change_to_explicit_anonymous.push(info); - } - (_, Other) => { panic!("This syntax / source combination has already been skipped"); } } - if matches!(syntax_source, (_, Path { .. } | OutlivesBound | PreciseCapturing)) { + if matches!(lifetime.source, Path { .. } | OutlivesBound | PreciseCapturing) { allow_suggesting_implicit = false; } - match syntax_source { - (_, Reference) => saw_a_reference = true, - (_, Path { .. }) => saw_a_path = true, + match lifetime.source { + Reference => saw_a_reference = true, + Path { .. } => saw_a_path = true, _ => {} } } @@ -398,10 +384,10 @@ fn emit_mismatch_diagnostic<'tcx>( |infos: &[&Info<'_>]| infos.iter().map(|i| i.removing_span()).collect::>(); let explicit_bound_suggestion = bound_lifetime.map(|info| { - build_mismatch_suggestion(info.lifetime_name(), &suggest_change_to_explicit_bound) + build_mismatch_suggestion(info.lifetime.ident.as_str(), &suggest_change_to_explicit_bound) }); - let is_bound_static = bound_lifetime.is_some_and(|info| info.is_static()); + let is_bound_static = bound_lifetime.is_some_and(|info| info.lifetime.is_static()); tracing::debug!(?bound_lifetime, ?explicit_bound_suggestion, ?is_bound_static); @@ -514,18 +500,6 @@ struct Info<'tcx> { } impl<'tcx> Info<'tcx> { - fn syntax_source(&self) -> (hir::LifetimeSyntax, LifetimeSource) { - (self.lifetime.syntax, self.lifetime.source) - } - - fn lifetime_name(&self) -> &str { - self.lifetime.ident.as_str() - } - - fn is_static(&self) -> bool { - self.lifetime.is_static() - } - /// When reporting a lifetime that is implicit, we expand the span /// to include the type. Otherwise we end up pointing at nothing, /// which is a bit confusing. @@ -547,7 +521,7 @@ impl<'tcx> Info<'tcx> { /// ``` // FIXME: Ideally, we'd also remove the lifetime declaration. fn removing_span(&self) -> Span { - let mut span = self.suggestion("'dummy").0; + let mut span = self.lifetime.ident.span; if let hir::TyKind::Ref(_, mut_ty) = self.ty.kind { span = span.until(mut_ty.ty.span); }