Skip to content

Commit 3eda3db

Browse files
nnethercotennethercote
committed
Improve Lifetime::suggestion.
It currently used `span.is_empty()` for the `is_anon_in_path` cases, and also adjusts the spans when they are non-empty. All this is very hard to understand. XXX - New `Option<AngleBracket>` type makes things much clearer. - Simpler spans in `maybe_insert_elided_lifetimes_in_path`. They're now all empty, and point to the lifetime insertion point. `suggestion` no longer has to call `shrink_to_hi` on any spans. - `make_suggestion` closure now just calls `Lifetime::suggestion`, because it's doing the same thing Finally, some error messages are improved. First, on the `Path` case, we have this diff: LL | fn f(_: impl Foo) {} - | ^^^ expected named lifetime parameter + | ^ expected named lifetime parameter The carets now point to where the named lifetime parameter must be inserted, as per the subsequent suggestion: LL | fn f<'a>(_: impl Foo<'a>) {} | ++++ ++++ Similar story on this diff on the `Path<>` case: LL | fn f(_: impl Foo<>) {} - | ^ expected named lifetime parameter + | ^ expected named lifetime parameter Plus we print the `Path<>` suggestion properly: -LL | fn f<'a>(_: impl Foo<<'a>>) {} - | ++++ ++++ +LL | fn f<'a>(_: impl Foo<'a>) {} + | ++++ ++
1 parent a719d8b commit 3eda3db

File tree

7 files changed

+112
-88
lines changed

7 files changed

+112
-88
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, IsAnonInPath, PredicateOrigin};
8+
use rustc_hir::{self as hir, HirId, 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;
@@ -1823,7 +1823,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
18231823
}
18241824
GenericParamKind::Lifetime => {
18251825
let lt_id = self.next_node_id();
1826-
let lifetime = self.new_named_lifetime(id, lt_id, ident, IsAnonInPath::No);
1826+
let lifetime = self.new_named_lifetime(id, lt_id, ident, None);
18271827
hir::WherePredicateKind::RegionPredicate(hir::WhereRegionPredicate {
18281828
lifetime,
18291829
bounds,

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ 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, IsAnonInPath, ItemLocalMap, LangItem, ParamName,
59-
TraitCandidate,
58+
self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
6059
};
6160
use rustc_index::{Idx, IndexSlice, IndexVec};
6261
use rustc_macros::extension;
@@ -1756,11 +1755,21 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17561755
}
17571756

17581757
fn lower_lifetime(&mut self, l: &Lifetime) -> &'hir hir::Lifetime {
1759-
self.new_named_lifetime(l.id, l.id, l.ident, IsAnonInPath::No)
1758+
self.new_named_lifetime(l.id, l.id, l.ident, None)
17601759
}
17611760

1762-
fn lower_lifetime_anon_in_path(&mut self, id: NodeId, span: Span) -> &'hir hir::Lifetime {
1763-
self.new_named_lifetime(id, id, Ident::new(kw::UnderscoreLifetime, span), IsAnonInPath::Yes)
1761+
fn lower_lifetime_anon_in_path(
1762+
&mut self,
1763+
id: NodeId,
1764+
span: Span,
1765+
angle_brackets: hir::AngleBrackets,
1766+
) -> &'hir hir::Lifetime {
1767+
self.new_named_lifetime(
1768+
id,
1769+
id,
1770+
Ident::new(kw::UnderscoreLifetime, span),
1771+
Some(angle_brackets),
1772+
)
17641773
}
17651774

17661775
#[instrument(level = "debug", skip(self))]
@@ -1769,7 +1778,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17691778
id: NodeId,
17701779
new_id: NodeId,
17711780
ident: Ident,
1772-
is_anon_in_path: IsAnonInPath,
1781+
is_anon_in_path: hir::IsAnonInPath,
17731782
) -> &'hir hir::Lifetime {
17741783
debug_assert_ne!(ident.name, kw::Empty);
17751784
let res = self.resolver.get_lifetime_res(id).unwrap_or(LifetimeRes::Error);
@@ -1795,7 +1804,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17951804
};
17961805

