Don't use kw::Empty in hir::Lifetime::ident.

`hir::Lifetime::ident` currently sometimes uses `kw::Empty` for elided
lifetimes and sometimes uses `kw::UnderscoreLifetime`, and the
distinction is used when creating some error suggestions, e.g. in
`Lifetime::suggestion` and `ImplicitLifetimeFinder::visit_ty`. I found
this *really* confusing, and it took me a while to understand what was
going on.

This commit replaces all uses of `kw::Empty` in `hir::Lifetime::ident`
with `kw::UnderscoreLifetime`. It adds a new field
`hir::Lifetime::is_path_anon` that mostly replaces the old
empty/underscore distinction and makes things much clearer.

Some other notable changes:

- Adds some assertions in `new_named_lifetime` about what ident values
  are permissible for the different `LifetimeRes` values.

- Adds a `Lifetime::new` constructor that does some checking to make
  sure the `is_elided` and `is_anonymous` states are valid.

- `add_static_impl_trait_suggestion` now looks at `Lifetime::res`
  instead of the ident when creating the suggestion. This is the one
  case where `is_path_anon` doesn't replace the old empty/underscore
  distinction.

Author: nnethercote

compiler/rustc_ast_lowering/src/item.rs

@@ -5,7 +5,7 @@ use rustc_ast::*;
 use rustc_errors::ErrorGuaranteed;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
-use rustc_hir::{self as hir, HirId, PredicateOrigin};
+use rustc_hir::{self as hir, HirId, IsAnonInPath, PredicateOrigin};
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::ty::{ResolverAstLowering, TyCtxt};
 use rustc_span::edit_distance::find_best_match_for_name;
@@ -1787,7 +1787,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             }
             GenericParamKind::Lifetime => {
                 let lt_id = self.next_node_id();
-                let lifetime = self.new_named_lifetime(id, lt_id, ident);
+                let lifetime = self.new_named_lifetime(id, lt_id, ident, IsAnonInPath::No);
                 hir::WherePredicateKind::RegionPredicate(hir::WhereRegionPredicate {
                     lifetime,
                     bounds,

compiler/rustc_ast_lowering/src/lib.rs

@@ -55,7 +55,8 @@ use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle, StashKey};
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
 use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
 use rustc_hir::{
-    self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
+    self as hir, ConstArg, GenericArg, HirId, IsAnonInPath, ItemLocalMap, LangItem, ParamName,
+    TraitCandidate,
 };
 use rustc_index::{Idx, IndexSlice, IndexVec};
 use rustc_macros::extension;
@@ -1777,7 +1778,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 
     fn lower_lifetime(&mut self, l: &Lifetime) -> &'hir hir::Lifetime {
-        self.new_named_lifetime(l.id, l.id, l.ident)
+        self.new_named_lifetime(l.id, l.id, l.ident, IsAnonInPath::No)
+    }
+
+    fn lower_lifetime_anon_in_path(&mut self, id: NodeId, span: Span) -> &'hir hir::Lifetime {
+        self.new_named_lifetime(id, id, Ident::new(kw::UnderscoreLifetime, span), IsAnonInPath::Yes)
     }
 
     #[instrument(level = "debug", skip(self))]
@@ -1786,28 +1791,50 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         id: NodeId,
         new_id: NodeId,
         ident: Ident,
+        is_anon_in_path: IsAnonInPath,
     ) -> &'hir hir::Lifetime {
+        debug_assert_ne!(ident.name, kw::Empty);
         let res = self.resolver.get_lifetime_res(id).unwrap_or(LifetimeRes::Error);
         let res = match res {
             LifetimeRes::Param { param, .. } => hir::LifetimeName::Param(param),
             LifetimeRes::Fresh { param, .. } => {
+                debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
                 let param = self.local_def_id(param);
                 hir::LifetimeName::Param(param)
             }
-            LifetimeRes::Infer => hir::LifetimeName::Infer,
-            LifetimeRes::Static { .. } => hir::LifetimeName::Static,
+            LifetimeRes::Infer => {
+                debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
+                hir::LifetimeName::Infer
+            }
+            LifetimeRes::Static { .. } => {
+                // `kw::UnderscoreLifetime` occurs when `'static` is elided,
+                // e.g. in consts and statics such as `A` and `B` here:
+                // ```
+                // struct Name<'a>(&'a str);
+                // const A: Name = Name("name"); // njn: IsAnonInPath::Yes
+                // static B: &str = "";          // njn: IsAnonInPath::No
+                // ```
+                debug_assert!(matches!(ident.name, kw::StaticLifetime | kw::UnderscoreLifetime));
+                hir::LifetimeName::Static
+            }
             LifetimeRes::Error => hir::LifetimeName::Error,
             LifetimeRes::ElidedAnchor { .. } => {
                 panic!("Unexpected `ElidedAnchar` {:?} at {:?}", ident, ident.span);
             }
         };
 
