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 a big comment to `Lifetime` talking about permissable field
  values.

- 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.

- A couple of minor pretty-printing improvements.

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;
@@ -1823,7 +1823,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;
@@ -1755,7 +1756,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))]
@@ -1764,28 +1769,43 @@ 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 { .. } => {
+                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(
@@ -2369,11 +2389,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,60 @@ 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,
+}
+
+/// A lifetime. The valid field combinations are non-obvious. The following
+/// example shows some of them. See also the comments on `LifetimeName`.
+/// ```
+/// #[repr(C)]
+/// struct S<'a>(&'a u32);       // res=Param, name='a, IsAnonInPath::No
+/// unsafe extern "C" {
+///     fn f1(s: S);             // res=Param, name='_, IsAnonInPath::Yes
+///     fn f2(s: S<'_>);         // res=Param, name='_, IsAnonInPath::No
+///     fn f3<'a>(s: S<'a>);     // res=Param, name='a, IsAnonInPath::No
+/// }
+///
+/// struct St<'a> { x: &'a u32 } // res=Param, name='a, IsAnonInPath::No
+/// fn f() {
+///     _ = St { x: &0 };        // res=Infer, name='_, IsAnonInPath::Yes
+///     _ = St::<'_> { x: &0 };  // res=Infer, name='_, IsAnonInPath::No
+/// }
+///
+/// struct Name<'a>(&'a str);    // res=Param,  name='a, IsAnonInPath::No
+/// const A: Name = Name("a");   // res=Static, name='_, IsAnonInPath::Yes
+/// const B: &str = "";          // res=Static, name='_, IsAnonInPath::No
+/// static C: &'_ str = "";      // res=Static, name='_, IsAnonInPath::No
+/// static D: &'static str = ""; // res=Static, name='static, IsAnonInPath::No
+///
+/// trait Tr {}
+/// fn tr(_: Box<dyn Tr>) {}     // res=ImplicitObjectLifetimeDefault, name='_, IsAnonInPath::No
+///
+/// // (commented out because these cases trigger errors)
+/// // struct S1<'a>(&'a str);   // res=Param, name='a, IsAnonInPath::No
+/// // struct S2(S1);            // res=Error, name='_, IsAnonInPath::Yes
+/// // struct S3(S1<'_>);        // res=Error, name='_, IsAnonInPath::No
+/// // struct S4(S1<'a>);        // res=Error, name='a, IsAnonInPath::No
+/// ```
 #[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`, `'static`) 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)]
@@ -111,11 +151,12 @@ pub enum LifetimeName {
     /// that was already reported.
     Error,
 
-    /// User wrote an anonymous lifetime, either `'_` or nothing.
-    /// The semantics of this lifetime should be inferred by typechecking code.
+    /// User wrote an anonymous lifetime, either `'_` or nothing (which gets
+    /// converted to `'_`). The semantics of this lifetime should be inferred
+    /// by typechecking code.
     Infer,
 
-    /// User wrote `'static`.
+    /// User wrote `'static` or nothing (which gets converted to `'_`).
     Static,
 }
 
@@ -135,34 +176,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/pretty/hir-lifetimes.pp

@@ -73,7 +73,7 @@
 struct S<'a>(&'a u32);
 
 extern "C" {
-    unsafe fn g1(s: S<>);
+    unsafe fn g1(s: S<'_>);
     unsafe fn g2(s: S<'_>);
     unsafe fn g3<'a>(s: S<'a>);
 }
@@ -86,7 +86,7 @@
 
 struct Name<'a>(&'a str);
 
-const A: Name<> = Name("a");
+const A: Name<'_> = Name("a");
 const B: &'_ str = "";
 static C: &'_ str = "";
 static D: &'static str = "";