17971806
#[cfg(debug_assertions)]
1798-
if is_anon_in_path == IsAnonInPath::Yes {
1807+
if is_anon_in_path.is_some() {
17991808
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
18001809
}
18011810

@@ -2393,7 +2402,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
23932402
self.next_id(),
23942403
Ident::new(kw::UnderscoreLifetime, self.lower_span(span)),
23952404
hir::LifetimeName::ImplicitObjectLifetimeDefault,
2396-
IsAnonInPath::No,
2405+
None,
23972406
);
23982407
debug!("elided_dyn_bound: r={:?}", r);
23992408
self.arena.alloc(r)

‎compiler/rustc_ast_lowering/src/path.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -433,24 +433,34 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
433433

434434
// Note: these spans are used for diagnostics when they can't be inferred.
435435
// See rustc_resolve::late::lifetimes::LifetimeContext::add_missing_lifetime_specifiers_label
436-
let elided_lifetime_span = if generic_args.span.is_empty() {
437-
// If there are no brackets, use the identifier span.
436+
let (elided_lifetime_span, angle_brackets) = if generic_args.span.is_empty() {
437+
// No brackets, e.g. `Path`: use an empty span just past the end of the identifier.
438438
// HACK: we use find_ancestor_inside to properly suggest elided spans in paths
439439
// originating from macros, since the segment's span might be from a macro arg.
440-
segment_ident_span.find_ancestor_inside(path_span).unwrap_or(path_span)
441-
} else if generic_args.is_empty() {
442-
// If there are brackets, but not generic arguments, then use the opening bracket
443-
generic_args.span.with_hi(generic_args.span.lo() + BytePos(1))
440+
(
441+
segment_ident_span
442+
.find_ancestor_inside(path_span)
443+
.unwrap_or(path_span)
444+
.shrink_to_hi(),
445+
hir::AngleBrackets::Missing,
446+
)
444447
} else {
445-
// Else use an empty span right after the opening bracket.
446-
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo()
448+
// Brackets, e.g. `Path<>` or `Path<T>`: use an empty span just after the `<`.
449+
(
450+
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo(),
451+
if generic_args.is_empty() {
452+
hir::AngleBrackets::Empty
453+
} else {
454+
hir::AngleBrackets::Full
455+
},
456+
)
447457
};
448458

449459
generic_args.args.insert_many(
450460
0,
451461
(start.as_u32()..end.as_u32()).map(|i| {
452462
let id = NodeId::from_u32(i);
453-
let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span);
463+
let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span, angle_brackets);
454464
GenericArg::Lifetime(l)
455465
}),
456466
);

‎compiler/rustc_hir/src/hir.rs

Lines changed: 55 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -36,42 +36,50 @@ pub(crate) use crate::hir_id::{HirId, ItemLocalId, ItemLocalMap, OwnerId};
3636
use crate::intravisit::{FnKind, VisitorExt};
3737

