Skip to content

Remove kw::Empty uses from hir::Lifetime::ident #138965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 28, 2025
Merged

Remove kw::Empty uses from hir::Lifetime::ident #138965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4924e0a
Add a HIR pretty-printing test focused on lifetimes.
nnethercote Mar 26, 2025
c6d8d65
Remove `LifetimeSuggestionPosition` and `Lifetime::suggestion_position`.
nnethercote Mar 24, 2025
cfd00f9
Remove `ImplicitObjectLifetimeDefault` case from `suggestion`.
nnethercote Mar 25, 2025
8d2c63f
Don't use `kw::Empty` in `hir::Lifetime::ident`.
nnethercote Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
47 changes: 34 additions & 13 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))]
Expand All @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when does this happen to be an underscore lifetime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the 'static is elided, e.g. for both C and D here:

struct Name<'a>(&'a str);
const C: Name = Name("name"); // IsAnonInPath::Yes
static D: &str = "";          // IsAnonInPath::No

I didn't even realize 'static could be elided like this!

I will add a comment about this.

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(
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_ast_lowering/src/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
}),
);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()));
Expand Down
135 changes: 87 additions & 48 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
}

Expand All @@ -135,59 +176,57 @@ 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)
}
}

pub enum LifetimeSuggestionPosition {
/// The user wrote `'a` or `'_`.
Normal,
/// The user wrote `&type` or `&mut type`.
Ampersand,
/// The user wrote `Path` and omitted the `<'_>`.
ElidedPath,
/// The user wrote `Path<T>`, and omitted the `'_,`.
ElidedPathArgument,
/// The user wrote `dyn Trait` and omitted the `+ '_`.
ObjectDefault,
}

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
}

pub fn suggestion_position(&self) -> (LifetimeSuggestionPosition, Span) {
if self.ident.name == kw::Empty {
if self.ident.span.is_empty() {
(LifetimeSuggestionPosition::ElidedPathArgument, self.ident.span)
} else {
(LifetimeSuggestionPosition::ElidedPath, self.ident.span.shrink_to_hi())
}
} else if self.res == LifetimeName::ImplicitObjectLifetimeDefault {
(LifetimeSuggestionPosition::ObjectDefault, self.ident.span)
} else if self.ident.span.is_empty() {
(LifetimeSuggestionPosition::Ampersand, self.ident.span)
} else {
(LifetimeSuggestionPosition::Normal, self.ident.span)
}
self.ident.name == kw::UnderscoreLifetime
}

pub fn suggestion(&self, new_lifetime: &str) -> (Span, String) {
debug_assert!(new_lifetime.starts_with('\''));
let (pos, span) = self.suggestion_position();
let code = match pos {
LifetimeSuggestionPosition::Normal => format!("{new_lifetime}"),
LifetimeSuggestionPosition::Ampersand => format!("{new_lifetime} "),
LifetimeSuggestionPosition::ElidedPath => format!("<{new_lifetime}>"),
LifetimeSuggestionPosition::ElidedPathArgument => format!("{new_lifetime}, "),
LifetimeSuggestionPosition::ObjectDefault => format!("+ {new_lifetime}"),
};
(span, code)

match (self.is_anon_in_path, self.ident.span.is_empty()) {
// The user wrote `Path<T>`, and omitted the `'_,`.
(IsAnonInPath::Yes, true) => (self.ident.span, format!("{new_lifetime}, ")),

// The user wrote `Path` and omitted the `<'_>`.
(IsAnonInPath::Yes, false) => {
(self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>"))
}

// The user wrote `&type` or `&mut type`.
(IsAnonInPath::No, true) => (self.ident.span, format!("{new_lifetime} ")),

// The user wrote `'a` or `'_`.
(IsAnonInPath::No, false) => (self.ident.span, format!("{new_lifetime}")),
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_trait_selection/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is this part of the condition ever false. I would expect all AnonInPath to have empty spans

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsAnonInPath::Yes lifetimes are inserted in maybe_insert_elided_lifetimes_in_path. In that function elided_lifetime_span can definitely be non-empty.

However, none of those non-empty spans reach here, at least when running the UI tests. (I checked by adding an assertion.) I don't know if that's guaranteed, or if we just have insufficient test coverage. So I'd prefer to keep the test here for now; it is retained from the original code.

{
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()
Comment on lines +575 to +576
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is where changing ident to a source enum would be helpful

{
format!("{} ", self.suggestion_param_name)
} else {
self.suggestion_param_name.clone()
Expand All @@ -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((
Expand All @@ -605,7 +609,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
{
self.suggestions.push((
lifetime.ident.span,
make_suggestion(lifetime.ident),
make_suggestion(lifetime),
));
}
}
Expand All @@ -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)));
}
_ => {}
}
Expand Down
Loading
Loading