Skip to content

Improve Lifetime::suggestion #139046

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.

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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, IsAnonInPath, PredicateOrigin};
use rustc_hir::{self as hir, HirId, 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, IsAnonInPath::No);
let lifetime = self.new_named_lifetime(id, lt_id, ident, None);
hir::WherePredicateKind::RegionPredicate(hir::WhereRegionPredicate {
lifetime,
bounds,
Expand Down
25 changes: 17 additions & 8 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ 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, IsAnonInPath, ItemLocalMap, LangItem, ParamName,
TraitCandidate,
self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
};
use rustc_index::{Idx, IndexSlice, IndexVec};
use rustc_macros::extension;
Expand Down Expand Up @@ -1756,11 +1755,21 @@ 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, IsAnonInPath::No)
self.new_named_lifetime(l.id, l.id, l.ident, None)
}

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)
fn lower_lifetime_anon_in_path(
&mut self,
id: NodeId,
span: Span,
angle_brackets: hir::AngleBrackets,
) -> &'hir hir::Lifetime {
self.new_named_lifetime(
id,
id,
Ident::new(kw::UnderscoreLifetime, span),
Some(angle_brackets),
)
}

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

#[cfg(debug_assertions)]
if is_anon_in_path == IsAnonInPath::Yes {
if is_anon_in_path.is_some() {
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
}

Expand Down Expand Up @@ -2393,7 +2402,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.next_id(),
Ident::new(kw::UnderscoreLifetime, self.lower_span(span)),
hir::LifetimeName::ImplicitObjectLifetimeDefault,
IsAnonInPath::No,
None,
);
debug!("elided_dyn_bound: r={:?}", r);
self.arena.alloc(r)
Expand Down
28 changes: 19 additions & 9 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,24 +433,34 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

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

generic_args.args.insert_many(
0,
(start.as_u32()..end.as_u32()).map(|i| {
let id = NodeId::from_u32(i);
let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span);
let l = self.lower_lifetime_anon_in_path(id, elided_lifetime_span, angle_brackets);
GenericArg::Lifetime(l)
}),
);
Expand Down
91 changes: 55 additions & 36 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,42 +36,50 @@ 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,
pub enum AngleBrackets {
/// E.g. `Path`.
Missing,
/// E.g. `Path<>`.
Empty,
/// E.g. `Path<T>`.
Full,
}

pub type IsAnonInPath = Option<AngleBrackets>;