3838
#[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable_Generic)]
39-
pub enum IsAnonInPath {
40-
No,
41-
Yes,
39+
pub enum AngleBrackets {
40+
/// E.g. `Path`.
41+
Missing,
42+
/// E.g. `Path<>`.
43+
Empty,
44+
/// E.g. `Path<T>`.
45+
Full,
4246
}
4347

48+
pub type IsAnonInPath = Option<AngleBrackets>;
49+
4450
/// A lifetime. The valid field combinations are non-obvious. The following
4551
/// example shows some of them. See also the comments on `LifetimeName`.
4652
/// ```
4753
/// #[repr(C)]
48-
/// struct S<'a>(&'a u32); // res=Param, name='a, IsAnonInPath::No
54+
/// struct S<'a>(&'a u32); // res=Param, name='a, is_anon_in_path=None
4955
/// unsafe extern "C" {
50-
/// fn f1(s: S); // res=Param, name='_, IsAnonInPath::Yes
51-
/// fn f2(s: S<'_>); // res=Param, name='_, IsAnonInPath::No
52-
/// fn f3<'a>(s: S<'a>); // res=Param, name='a, IsAnonInPath::No
56+
/// fn f1(s: S); // res=Param, name='_, is_anon_in_path=Some(Missing)
57+
/// fn f2(s: S<>); // res=Param, name='_, is_anon_in_path=Some(Empty)
58+
/// fn f3(s: S<'_>); // res=Param, name='_, is_anon_in_path=None
59+
/// fn f4<'a>(s: S<'a>); // res=Param, name='a, is_anon_in_path=None
5360
/// }
5461
///
55-
/// struct St<'a> { x: &'a u32 } // res=Param, name='a, IsAnonInPath::No
62+
/// struct St<'a, T> { x: &'a T } // res=Param, name='a, is_anon_in_path=None
5663
/// fn f() {
57-
/// _ = St { x: &0 }; // res=Infer, name='_, IsAnonInPath::Yes
58-
/// _ = St::<'_> { x: &0 }; // res=Infer, name='_, IsAnonInPath::No
64+
/// _ = St::<u8> { x: &0 }; // res=Infer, name='_, is_anon_in_path=Some(Full)
65+
/// _ = St::<'_, u8> { x: &0 }; // res=Infer, name='_, is_anon_in_path=None
5966
/// }
6067
///
61-
/// struct Name<'a>(&'a str); // res=Param, name='a, IsAnonInPath::No
62-
/// const A: Name = Name("a"); // res=Static, name='_, IsAnonInPath::Yes
63-
/// const B: &str = ""; // res=Static, name='_, IsAnonInPath::No
64-
/// static C: &'_ str = ""; // res=Static, name='_, IsAnonInPath::No
65-
/// static D: &'static str = ""; // res=Static, name='static, IsAnonInPath::No
68+
/// struct Name<'a>(&'a str); // res=Param, name='a, is_anon_in_path=None
69+
/// const A: Name = Name("a"); // res=Static, name='_, is_anon_in_path=Some(Missing)
70+
/// const B: &str = ""; // res=Static, name='_, is_anon_in_path=None
71+
/// static C: &'_ str = ""; // res=Static, name='_, is_anon_in_path=None
72+
/// static D: &'static str = ""; // res=Static, name='static, is_anon_in_path=None
6673
///
6774
/// trait Tr {}
68-
/// fn tr(_: Box<dyn Tr>) {} // res=ImplicitObjectLifetimeDefault, name='_, IsAnonInPath::No
75+
/// fn tr(_: Box<dyn Tr>) {} // res=ImplicitObjectLifetimeDefault,
76+
/// // name='_, is_anon_in_path=None
6977
///
7078
/// // (commented out because these cases trigger errors)
71-
/// // struct S1<'a>(&'a str); // res=Param, name='a, IsAnonInPath::No
72-
/// // struct S2(S1); // res=Error, name='_, IsAnonInPath::Yes
73-
/// // struct S3(S1<'_>); // res=Error, name='_, IsAnonInPath::No
74-
/// // struct S4(S1<'a>); // res=Error, name='a, IsAnonInPath::No
79+
/// // struct S1<'a>(&'a str); // res=Param, name='a, is_anon_in_path=None
80+
/// // struct S2(S1<>); // res=Error, name='_, is_anon_in_path=Some(Empty)
81+
/// // struct S3(S1<'_>); // res=Error, name='_, is_anon_in_path=None
82+
/// // struct S4(S1<'a>); // res=Error, name='a, is_anon_in_path=None
7583
/// ```
7684
#[derive(Debug, Copy, Clone, HashStable_Generic)]
7785
pub struct Lifetime {
@@ -86,8 +94,9 @@ pub struct Lifetime {
8694
/// Semantics of this lifetime.
8795
pub res: LifetimeName,
8896

89-
/// Is the lifetime anonymous and in a path? Used only for error
90-
/// suggestions. See `Lifetime::suggestion` for example use.
97+
/// Is the lifetime anonymous and in a path? And if so, what do the path
98+
/// generics look like? Used only for error suggestions. See
99+
/// `Lifetime::suggestion` for example use.
91100
pub is_anon_in_path: IsAnonInPath,
92101
}
93102

@@ -212,21 +221,31 @@ impl Lifetime {
212221
pub fn suggestion(&self, new_lifetime: &str) -> (Span, String) {
213222
debug_assert!(new_lifetime.starts_with('\''));
214223

215-
match (self.is_anon_in_path, self.ident.span.is_empty()) {
216-
// The user wrote `Path<T>`, and omitted the `'_,`.
217-
(IsAnonInPath::Yes, true) => (self.ident.span, format!("{new_lifetime}, ")),
218-
219-
// The user wrote `Path` and omitted the `<'_>`.
220-
(IsAnonInPath::Yes, false) => {
221-
(self.ident.span.shrink_to_hi(), format!("<{new_lifetime}>"))
224+
let s = match self.is_anon_in_path {
225+
Some(AngleBrackets::Missing) => {
226+
// `Path`: insert suggestion just after the identifier.
227+
format!("<{new_lifetime}>")
222228
}
223-
224-
// The user wrote `&type` or `&mut type`.
225-
(IsAnonInPath::No, true) => (self.ident.span, format!("{new_lifetime} ")),
226-
227-
// The user wrote `'a` or `'_`.
228-
(IsAnonInPath::No, false) => (self.ident.span, format!("{new_lifetime}")),
229-
}
229+
Some(AngleBrackets::Empty) => {
230+
// `Path<>`: insert suggestion just after the `<`.
231+
format!("{new_lifetime}")
232+
}
233+
Some(AngleBrackets::Full) => {
234+
// `Path<T>`: insert suggestion just after the `<`.
235+
format!("{new_lifetime}, ")
236+
}
237+
None => {
238+
if self.ident.span.is_empty() {
239+
// The user wrote `&type` or `&mut type`.
240+
// njn: does this get the mut case right? `&mut 'atype`?
241+
format!("{new_lifetime} ")
242+
} else {
243+
// The user wrote `'a` or `'_`.
244+
format!("{new_lifetime}")
245+
}
246+
}
247+
};
248+
(self.ident.span, s)
230249
}
231250
}
232251

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +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,
61+
is_anon_in_path: None,
6262
}
6363
},
6464
syntax,

‎compiler/rustc_trait_selection/src/errors.rs

Lines changed: 6 additions & 20 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, IsAnonInPath, Node};
12+
use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, 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,19 +567,8 @@ 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 = |lifetime: &hir::Lifetime| {
571-
if lifetime.is_anon_in_path == IsAnonInPath::Yes
572-
&& lifetime.ident.span.is_empty()
573-
{
574-
format!("{}, ", self.suggestion_param_name)
575-
} else if lifetime.ident.name == kw::UnderscoreLifetime
576-
&& lifetime.ident.span.is_empty()
577-
{
578-
format!("{} ", self.suggestion_param_name)
579-
} else {
580-
self.suggestion_param_name.clone()
581-
}
582-
};
570+
let make_suggestion =
571+
|lifetime: &hir::Lifetime| lifetime.suggestion(&self.suggestion_param_name);
583572
match ty.kind {
584573
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => {
585574
for segment in path.segments {
@@ -588,7 +577,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
588577
matches!(
589578
arg,
590579
hir::GenericArg::Lifetime(lifetime)
591-
if lifetime.is_anon_in_path == IsAnonInPath::Yes
580+
if lifetime.is_anon_in_path.is_some()
592581
)
593582
}) {
594583
self.suggestions.push((
@@ -607,18 +596,15 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
607596
if let hir::GenericArg::Lifetime(lifetime) = arg
608597
&& lifetime.is_anonymous()
609598
{
610-
self.suggestions.push((
611-
lifetime.ident.span,
612-
make_suggestion(lifetime),
613-
));
599+
self.suggestions.push(make_suggestion(lifetime));
614600
}
615601
}
616602
}
617603
}
618604
}
619605
}
620606
hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => {
621-
self.suggestions.push((lifetime.ident.span, make_suggestion(lifetime)));
607+
self.suggestions.push(make_suggestion(lifetime));
622608
}
623609
_ => {}
624610
}

