Auto merge of rust-lang#121587 - ShoyuVanilla:fix-issue-121267, r=TaKO8Ki

Fix bad span for explicit lifetime suggestions

Fixes rust-lang#121267

Current explicit lifetime suggestions are not showing correct spans for some lifetimes - e.g. elided lifetime generic parameters;

This should be done correctly regarding elided lifetime kind like the following code

https://github.com/rust-lang/rust/blob/43fdd4916d19f4004e23d422b5547637ad67ab21/compiler/rustc_resolve/src/late/diagnostics.rs#L3015-L3044

Author: bors

compiler/rustc_ast_lowering/src/lib.rs

@@ -55,7 +55,9 @@ use rustc_errors::{DiagArgFromDisplay, DiagCtxt, StashKey};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
 use rustc_hir::def_id::{LocalDefId, LocalDefIdMap, CRATE_DEF_ID, LOCAL_CRATE};
-use rustc_hir::{ConstArg, GenericArg, ItemLocalMap, ParamName, TraitCandidate};
+use rustc_hir::{
+    ConstArg, GenericArg, ItemLocalMap, MissingLifetimeKind, ParamName, TraitCandidate,
+};
 use rustc_index::{Idx, IndexSlice, IndexVec};
 use rustc_macros::extension;
 use rustc_middle::span_bug;
@@ -797,7 +799,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             LifetimeRes::Param { .. } => {
                 (hir::ParamName::Plain(ident), hir::LifetimeParamKind::Explicit)
             }