/// 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
/// struct S<'a>(&'a u32); // res=Param, name='a, is_anon_in_path=None
/// 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
/// fn f1(s: S); // res=Param, name='_, is_anon_in_path=Some(Missing)
/// fn f2(s: S<>); // res=Param, name='_, is_anon_in_path=Some(Empty)
/// fn f3(s: S<'_>); // res=Param, name='_, is_anon_in_path=None
/// fn f4<'a>(s: S<'a>); // res=Param, name='a, is_anon_in_path=None
/// }
///
/// struct St<'a> { x: &'a u32 } // res=Param, name='a, IsAnonInPath::No
/// struct St<'a, T> { x: &'a T } // res=Param, name='a, is_anon_in_path=None
/// fn f() {
/// _ = St { x: &0 }; // res=Infer, name='_, IsAnonInPath::Yes
/// _ = St::<'_> { x: &0 }; // res=Infer, name='_, IsAnonInPath::No
/// _ = St::<u8> { x: &0 }; // res=Infer, name='_, is_anon_in_path=Some(Full)
/// _ = St::<'_, u8> { x: &0 }; // res=Infer, name='_, is_anon_in_path=None
/// }
///
/// 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
/// struct Name<'a>(&'a str); // res=Param, name='a, is_anon_in_path=None
/// const A: Name = Name("a"); // res=Static, name='_, is_anon_in_path=Some(Missing)
/// const B: &str = ""; // res=Static, name='_, is_anon_in_path=None
/// static C: &'_ str = ""; // res=Static, name='_, is_anon_in_path=None
/// static D: &'static str = ""; // res=Static, name='static, is_anon_in_path=None
///
/// trait Tr {}
/// fn tr(_: Box<dyn Tr>) {} // res=ImplicitObjectLifetimeDefault, name='_, IsAnonInPath::No
/// fn tr(_: Box<dyn Tr>) {} // res=ImplicitObjectLifetimeDefault,
/// // name='_, is_anon_in_path=None
///
/// // (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
/// // struct S1<'a>(&'a str); // res=Param, name='a, is_anon_in_path=None
/// // struct S2(S1<>); // res=Error, name='_, is_anon_in_path=Some(Empty)
/// // struct S3(S1<'_>); // res=Error, name='_, is_anon_in_path=None
/// // struct S4(S1<'a>); // res=Error, name='a, is_anon_in_path=None
/// ```
#[derive(Debug, Copy, Clone, HashStable_Generic)]
pub struct Lifetime {
Expand All @@ -86,8 +94,9 @@ pub struct Lifetime {
/// 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.
/// Is the lifetime anonymous and in a path? And if so, what do the path
/// generics look like? Used only for error suggestions. See
/// `Lifetime::suggestion` for example use.
pub is_anon_in_path: IsAnonInPath,
}

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

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}>"))
let s = match self.is_anon_in_path {
Some(AngleBrackets::Missing) => {
// `Path`: insert suggestion just after the identifier.
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}")),
}
Some(AngleBrackets::Empty) => {
// `Path<>`: insert suggestion just after the `<`.
format!("{new_lifetime}")
}
Some(AngleBrackets::Full) => {
// `Path<T>`: insert suggestion just after the `<`.
format!("{new_lifetime}, ")
}
None => {
if self.ident.span.is_empty() {
// The user wrote `&type` or `&mut type`.
// njn: does this get the mut case right? `&mut 'atype`?
format!("{new_lifetime} ")
} else {
// The user wrote `'a` or `'_`.
format!("{new_lifetime}")
}
}
};
(self.ident.span, s)
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/hir/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +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,
is_anon_in_path: None,
}
},
syntax,
Expand Down
26 changes: 6 additions & 20 deletions compiler/rustc_trait_selection/src/errors.rs
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, IsAnonInPath, Node};
use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, 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,19 +567,8 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {

impl<'v> Visitor<'v> for ImplicitLifetimeFinder {
fn visit_ty(&mut self, ty: &'v hir::Ty<'v, AmbigArg>) {
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 lifetime.ident.name == kw::UnderscoreLifetime
&& lifetime.ident.span.is_empty()
{
format!("{} ", self.suggestion_param_name)
} else {
self.suggestion_param_name.clone()
}
};
let make_suggestion =
|lifetime: &hir::Lifetime| lifetime.suggestion(&self.suggestion_param_name);
match ty.kind {
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => {
for segment in path.segments {
Expand All @@ -588,7 +577,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
matches!(
arg,
hir::GenericArg::Lifetime(lifetime)
if lifetime.is_anon_in_path == IsAnonInPath::Yes
if lifetime.is_anon_in_path.is_some()
)
}) {
self.suggestions.push((
Expand All @@ -607,18 +596,15 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
if let hir::GenericArg::Lifetime(lifetime) = arg
&& lifetime.is_anonymous()
{
self.suggestions.push((
lifetime.ident.span,
make_suggestion(lifetime),
));
self.suggestions.push(make_suggestion(lifetime));
}
}
}
}
}
}
hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => {
self.suggestions.push((lifetime.ident.span, make_suggestion(lifetime)));
self.suggestions.push(make_suggestion(lifetime));
}
_ => {}
}
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/suggestions/impl-trait-missing-lifetime-gated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ mod alone_in_path {
//~| ERROR missing lifetime specifier
}

mod alone_in_path2 {
trait Foo<'a> { fn next(&mut self) -> Option<&'a ()>; }

fn f(_: impl Foo<>) {}
//~^ ERROR anonymous lifetimes in `impl Trait` are unstable

fn g(mut x: impl Foo<>) -> Option<&()> { x.next() }
//~^ ERROR anonymous lifetimes in `impl Trait` are unstable
//~| ERROR missing lifetime specifier
}

mod in_path {
trait Foo<'a, T> { fn next(&mut self) -> Option<&'a T>; }

Expand Down
Loading
Loading