‎tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,10 @@ LL + fn g<'a>(mut x: impl Iterator<Item = &'a ()>) -> Option<&'_ ()> { x.nex
212212
|
213213

214214
error[E0658]: anonymous lifetimes in `impl Trait` are unstable
215-
--> $DIR/impl-trait-missing-lifetime-gated.rs:44:18
215+
--> $DIR/impl-trait-missing-lifetime-gated.rs:44:21
216216
|
217217
LL | fn f(_: impl Foo) {}
218-
| ^^^ expected named lifetime parameter
218+
| ^ expected named lifetime parameter
219219
|
220220
= help: add `#![feature(anonymous_lifetime_in_impl_trait)]` to the crate attributes to enable
221221
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
@@ -225,10 +225,10 @@ LL | fn f<'a>(_: impl Foo<'a>) {}
225225
| ++++ ++++
226226

227227
error[E0658]: anonymous lifetimes in `impl Trait` are unstable
228-
--> $DIR/impl-trait-missing-lifetime-gated.rs:47:22
228+
--> $DIR/impl-trait-missing-lifetime-gated.rs:47:25
229229
|
230230
LL | fn g(mut x: impl Foo) -> Option<&()> { x.next() }
231-
| ^^^ expected named lifetime parameter
231+
| ^ expected named lifetime parameter
232232
|
233233
= help: add `#![feature(anonymous_lifetime_in_impl_trait)]` to the crate attributes to enable
234234
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
@@ -238,30 +238,30 @@ LL | fn g<'a>(mut x: impl Foo<'a>) -> Option<&()> { x.next() }
238238
| ++++ ++++
239239