+        #[cfg(debug_assertions)]
+        if is_anon_in_path == IsAnonInPath::Yes {
+            debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
+        }
+
         debug!(?res);
-        self.arena.alloc(hir::Lifetime {
-            hir_id: self.lower_node_id(new_id),
-            ident: self.lower_ident(ident),
+        self.arena.alloc(hir::Lifetime::new(
+            self.lower_node_id(new_id),
+            self.lower_ident(ident),
             res,
-        })
+            is_anon_in_path,
+        ))
     }
 
     fn lower_generic_params_mut(
@@ -2391,11 +2418,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     /// when the bound is written, even if it is written with `'_` like in
     /// `Box<dyn Debug + '_>`. In those cases, `lower_lifetime` is invoked.
     fn elided_dyn_bound(&mut self, span: Span) -> &'hir hir::Lifetime {
-        let r = hir::Lifetime {
-            hir_id: self.next_id(),
-            ident: Ident::new(kw::Empty, self.lower_span(span)),
-            res: hir::LifetimeName::ImplicitObjectLifetimeDefault,
-        };
+        let r = hir::Lifetime::new(
+            self.next_id(),
+            Ident::new(kw::UnderscoreLifetime, self.lower_span(span)),
+            hir::LifetimeName::ImplicitObjectLifetimeDefault,
+            IsAnonInPath::No,
+        );
         debug!("elided_dyn_bound: r={:?}", r);
         self.arena.alloc(r)
     }

compiler/rustc_ast_lowering/src/path.rs

@@ -7,7 +7,7 @@ use rustc_hir::def::{DefKind, PartialRes, Res};
 use rustc_hir::def_id::DefId;
 use rustc_middle::span_bug;
 use rustc_session::parse::add_feature_diagnostics;
-use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, kw, sym};
+use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, sym};
 use smallvec::{SmallVec, smallvec};
 use tracing::{debug, instrument};
 
