Skip to content

Commit 4f1b408

Browse files
nnethercotennethercote
committed
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.
1 parent 0c34070 commit 4f1b408

File tree

9 files changed

+136
-51
lines changed

9 files changed

+136
-51
lines changed

‎compiler/rustc_ast_lowering/src/item.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_ast::*;
55
use rustc_errors::ErrorGuaranteed;
66
use rustc_hir::def::{DefKind, Res};
77
use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
8-
use rustc_hir::{self as hir, HirId, PredicateOrigin};
8+
use rustc_hir::{self as hir, HirId, IsAnonInPath, PredicateOrigin};
99
use rustc_index::{IndexSlice, IndexVec};
1010
use rustc_middle::ty::{ResolverAstLowering, TyCtxt};
1111
use rustc_span::edit_distance::find_best_match_for_name;
@@ -1787,7 +1787,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
17871787
}
17881788
GenericParamKind::Lifetime => {
17891789
let lt_id = self.next_node_id();
1790-
let lifetime = self.new_named_lifetime(id, lt_id, ident);
1790+
let lifetime = self.new_named_lifetime(id, lt_id, ident, IsAnonInPath::No);
17911791
hir::WherePredicateKind::RegionPredicate(hir::WhereRegionPredicate {
17921792
lifetime,
17931793
bounds,

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle, StashKey};
5555
use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
5656
use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
5757
use rustc_hir::{
58-
self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
58+
self as hir, ConstArg, GenericArg, HirId, IsAnonInPath, ItemLocalMap, LangItem, ParamName,
59+
TraitCandidate,
5960
};
6061
use rustc_index::{Idx, IndexSlice, IndexVec};
6162
use rustc_macros::extension;
@@ -1777,7 +1778,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17771778
}
17781779

17791780
fn lower_lifetime(&mut self, l: &Lifetime) -> &'hir hir::Lifetime {
1780-
self.new_named_lifetime(l.id, l.id, l.ident)
1781+
self.new_named_lifetime(l.id, l.id, l.ident, IsAnonInPath::No)
1782+
}
1783+
1784+
fn lower_lifetime_anon_in_path(&mut self, id: NodeId, span: Span) -> &'hir hir::Lifetime {
1785+
self.new_named_lifetime(id, id, Ident::new(kw::UnderscoreLifetime, span), IsAnonInPath::Yes)
17811786
}
17821787

17831788
#[instrument(level = "debug", skip(self))]
@@ -1786,28 +1791,43 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17861791
id: NodeId,
17871792
new_id: NodeId,
17881793
ident: Ident,
1794+
is_anon_in_path: IsAnonInPath,
17891795
) -> &'hir hir::Lifetime {
1796+
debug_assert_ne!(ident.name, kw::Empty);
17901797
let res = self.resolver.get_lifetime_res(id).unwrap_or(LifetimeRes::Error);
17911798
let res = match res {
17921799
LifetimeRes::Param { param, .. } => hir::LifetimeName::Param(param),
17931800
LifetimeRes::Fresh { param, .. } => {
1801+
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
17941802
let param = self.local_def_id(param);
17951803
hir::LifetimeName::Param(param)
17961804
}
1797-
LifetimeRes::Infer => hir::LifetimeName::Infer,
1798-
LifetimeRes::Static { .. } => hir::LifetimeName::Static,
1805+
LifetimeRes::Infer => {
1806+
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
1807+
hir::LifetimeName::Infer
1808+
}
1809+
LifetimeRes::Static { .. } => {
1810+
debug_assert!(matches!(ident.name, kw::StaticLifetime | kw::UnderscoreLifetime));
1811+
hir::LifetimeName::Static
1812+
}
17991813
LifetimeRes::Error => hir::LifetimeName::Error,
18001814
LifetimeRes::ElidedAnchor { .. } => {
18011815
panic!("Unexpected `ElidedAnchar` {:?} at {:?}", ident, ident.span);
18021816
}
18031817
};
18041818

1819+
#[cfg(debug_assertions)]
1820+
if is_anon_in_path == IsAnonInPath::Yes {
1821+
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
1822+
}
1823+
18051824
debug!(?res);
1806-
self.arena.alloc(hir::Lifetime {
1807-
hir_id: self.lower_node_id(new_id),
1808-
ident: self.lower_ident(ident),
1825+
self.arena.alloc(hir::Lifetime::new(
1826+
self.lower_node_id(new_id),
1827+
self.lower_ident(ident),
18091828
res,
1810-
})
1829+
is_anon_in_path,
1830+
))
18111831
}
18121832