240240
error[E0658]: anonymous lifetimes in `impl Trait` are unstable
241-
--> $DIR/impl-trait-missing-lifetime-gated.rs:55:21
241+
--> $DIR/impl-trait-missing-lifetime-gated.rs:55:22
242242
|
243243
LL | fn f(_: impl Foo<>) {}
244-
| ^ expected named lifetime parameter
244+
| ^ expected named lifetime parameter
245245
|
246246
= help: add `#![feature(anonymous_lifetime_in_impl_trait)]` to the crate attributes to enable
247247
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
248248
help: consider introducing a named lifetime parameter
249249
|
250-
LL | fn f<'a>(_: impl Foo<<'a>>) {}
251-
| ++++ ++++
250+
LL | fn f<'a>(_: impl Foo<'a>) {}
251+
| ++++ ++
252252

253253
error[E0658]: anonymous lifetimes in `impl Trait` are unstable
254-
--> $DIR/impl-trait-missing-lifetime-gated.rs:58:25
254+
--> $DIR/impl-trait-missing-lifetime-gated.rs:58:26
255255
|
256256
LL | fn g(mut x: impl Foo<>) -> Option<&()> { x.next() }
257-
| ^ expected named lifetime parameter
257+
| ^ expected named lifetime parameter
258258
|
259259
= help: add `#![feature(anonymous_lifetime_in_impl_trait)]` to the crate attributes to enable
260260
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
261261
help: consider introducing a named lifetime parameter
262262
|
263-
LL | fn g<'a>(mut x: impl Foo<<'a>>) -> Option<&()> { x.next() }
264-
| ++++ ++++
263+
LL | fn g<'a>(mut x: impl Foo<'a>) -> Option<&()> { x.next() }
264+
| ++++ ++
265265

266266
error[E0658]: anonymous lifetimes in `impl Trait` are unstable
267267
--> $DIR/impl-trait-missing-lifetime-gated.rs:66:22

0 commit comments

Comments
 (0)