@@ -450,10 +450,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             0,
             (start.as_u32()..end.as_u32()).map(|i| {
                 let id = NodeId::from_u32(i);
-                let l = self.lower_lifetime(&Lifetime {
-                    id,
-                    ident: Ident::new(kw::Empty, elided_lifetime_span),
-                });
+                let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span);
                 GenericArg::Lifetime(l)
             }),
         );

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -513,14 +513,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             ty::VarianceDiagInfo::Invariant { ty, param_index } => {
                 let (desc, note) = match ty.kind() {
                     ty::RawPtr(ty, mutbl) => {
-                        assert_eq!(*mutbl, rustc_hir::Mutability::Mut);
+                        assert_eq!(*mutbl, hir::Mutability::Mut);
                         (
                             format!("a mutable pointer to `{}`", ty),
                             "mutable pointers are invariant over their type parameter".to_string(),
                         )
                     }
                     ty::Ref(_, inner_ty, mutbl) => {
-                        assert_eq!(*mutbl, rustc_hir::Mutability::Mut);
+                        assert_eq!(*mutbl, hir::Mutability::Mut);
                         (
                             format!("a mutable reference to `{inner_ty}`"),
                             "mutable references are invariant over their type parameter"
@@ -887,7 +887,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     // Skip `async` desugaring `impl Future`.
                 }
                 if let TyKind::TraitObject(_, lt) = alias_ty.kind {
-                    if lt.ident.name == kw::Empty {
+                    if lt.res == hir::LifetimeName::ImplicitObjectLifetimeDefault {
                         spans_suggs.push((lt.ident.span.shrink_to_hi(), " + 'a".to_string()));
                     } else {
                         spans_suggs.push((lt.ident.span, "'a".to_string()));

compiler/rustc_hir/src/hir.rs

@@ -35,20 +35,27 @@ use crate::def_id::{DefId, LocalDefIdMap};
 pub(crate) use crate::hir_id::{HirId, ItemLocalId, ItemLocalMap, OwnerId};
 use crate::intravisit::{FnKind, VisitorExt};
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable_Generic)]
+pub enum IsAnonInPath {
+    No,
+    Yes,
+}
+
 #[derive(Debug, Copy, Clone, HashStable_Generic)]
 pub struct Lifetime {
     #[stable_hasher(ignore)]
     pub hir_id: HirId,
 
-    /// Either "`'a`", referring to a named lifetime definition,
-    /// `'_` referring to an anonymous lifetime (either explicitly `'_` or `&type`),
-    /// or "``" (i.e., `kw::Empty`) when appearing in path.
-    ///
-    /// See `Lifetime::suggestion_position` for practical use.
+    /// Either a named lifetime definition (e.g. `'a`) or an anonymous lifetime
+    /// (`'_`, either explicitly written, or inserted for things like `&type`).
     pub ident: Ident,
 
     /// Semantics of this lifetime.
     pub res: LifetimeName,
+
+    /// Is the lifetime anonymous and in a path? Used only for error
+    /// suggestions. See `Lifetime::suggestion` for example use.
+    pub is_anon_in_path: IsAnonInPath,
 }
 
 #[derive(Debug, Copy, Clone, HashStable_Generic)]
@@ -135,34 +142,56 @@ impl LifetimeName {
 
 impl fmt::Display for Lifetime {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if self.ident.name != kw::Empty { self.ident.name.fmt(f) } else { "'_".fmt(f) }
+        self.ident.name.fmt(f)
     }
 }
 
 impl Lifetime {
+    pub fn new(
+        hir_id: HirId,
+        ident: Ident,
+        res: LifetimeName,
+        is_anon_in_path: IsAnonInPath,
+    ) -> Lifetime {
+        let lifetime = Lifetime { hir_id, ident, res, is_anon_in_path };
+
+        // Sanity check: elided lifetimes form a strict subset of anonymous lifetimes.
+        #[cfg(debug_assertions)]
+        match (lifetime.is_elided(), lifetime.is_anonymous()) {
+            (false, false) => {} // e.g. `'a`
+            (false, true) => {}  // e.g. explicit `'_`
+            (true, true) => {}   // e.g. `&x`
+            (true, false) => panic!("bad Lifetime"),
+        }
+
+        lifetime
+    }
+
     pub fn is_elided(&self) -> bool {
         self.res.is_elided()
     }
 
     pub fn is_anonymous(&self) -> bool {
-        self.ident.name == kw::Empty || self.ident.name == kw::UnderscoreLifetime
+        self.ident.name == kw::UnderscoreLifetime
     }
 
     pub fn suggestion(&self, new_lifetime: &str) -> (Span, String) {
         debug_assert!(new_lifetime.starts_with('\''));
 
-        match (self.ident.name.is_empty(), self.ident.span.is_empty()) {
+        match (self.is_anon_in_path, self.ident.span.is_empty()) {
             // The user wrote `Path<T>`, and omitted the `'_,`.
-            (true, true) => (self.ident.span, format!("{new_lifetime}, ")),
+            (IsAnonInPath::Yes, true) => (self.ident.span, format!("{new_lifetime}, ")),
 
             // The user wrote `Path` and omitted the `<'_>`.
-            (true, false) => (self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>")),
+            (IsAnonInPath::Yes, false) => {
+                (self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>"))
+            }
 
             // The user wrote `&type` or `&mut type`.
-            (false, true) => (self.ident.span, format!("{new_lifetime} ")),
+            (IsAnonInPath::No, true) => (self.ident.span, format!("{new_lifetime} ")),
 
             // The user wrote `'a` or `'_`.
-            (false, false) => (self.ident.span, format!("{new_lifetime}")),
+            (IsAnonInPath::No, false) => (self.ident.span, format!("{new_lifetime}")),
         }
     }
 }

compiler/rustc_hir/src/hir/tests.rs

@@ -58,6 +58,7 @@ fn trait_object_roundtrips_impl(syntax: TraitObjectSyntax) {
                     hir_id: HirId::INVALID,
                     ident: Ident::new(sym::name, DUMMY_SP),
                     res: LifetimeName::Static,
+                    is_anon_in_path: IsAnonInPath::No,
                 }
             },
             syntax,

compiler/rustc_trait_selection/src/errors.rs

@@ -9,7 +9,7 @@ use rustc_errors::{
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit::{Visitor, VisitorExt, walk_ty};
-use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, Node};
+use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, IsAnonInPath, Node};
 use rustc_macros::{Diagnostic, Subdiagnostic};
 use rustc_middle::ty::print::{PrintTraitRefExt as _, TraitRefPrintOnlyTraitPath};
 use rustc_middle::ty::{self, Binder, ClosureKind, FnSig, Region, Ty, TyCtxt};
@@ -567,10 +567,14 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
 
             impl<'v> Visitor<'v> for ImplicitLifetimeFinder {
                 fn visit_ty(&mut self, ty: &'v hir::Ty<'v, AmbigArg>) {
-                    let make_suggestion = |ident: Ident| {
-                        if ident.name == kw::Empty && ident.span.is_empty() {
+                    let make_suggestion = |lifetime: &hir::Lifetime| {
+                        if lifetime.is_anon_in_path == IsAnonInPath::Yes
+                            && lifetime.ident.span.is_empty()
+                        {
                             format!("{}, ", self.suggestion_param_name)
-                        } else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() {
+                        } else if lifetime.ident.name == kw::UnderscoreLifetime
+                            && lifetime.ident.span.is_empty()
+                        {
                             format!("{} ", self.suggestion_param_name)
                         } else {
                             self.suggestion_param_name.clone()
@@ -584,7 +588,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
                                         matches!(
                                             arg,
                                             hir::GenericArg::Lifetime(lifetime)
-                                            if lifetime.ident.name == kw::Empty
+                                                if lifetime.is_anon_in_path == IsAnonInPath::Yes
                                         )
                                     }) {
                                         self.suggestions.push((
@@ -605,7 +609,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
                                             {
                                                 self.suggestions.push((
                                                     lifetime.ident.span,
-                                                    make_suggestion(lifetime.ident),
+                                                    make_suggestion(lifetime),
                                                 ));
                                             }
                                         }
@@ -614,8 +618,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
                             }
                         }
                         hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => {
-                            self.suggestions
-                                .push((lifetime.ident.span, make_suggestion(lifetime.ident)));
+                            self.suggestions.push((lifetime.ident.span, make_suggestion(lifetime)));
                         }
                         _ => {}
                     }

tests/ui/stats/input-stats.stderr

@@ -119,7 +119,7 @@ hir-stats HIR STATS
 hir-stats Name                Accumulated Size         Count     Item Size
 hir-stats ----------------------------------------------------------------
 hir-stats ForeignItemRef            24 ( 0.3%)             1            24
-hir-stats Lifetime                  24 ( 0.3%)             1            24
+hir-stats Lifetime                  28 ( 0.3%)             1            28
 hir-stats Mod                       32 ( 0.4%)             1            32
 hir-stats ExprField                 40 ( 0.4%)             1            40
 hir-stats TraitItemRef              56 ( 0.6%)             2            28
@@ -155,7 +155,7 @@ hir-stats Generics                 560 ( 6.2%)            10            56
 hir-stats Ty                       720 ( 8.0%)            15            48
 hir-stats - Ptr                       48 ( 0.5%)             1
 hir-stats - Ref                       48 ( 0.5%)             1
-hir-stats - Path                     624 ( 7.0%)            13
+hir-stats - Path                     624 ( 6.9%)            13
 hir-stats Expr                     768 ( 8.6%)            12            64
 hir-stats - InlineAsm                 64 ( 0.7%)             1
 hir-stats - Match                     64 ( 0.7%)             1
@@ -174,5 +174,5 @@ hir-stats - Use                      352 ( 3.9%)             4
 hir-stats Path                   1_240 (13.8%)            31            40
 hir-stats PathSegment            1_920 (21.4%)            40            48
 hir-stats ----------------------------------------------------------------
-hir-stats Total                  8_976                   180
+hir-stats Total                  8_980                   180
 hir-stats