18131833
fn lower_generic_params_mut(
@@ -2391,11 +2411,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
23912411
/// when the bound is written, even if it is written with `'_` like in
23922412
/// `Box<dyn Debug + '_>`. In those cases, `lower_lifetime` is invoked.
23932413
fn elided_dyn_bound(&mut self, span: Span) -> &'hir hir::Lifetime {
2394-
let r = hir::Lifetime {
2395-
hir_id: self.next_id(),
2396-
ident: Ident::new(kw::Empty, self.lower_span(span)),
2397-
res: hir::LifetimeName::ImplicitObjectLifetimeDefault,
2398-
};
2414+
let r = hir::Lifetime::new(
2415+
self.next_id(),
2416+
Ident::new(kw::UnderscoreLifetime, self.lower_span(span)),
2417+
hir::LifetimeName::ImplicitObjectLifetimeDefault,
2418+
IsAnonInPath::No,
2419+
);
23992420
debug!("elided_dyn_bound: r={:?}", r);
24002421
self.arena.alloc(r)
24012422
}

‎compiler/rustc_ast_lowering/src/path.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_hir::def::{DefKind, PartialRes, Res};
77
use rustc_hir::def_id::DefId;
88
use rustc_middle::span_bug;
99
use rustc_session::parse::add_feature_diagnostics;
10-
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, kw, sym};
10+
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, sym};
1111
use smallvec::{SmallVec, smallvec};
1212
use tracing::{debug, instrument};
1313

@@ -450,10 +450,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
450450
0,
451451
(start.as_u32()..end.as_u32()).map(|i| {
452452
let id = NodeId::from_u32(i);
453-
let l = self.lower_lifetime(&Lifetime {
454-
id,
455-
ident: Ident::new(kw::Empty, elided_lifetime_span),
456-
});
453+
let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span);
457454
GenericArg::Lifetime(l)
458455
}),
459456
);