-            LifetimeRes::Fresh { param, .. } => {
+            LifetimeRes::Fresh { param, kind, .. } => {
                 // Late resolution delegates to us the creation of the `LocalDefId`.
                 let _def_id = self.create_def(
                     self.current_hir_id_owner.def_id,
@@ -808,7 +810,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 );
                 debug!(?_def_id);
 
-                (hir::ParamName::Fresh, hir::LifetimeParamKind::Elided)
+                (hir::ParamName::Fresh, hir::LifetimeParamKind::Elided(kind))
             }
             LifetimeRes::Static | LifetimeRes::Error => return None,
             res => panic!(
@@ -1605,13 +1607,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         for lifetime in captured_lifetimes_to_duplicate {
             let res = self.resolver.get_lifetime_res(lifetime.id).unwrap_or(LifetimeRes::Error);
-            let old_def_id = match res {
-                LifetimeRes::Param { param: old_def_id, binder: _ } => old_def_id,
+            let (old_def_id, missing_kind) = match res {
+                LifetimeRes::Param { param: old_def_id, binder: _ } => (old_def_id, None),
 
-                LifetimeRes::Fresh { param, binder: _ } => {
+                LifetimeRes::Fresh { param, kind, .. } => {
                     debug_assert_eq!(lifetime.ident.name, kw::UnderscoreLifetime);
                     if let Some(old_def_id) = self.orig_opt_local_def_id(param) {
-                        old_def_id
+                        (old_def_id, Some(kind))
                     } else {
                         self.dcx()
                             .span_delayed_bug(lifetime.ident.span, "no def-id for fresh lifetime");
@@ -1651,6 +1653,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     duplicated_lifetime_node_id,
                     duplicated_lifetime_def_id,
                     self.lower_ident(lifetime.ident),
+                    missing_kind,
                 ));
 
                 // Now make an arg that we can use for the generic params of the opaque tykind.
@@ -1668,27 +1671,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             let bounds = this
                 .with_remapping(captured_to_synthesized_mapping, |this| lower_item_bounds(this));
 
-            let generic_params = this.arena.alloc_from_iter(
-                synthesized_lifetime_definitions.iter().map(|&(new_node_id, new_def_id, ident)| {
-                    let hir_id = this.lower_node_id(new_node_id);
-                    let (name, kind) = if ident.name == kw::UnderscoreLifetime {
-                        (hir::ParamName::Fresh, hir::LifetimeParamKind::Elided)
-                    } else {
-                        (hir::ParamName::Plain(ident), hir::LifetimeParamKind::Explicit)
-                    };
+            let generic_params =
+                this.arena.alloc_from_iter(synthesized_lifetime_definitions.iter().map(
+                    |&(new_node_id, new_def_id, ident, missing_kind)| {
+                        let hir_id = this.lower_node_id(new_node_id);
+                        let (name, kind) = if ident.name == kw::UnderscoreLifetime {
+                            (
+                                hir::ParamName::Fresh,
+                                hir::LifetimeParamKind::Elided(
+                                    missing_kind.unwrap_or(MissingLifetimeKind::Underscore),
+                                ),
+                            )
+                        } else {
+                            (hir::ParamName::Plain(ident), hir::LifetimeParamKind::Explicit)
+                        };
 
-                    hir::GenericParam {
-                        hir_id,
-                        def_id: new_def_id,
-                        name,
-                        span: ident.span,
-                        pure_wrt_drop: false,
-                        kind: hir::GenericParamKind::Lifetime { kind },
-                        colon_span: None,
-                        source: hir::GenericParamSource::Generics,
-                    }
-                }),
-            );
+                        hir::GenericParam {
+                            hir_id,
+                            def_id: new_def_id,
+                            name,
+                            span: ident.span,
+                            pure_wrt_drop: false,
+                            kind: hir::GenericParamKind::Lifetime { kind },
+                            colon_span: None,
+                            source: hir::GenericParamSource::Generics,
+                        }
+                    },
+                ));
             debug!("lower_async_fn_ret_ty: generic_params={:#?}", generic_params);
 
             let lifetime_mapping = self.arena.alloc_slice(&synthesized_lifetime_args);

compiler/rustc_hir/src/def.rs

@@ -810,6 +810,8 @@ pub enum LifetimeRes {
         param: NodeId,
         /// Id of the introducing place. See `Param`.
         binder: NodeId,
+        /// Kind of elided lifetime
+        kind: hir::MissingLifetimeKind,
     },
     /// This variant is used for anonymous lifetimes that we did not resolve during
     /// late resolution. Those lifetimes will be inferred by typechecking.

compiler/rustc_hir/src/hir.rs

@@ -456,6 +456,18 @@ impl GenericBound<'_> {
 
 pub type GenericBounds<'hir> = &'hir [GenericBound<'hir>];
 
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, HashStable_Generic, Debug)]
+pub enum MissingLifetimeKind {
+    /// An explicit `'_`.
+    Underscore,
+    /// An elided lifetime `&' ty`.
+    Ampersand,
+    /// An elided lifetime in brackets with written brackets.
+    Comma,
+    /// An elided lifetime with elided brackets.
+    Brackets,
+}
+
 #[derive(Copy, Clone, Debug, HashStable_Generic)]
 pub enum LifetimeParamKind {
     // Indicates that the lifetime definition was explicitly declared (e.g., in
@@ -464,7 +476,7 @@ pub enum LifetimeParamKind {
 
     // Indication that the lifetime was elided (e.g., in both cases in
     // `fn foo(x: &u8) -> &'_ u8 { x }`).
-    Elided,
+    Elided(MissingLifetimeKind),
 
     // Indication that the lifetime name was somehow in error.
     Error,
@@ -512,7 +524,7 @@ impl<'hir> GenericParam<'hir> {
     ///
     /// See `lifetime_to_generic_param` in `rustc_ast_lowering` for more information.
     pub fn is_elided_lifetime(&self) -> bool {
-        matches!(self.kind, GenericParamKind::Lifetime { kind: LifetimeParamKind::Elided })
+        matches!(self.kind, GenericParamKind::Lifetime { kind: LifetimeParamKind::Elided(_) })
     }
 }
 

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -1305,7 +1305,7 @@ fn compare_number_of_generics<'tcx>(
                     .iter()
                     .filter(|p| match p.kind {
                         hir::GenericParamKind::Lifetime {
-                            kind: hir::LifetimeParamKind::Elided,
+                            kind: hir::LifetimeParamKind::Elided(_),
                         } => {
                             // A fn can have an arbitrary number of extra elided lifetimes for the
                             // same signature.

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -13,8 +13,8 @@ use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan, Subdiagnosti
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{walk_ty, Visitor};
 use rustc_hir::{
-    self as hir, GenericBound, GenericParamKind, Item, ItemKind, Lifetime, LifetimeName, Node,
-    TyKind,
+    self as hir, GenericBound, GenericParam, GenericParamKind, Item, ItemKind, Lifetime,
+    LifetimeName, LifetimeParamKind, MissingLifetimeKind, Node, TyKind,
 };
 use rustc_middle::ty::{
     self, AssocItemContainer, StaticLifetimeVisitor, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor,
@@ -355,20 +355,8 @@ pub fn suggest_new_region_bound(
                     // introducing a new lifetime `'a` or making use of one from existing named lifetimes if any
                     if let Some(id) = scope_def_id
                         && let Some(generics) = tcx.hir().get_generics(id)
-                        && let mut spans_suggs = generics
-                            .params
-                            .iter()
-                            .filter(|p| p.is_elided_lifetime())
-                            .map(|p| {
-                                if p.span.hi() - p.span.lo() == rustc_span::BytePos(1) {
-                                    // Ampersand (elided without '_)
-                                    (p.span.shrink_to_hi(), format!("{name} "))
-                                } else {
-                                    // Underscore (elided with '_)
-                                    (p.span, name.to_string())
-                                }
-                            })
-                            .collect::<Vec<_>>()
+                        && let mut spans_suggs =
+                            make_elided_region_spans_suggs(name, generics.params.iter())
                         && spans_suggs.len() > 1
                     {
                         let use_lt = if existing_lt_name == None {
@@ -430,6 +418,57 @@ pub fn suggest_new_region_bound(
     }
 }
 
+fn make_elided_region_spans_suggs<'a>(
+    name: &str,
+    generic_params: impl Iterator<Item = &'a GenericParam<'a>>,
+) -> Vec<(Span, String)> {
+    let mut spans_suggs = Vec::new();
+    let mut bracket_span = None;
+    let mut consecutive_brackets = 0;
+
+    let mut process_consecutive_brackets =
+        |span: Option<Span>, spans_suggs: &mut Vec<(Span, String)>| {
+            if span
+                .is_some_and(|span| bracket_span.map_or(true, |bracket_span| span == bracket_span))
+            {
+                consecutive_brackets += 1;
+            } else if let Some(bracket_span) = bracket_span.take() {
+                let sugg = std::iter::once("<")
+                    .chain(std::iter::repeat(name).take(consecutive_brackets).intersperse(", "))
+                    .chain([">"])
+                    .collect();
+                spans_suggs.push((bracket_span.shrink_to_hi(), sugg));
+                consecutive_brackets = 0;
+            }
+            bracket_span = span;
+        };
+
+    for p in generic_params {
+        if let GenericParamKind::Lifetime { kind: LifetimeParamKind::Elided(kind) } = p.kind {
+            match kind {
+                MissingLifetimeKind::Underscore => {
+                    process_consecutive_brackets(None, &mut spans_suggs);
+                    spans_suggs.push((p.span, name.to_string()))
+                }
+                MissingLifetimeKind::Ampersand => {
+                    process_consecutive_brackets(None, &mut spans_suggs);
+                    spans_suggs.push((p.span.shrink_to_hi(), format!("{name} ")));
+                }
+                MissingLifetimeKind::Comma => {
+                    process_consecutive_brackets(None, &mut spans_suggs);
+                    spans_suggs.push((p.span.shrink_to_hi(), format!("{name}, ")));
+                }
+                MissingLifetimeKind::Brackets => {
+                    process_consecutive_brackets(Some(p.span), &mut spans_suggs);
+                }
+            }
+        }
+    }
+    process_consecutive_brackets(None, &mut spans_suggs);
+
+    spans_suggs
+}
+
 impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
     pub fn get_impl_ident_and_self_ty_from_trait(
         tcx: TyCtxt<'tcx>,

compiler/rustc_infer/src/lib.rs

@@ -24,6 +24,7 @@
 #![feature(extend_one)]
 #![feature(let_chains)]
 #![feature(if_let_guard)]
+#![feature(iter_intersperse)]
 #![feature(iterator_try_collect)]
 #![feature(try_blocks)]
 #![feature(yeet_expr)]

compiler/rustc_resolve/src/late.rs

@@ -22,7 +22,7 @@ use rustc_errors::{
 use rustc_hir::def::Namespace::{self, *};
 use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS};
 use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
-use rustc_hir::{PrimTy, TraitCandidate};
+use rustc_hir::{MissingLifetimeKind, PrimTy, TraitCandidate};
 use rustc_middle::middle::resolve_bound_vars::Set1;
 use rustc_middle::{bug, span_bug};
 use rustc_session::config::{CrateType, ResolveDocLinks};
@@ -44,9 +44,7 @@ type Res = def::Res<NodeId>;
 
 type IdentMap<T> = FxHashMap<Ident, T>;
 
-use diagnostics::{
-    ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime, MissingLifetimeKind,
-};
+use diagnostics::{ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime};
 
 #[derive(Copy, Clone, Debug)]
 struct BindingInfo {
@@ -1637,22 +1635,16 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
     fn resolve_anonymous_lifetime(&mut self, lifetime: &Lifetime, elided: bool) {
         debug_assert_eq!(lifetime.ident.name, kw::UnderscoreLifetime);
 
-        let missing_lifetime = MissingLifetime {
-            id: lifetime.id,
-            span: lifetime.ident.span,
-            kind: if elided {
-                MissingLifetimeKind::Ampersand
-            } else {
-                MissingLifetimeKind::Underscore
-            },
-            count: 1,
-        };
+        let kind =
+            if elided { MissingLifetimeKind::Ampersand } else { MissingLifetimeKind::Underscore };
+        let missing_lifetime =
+            MissingLifetime { id: lifetime.id, span: lifetime.ident.span, kind, count: 1 };
         let elision_candidate = LifetimeElisionCandidate::Missing(missing_lifetime);
         for (i, rib) in self.lifetime_ribs.iter().enumerate().rev() {
             debug!(?rib.kind);
             match rib.kind {
                 LifetimeRibKind::AnonymousCreateParameter { binder, .. } => {
-                    let res = self.create_fresh_lifetime(lifetime.ident, binder);
+                    let res = self.create_fresh_lifetime(lifetime.ident, binder, kind);
                     self.record_lifetime_res(lifetime.id, res, elision_candidate);
                     return;
                 }
@@ -1744,13 +1736,18 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
     }
 
     #[instrument(level = "debug", skip(self))]
-    fn create_fresh_lifetime(&mut self, ident: Ident, binder: NodeId) -> LifetimeRes {
+    fn create_fresh_lifetime(
+        &mut self,
+        ident: Ident,
+        binder: NodeId,
+        kind: MissingLifetimeKind,
+    ) -> LifetimeRes {
         debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
         debug!(?ident.span);
 
         // Leave the responsibility to create the `LocalDefId` to lowering.
         let param = self.r.next_node_id();
-        let res = LifetimeRes::Fresh { param, binder };
+        let res = LifetimeRes::Fresh { param, binder, kind };
         self.record_lifetime_param(param, res);
 
         // Record the created lifetime parameter so lowering can pick it up and add it to HIR.
@@ -1844,14 +1841,15 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             };
             let ident = Ident::new(kw::UnderscoreLifetime, elided_lifetime_span);
 
+            let kind = if segment.has_generic_args {
+                MissingLifetimeKind::Comma
+            } else {
+                MissingLifetimeKind::Brackets
+            };
             let missing_lifetime = MissingLifetime {
                 id: node_ids.start,
                 span: elided_lifetime_span,
-                kind: if segment.has_generic_args {
-                    MissingLifetimeKind::Comma
-                } else {
-                    MissingLifetimeKind::Brackets
-                },
+                kind,
                 count: expected_lifetimes,
             };
             let mut should_lint = true;
@@ -1897,7 +1895,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
                         // Group all suggestions into the first record.
                         let mut candidate = LifetimeElisionCandidate::Missing(missing_lifetime);
                         for id in node_ids {
-                            let res = self.create_fresh_lifetime(ident, binder);
+                            let res = self.create_fresh_lifetime(ident, binder, kind);
                             self.record_lifetime_res(
                                 id,
                                 res,

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -24,7 +24,7 @@ use rustc_errors::{
 use rustc_hir as hir;
 use rustc_hir::def::{self, CtorKind, CtorOf, DefKind};
 use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
-use rustc_hir::PrimTy;
+use rustc_hir::{MissingLifetimeKind, PrimTy};
 use rustc_session::lint;
 use rustc_session::Session;
 use rustc_span::edit_distance::find_best_match_for_name;
@@ -109,18 +109,6 @@ pub(super) struct MissingLifetime {
     pub count: usize,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
-pub(super) enum MissingLifetimeKind {
-    /// An explicit `'_`.
-    Underscore,
-    /// An elided lifetime `&' ty`.
-    Ampersand,
-    /// An elided lifetime in brackets with written brackets.
-    Comma,
-    /// An elided lifetime with elided brackets.
-    Brackets,
-}
-
 /// Description of the lifetimes appearing in a function parameter.
 /// This is used to provide a literal explanation to the elision failure.
 #[derive(Clone, Debug)]