diff options
| author | Nicholas Nethercote <n.nethercote@gmail.com> | 2025-03-25 08:21:28 +1100 |
|---|---|---|
| committer | Nicholas Nethercote <n.nethercote@gmail.com> | 2025-03-28 10:15:23 +1100 |
| commit | 8d2c63f514a74752a3186b0bf3e8f3aef57dc12e (patch) | |
| tree | 887b83d34fc08db03dd8b3c09fc80183a15734b5 | |
| parent | cfd00f9c16d9b3ab98d6e06125c6baa83dfa4a03 (diff) | |
| download | rust-8d2c63f514a74752a3186b0bf3e8f3aef57dc12e.tar.gz rust-8d2c63f514a74752a3186b0bf3e8f3aef57dc12e.zip | |
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.
| -rw-r--r-- | compiler/rustc_ast_lowering/src/item.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_ast_lowering/src/lib.rs | 47 | ||||
| -rw-r--r-- | compiler/rustc_ast_lowering/src/path.rs | 7 | ||||
| -rw-r--r-- | compiler/rustc_borrowck/src/diagnostics/region_errors.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_hir/src/hir.rs | 93 | ||||
| -rw-r--r-- | compiler/rustc_hir/src/hir/tests.rs | 1 | ||||
| -rw-r--r-- | compiler/rustc_trait_selection/src/errors.rs | 19 | ||||
| -rw-r--r-- | tests/pretty/hir-lifetimes.pp | 4 | ||||
| -rw-r--r-- | tests/ui/stats/input-stats.stderr | 6 |
9 files changed, 136 insertions, 51 deletions
diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index c03d5e53d97..43bf951eddc 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/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, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index ced9064fd9f..d5d6dcd8d63 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/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) } diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index d00c797755f..c464c159c34 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/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) }), ); diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 6b11f1a3681..50a18b04de4 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/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())); diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 89249eab351..e3e96894ed1 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/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}")), } } } diff --git a/compiler/rustc_hir/src/hir/tests.rs b/compiler/rustc_hir/src/hir/tests.rs index f75b9662132..62ef02d2f50 100644 --- a/compiler/rustc_hir/src/hir/tests.rs +++ b/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, diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs index 7159397c4b1..b30390a9330 100644 --- a/compiler/rustc_trait_selection/src/errors.rs +++ b/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))); } _ => {} } diff --git a/tests/pretty/hir-lifetimes.pp b/tests/pretty/hir-lifetimes.pp index df3aba516b9..e545b0c8f57 100644 --- a/tests/pretty/hir-lifetimes.pp +++ b/tests/pretty/hir-lifetimes.pp @@ -73,7 +73,7 @@ fn h<'b, F>(f: F, y: Foo<'b>) where F: for<'d> MyTrait<'d, 'b> { } 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 @@ fn f() { { let _ = St{ x: &0,}; }; { let _ = St{ x: &0,}; }; } 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 = ""; diff --git a/tests/ui/stats/input-stats.stderr b/tests/ui/stats/input-stats.stderr index dbc9e7d254c..191daff2137 100644 --- a/tests/ui/stats/input-stats.stderr +++ b/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 |