‎compiler/rustc_borrowck/src/diagnostics/region_errors.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,14 +513,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
513513
ty::VarianceDiagInfo::Invariant { ty, param_index } => {
514514
let (desc, note) = match ty.kind() {
515515
ty::RawPtr(ty, mutbl) => {
516-
assert_eq!(*mutbl, rustc_hir::Mutability::Mut);
516+
assert_eq!(*mutbl, hir::Mutability::Mut);
517517
(
518518
format!("a mutable pointer to `{}`", ty),
519519
"mutable pointers are invariant over their type parameter".to_string(),
520520
)
521521
}
522522
ty::Ref(_, inner_ty, mutbl) => {
523-
assert_eq!(*mutbl, rustc_hir::Mutability::Mut);
523+
assert_eq!(*mutbl, hir::Mutability::Mut);
524524
(
525525
format!("a mutable reference to `{inner_ty}`"),
526526
"mutable references are invariant over their type parameter"
@@ -887,7 +887,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
887887
// Skip `async` desugaring `impl Future`.
888888
}
889889
if let TyKind::TraitObject(_, lt) = alias_ty.kind {
890-
if lt.ident.name == kw::Empty {
890+
if lt.res == hir::LifetimeName::ImplicitObjectLifetimeDefault {
891891
spans_suggs.push((lt.ident.span.shrink_to_hi(), " + 'a".to_string()));
892892
} else {
893893
spans_suggs.push((lt.ident.span, "'a".to_string()));

‎compiler/rustc_hir/src/hir.rs

Lines changed: 78 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,60 @@ use crate::def_id::{DefId, LocalDefIdMap};
3535
pub(crate) use crate::hir_id::{HirId, ItemLocalId, ItemLocalMap, OwnerId};
3636
use crate::intravisit::{FnKind, VisitorExt};
3737

38+
#[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable_Generic)]
39+
pub enum IsAnonInPath {
40+
No,
41+
Yes,
42+
}
43+
44+
/// A lifetime. The valid field combinations are non-obvious. The following
45+
/// example shows some of them. See also the comments on `LifetimeName`.
46+
/// ```
47+
/// #[repr(C)]
48+
/// struct S<'a>(&'a u32); // LifetimeName::Param, name='a, IsAnonInPath::No
49+
/// extern "C" {
50+
/// fn f1(s: S); // LifetimeName::Param, name='_, IsAnonInPath::Yes
51+
/// fn f2(s: S<'_>); // LifetimeName::Param, name='_, IsAnonInPath::No
52+
/// fn f3<'a>(s: S<'a>); // LifetimeName::Param, name='a, IsAnonInPath::No
53+
/// }
54+
///
55+
/// struct St<'a> { x: &'a u32 } // LifetimeName::Param, name='a, IsAnonInPath::No
56+
/// fn f() {
57+
/// _ = St { x: &0 }; // LifetimeName::Infer, name='_, IsAnonInPath::Yes
58+
/// _ = St::<'_> { x: &0 }; // LifetimeName::Infer, name='_, IsAnonInPath::No
59+
/// }
60+
///
61+
/// struct Name<'a>(&'a str); // LifetimeName::Param, name='a, IsAnonInPath::No
62+
/// const A: Name = Name("a"); // LifetimeName::Static, name='_, IsAnonInPath::Yes
63+
/// const B: &str = ""; // LifetimeName::Static, name='_, IsAnonInPath::No
64+
/// static C: &'_ str = ""; // LifetimeName::Static, name='_, IsAnonInPath::No
65+
/// static D: &'static str = ""; // LifetimeName::Static, name='static, IsAnonInPath::No
66+
///
67+
/// trait Tr {}
68+
/// fn tr(_: Box<dyn Tr>) {} // LifetimeName::ImplicitObjectLifetimeDefault,
69+
/// name='_, IsAnonInPath::No
70+
/// // (commented out because these cases trigger errors)
71+
/// // struct S1<'a>(&'a str); // LifetimeName::Param, name='a, IsAnonInPath::No
72+
/// // struct S2(S1); // LifetimeName::Error, name='_, IsAnonInPath::Yes
73+
/// // struct S3(S1<'_>); // LifetimeName::Error, name='_, IsAnonInPath::No
74+
/// // struct S4(S1<'a>); // LifetimeName::Error, name='a, IsAnonInPath::No
75+
/// ```
3876
#[derive(Debug, Copy, Clone, HashStable_Generic)]
3977
pub struct Lifetime {
4078
#[stable_hasher(ignore)]
4179
pub hir_id: HirId,
4280

43-
/// Either "`'a`", referring to a named lifetime definition,
44-
/// `'_` referring to an anonymous lifetime (either explicitly `'_` or `&type`),
45-
/// or "``" (i.e., `kw::Empty`) when appearing in path.
46-
///
47-
/// See `Lifetime::suggestion_position` for practical use.
81+
/// Either a named lifetime definition (e.g. `'a`, `'static`) or an
82+
/// anonymous lifetime (`'_`, either explicitly written, or inserted for
83+
/// things like `&type`).
4884
pub ident: Ident,
4985

5086
/// Semantics of this lifetime.
5187
pub res: LifetimeName,
88+
89+
/// Is the lifetime anonymous and in a path? Used only for error
90+
/// suggestions. See `Lifetime::suggestion` for example use.
91+
pub is_anon_in_path: IsAnonInPath,
5292
}
5393

5494
#[derive(Debug, Copy, Clone, HashStable_Generic)]
@@ -111,11 +151,12 @@ pub enum LifetimeName {
111151
/// that was already reported.
112152
Error,
113153

114-
/// User wrote an anonymous lifetime, either `'_` or nothing.
115-
/// The semantics of this lifetime should be inferred by typechecking code.
154+
/// User wrote an anonymous lifetime, either `'_` or nothing (which gets
155+
/// converted to `'_`). The semantics of this lifetime should be inferred
156+
/// by typechecking code.
116157
Infer,
117158

118-
/// User wrote `'static`.
159+
/// User wrote `'static` or nothing (which gets converted to `'_`).
119160
Static,
120161
}
121162

@@ -135,34 +176,56 @@ impl LifetimeName {
135176

136177
impl fmt::Display for Lifetime {
137178
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
138-
if self.ident.name != kw::Empty { self.ident.name.fmt(f) } else { "'_".fmt(f) }
179+
self.ident.name.fmt(f)
139180
}
140181
}
141182

142183
impl Lifetime {
184+
pub fn new(
185+
hir_id: HirId,
186+
ident: Ident,
187+
res: LifetimeName,
188+
is_anon_in_path: IsAnonInPath,
189+
) -> Lifetime {
190+
let lifetime = Lifetime { hir_id, ident, res, is_anon_in_path };
191+
192+
// Sanity check: elided lifetimes form a strict subset of anonymous lifetimes.
193+
#[cfg(debug_assertions)]
194+
match (lifetime.is_elided(), lifetime.is_anonymous()) {
195+
(false, false) => {} // e.g. `'a`
196+
(false, true) => {} // e.g. explicit `'_`
197+
(true, true) => {} // e.g. `&x`
198+
(true, false) => panic!("bad Lifetime"),
199+
}
200+
201+
lifetime
202+
}
203+
143204
pub fn is_elided(&self) -> bool {
144205
self.res.is_elided()
145206
}
146207

147208
pub fn is_anonymous(&self) -> bool {
148-
self.ident.name == kw::Empty || self.ident.name == kw::UnderscoreLifetime
209+
self.ident.name == kw::UnderscoreLifetime
149210
}
150211

151212
pub fn suggestion(&self, new_lifetime: &str) -> (Span, String) {
152213
debug_assert!(new_lifetime.starts_with('\''));
153214

154-
match (self.ident.name.is_empty(), self.ident.span.is_empty()) {
215+
match (self.is_anon_in_path, self.ident.span.is_empty()) {
155216
// The user wrote `Path<T>`, and omitted the `'_,`.
156-
(true, true) => (self.ident.span, format!("{new_lifetime}, ")),
217+
(IsAnonInPath::Yes, true) => (self.ident.span, format!("{new_lifetime}, ")),
157218

158219
// The user wrote `Path` and omitted the `<'_>`.
159-
(true, false) => (self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>")),
220+
(IsAnonInPath::Yes, false) => {
221+
(self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>"))
222+
}
160223

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

164227
// The user wrote `'a` or `'_`.
165-
(false, false) => (self.ident.span, format!("{new_lifetime}")),
228+
(IsAnonInPath::No, false) => (self.ident.span, format!("{new_lifetime}")),
166229
}
167230
}
168231
}

‎compiler/rustc_hir/src/hir/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ fn trait_object_roundtrips_impl(syntax: TraitObjectSyntax) {
5858
hir_id: HirId::INVALID,
5959
ident: Ident::new(sym::name, DUMMY_SP),
6060
res: LifetimeName::Static,
61+
is_anon_in_path: IsAnonInPath::No,
6162
}
6263
},
6364
syntax,

‎compiler/rustc_trait_selection/src/errors.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use rustc_errors::{
99
use rustc_hir::def::DefKind;
1010
use rustc_hir::def_id::{DefId, LocalDefId};
1111
use rustc_hir::intravisit::{Visitor, VisitorExt, walk_ty};
12-
use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, Node};
12+
use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, IsAnonInPath, Node};
1313
use rustc_macros::{Diagnostic, Subdiagnostic};
1414
use rustc_middle::ty::print::{PrintTraitRefExt as _, TraitRefPrintOnlyTraitPath};
1515
use rustc_middle::ty::{self, Binder, ClosureKind, FnSig, Region, Ty, TyCtxt};
@@ -567,10 +567,14 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
567567

568568
impl<'v> Visitor<'v> for ImplicitLifetimeFinder {
569569
fn visit_ty(&mut self, ty: &'v hir::Ty<'v, AmbigArg>) {
570-
let make_suggestion = |ident: Ident| {
571-
if ident.name == kw::Empty && ident.span.is_empty() {
570+
let make_suggestion = |lifetime: &hir::Lifetime| {
571+
if lifetime.is_anon_in_path == IsAnonInPath::Yes
572+
&& lifetime.ident.span.is_empty()
573+
{
572574
format!("{}, ", self.suggestion_param_name)
573-
} else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() {
575+
} else if lifetime.ident.name == kw::UnderscoreLifetime
576+
&& lifetime.ident.span.is_empty()
577+
{
574578
format!("{} ", self.suggestion_param_name)
575579
} else {
576580
self.suggestion_param_name.clone()
@@ -584,7 +588,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
584588
matches!(
585589
arg,
586590
hir::GenericArg::Lifetime(lifetime)
587-
if lifetime.ident.name == kw::Empty
591+
if lifetime.is_anon_in_path == IsAnonInPath::Yes
588592
)
589593
}) {
590594
self.suggestions.push((
@@ -605,7 +609,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
605609
{
606610
self.suggestions.push((
607611
lifetime.ident.span,
608-
make_suggestion(lifetime.ident),
612+
make_suggestion(lifetime),
609613
));
610614
}
611615
}
@@ -614,8 +618,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
614618
}
615619
}
616620
hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => {
617-
self.suggestions
618-
.push((lifetime.ident.span, make_suggestion(lifetime.ident)));
621+
self.suggestions.push((lifetime.ident.span, make_suggestion(lifetime)));
619622
}
620623
_ => {}
621624
}

‎tests/pretty/hir-lifetimes.pp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
struct S<'a>(&'a u32);
7474

7575
extern "C" {
76-
unsafe fn g1(s: S<>);
76+
unsafe fn g1(s: S<'_>);
7777
unsafe fn g2(s: S<'_>);
7878
unsafe fn g3<'a>(s: S<'a>);
7979
}
@@ -86,7 +86,7 @@
8686

8787
struct Name<'a>(&'a str);
8888

89-
const A: Name<> = Name("a");
89+
const A: Name<'_> = Name("a");
9090
const B: &'_ str = "";
9191
static C: &'_ str = "";
9292
static D: &'static str = "";

0 commit comments

Comments
 (0)