bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #90446 - cjgillot:late-elided, r=jackh726

Browse source 
Lint elided lifetimes in path during lifetime resolution.

The lifetime elision lint is known to be brittle and can be redundant with later lifetime resolution errors. This PR aims to remove the redundancy by performing the lint after lifetime resolution.

This PR proposes to carry the information that an elision should be linted against by using a special `LifetimeName`. I am not certain this is the best solution, but it is certainly the easiest.

Fixes https://github.com/rust-lang/rust/issues/60199
Fixes https://github.com/rust-lang/rust/issues/55768
Fixes https://github.com/rust-lang/rust/issues/63110
Fixes https://github.com/rust-lang/rust/issues/71957
This commit is contained in:
bors 2021-12-01 23:22:43 +00:00
commit 76938d64a4
13 changed files with 401 additions and 241 deletions
Download patch file Download diff file Expand all files Collapse all files

115
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -1871,6 +1871,117 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
err.emit();
}
/// Returns whether to add `'static` lifetime to the suggested lifetime list.
crate fn report_elision_failure(
&mut self,
db: &mut DiagnosticBuilder<'_>,
params: &[ElisionFailureInfo],
) -> bool {
let mut m = String::new();
let len = params.len();
let elided_params: Vec<_> =
params.iter().cloned().filter(|info| info.lifetime_count > 0).collect();
let elided_len = elided_params.len();
for (i, info) in elided_params.into_iter().enumerate() {
let ElisionFailureInfo { parent, index, lifetime_count: n, have_bound_regions, span } =
info;
db.span_label(span, "");
let help_name = if let Some(ident) =
parent.and_then(|body| self.tcx.hir().body(body).params[index].pat.simple_ident())
{
format!("`{}`", ident)
} else {
format!("argument {}", index + 1)
};
m.push_str(
&(if n == 1 {
help_name
} else {
format!(
"one of {}'s {} {}lifetimes",
help_name,
n,
if have_bound_regions { "free " } else { "" }
)
})[..],
);
if elided_len == 2 && i == 0 {
m.push_str(" or ");
} else if i + 2 == elided_len {
m.push_str(", or ");
} else if i != elided_len - 1 {
m.push_str(", ");
}
}
if len == 0 {
db.help(
"this function's return type contains a borrowed value, \
but there is no value for it to be borrowed from",
);
true
} else if elided_len == 0 {
db.help(
"this function's return type contains a borrowed value with \
an elided lifetime, but the lifetime cannot be derived from \
the arguments",
);
true
} else if elided_len == 1 {
db.help(&format!(
"this function's return type contains a borrowed value, \
but the signature does not say which {} it is borrowed from",
m
));
false
} else {
db.help(&format!(
"this function's return type contains a borrowed value, \
but the signature does not say whether it is borrowed from {}",
m
));
false
}
}
crate fn report_elided_lifetime_in_ty(&self, lifetime_refs: &[&hir::Lifetime]) {
let Some(missing_lifetime) = lifetime_refs.iter().find(|lt| {
lt.name == hir::LifetimeName::Implicit(true)
}) else { return };
let mut spans: Vec<_> = lifetime_refs.iter().map(|lt| lt.span).collect();
spans.sort();
let mut spans_dedup = spans.clone();
spans_dedup.dedup();
let spans_with_counts: Vec<_> = spans_dedup
.into_iter()
.map(|sp| (sp, spans.iter().filter(|nsp| *nsp == &sp).count()))
.collect();
self.tcx.struct_span_lint_hir(
rustc_session::lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
missing_lifetime.hir_id,
spans,
|lint| {
let mut db = lint.build("hidden lifetime parameters in types are deprecated");
self.add_missing_lifetime_specifiers_label(
&mut db,
spans_with_counts,
&FxHashSet::from_iter([kw::UnderscoreLifetime]),
Vec::new(),
&[],
);
db.emit()
},
);
}
// FIXME(const_generics): This patches over an ICE caused by non-'static lifetimes in const
// generics. We are disallowing this until we can decide on how we want to handle non-'static
// lifetimes in const generics. See issue #74052 for discussion.
@ -2297,7 +2408,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
);
let is_allowed_lifetime = matches!(
lifetime_ref.name,
hir::LifetimeName::Implicit | hir::LifetimeName::Static | hir::LifetimeName::Underscore
hir::LifetimeName::Implicit(_)
| hir::LifetimeName::Static
| hir::LifetimeName::Underscore
);
if !self.tcx.lazy_normalization() && is_anon_const && !is_allowed_lifetime {

182
compiler/rustc_resolve/src/late/lifetimes.rs
View file

@ -357,11 +357,11 @@ enum Elide {
#[derive(Clone, Debug)]
crate struct ElisionFailureInfo {
/// Where we can find the argument pattern.
parent: Option<hir::BodyId>,
crate parent: Option<hir::BodyId>,
/// The index of the argument in the original definition.
index: usize,
lifetime_count: usize,
have_bound_regions: bool,
crate index: usize,
crate lifetime_count: usize,
crate have_bound_regions: bool,
crate span: Span,
}
@ -923,7 +923,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
}
});
match lifetime.name {
LifetimeName::Implicit => {
LifetimeName::Implicit(_) => {
// For types like `dyn Foo`, we should
// generate a special form of elided.
span_bug!(ty.span, "object-lifetime-default expected, not implicit",);
@ -3057,9 +3057,9 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
let error = loop {
match *scope {
// Do not assign any resolution, it will be inferred.
Scope::Body { .. } => return,
Scope::Body { .. } => break Ok(()),
Scope::Root => break None,
Scope::Root => break Err(None),
Scope::Binder { s, ref lifetimes, scope_type, .. } => {
// collect named lifetimes for suggestions
@ -3076,50 +3076,54 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
scope = s;
}
Scope::Elision { ref elide, ref s, .. } => {
let lifetime = match *elide {
Elide::FreshLateAnon(named_late_bound_vars, ref counter) => {
for lifetime_ref in lifetime_refs {
let lifetime = Region::late_anon(named_late_bound_vars, counter)
.shifted(late_depth);
Scope::Elision {
elide: Elide::FreshLateAnon(named_late_bound_vars, ref counter),
..
} => {
for lifetime_ref in lifetime_refs {
let lifetime =
Region::late_anon(named_late_bound_vars, counter).shifted(late_depth);
self.insert_lifetime(lifetime_ref, lifetime);
}
return;
}
Elide::Exact(l) => l.shifted(late_depth),
Elide::Error(ref e) => {
let mut scope = s;
loop {
match scope {
Scope::Binder { ref lifetimes, s, .. } => {
// Collect named lifetimes for suggestions.
for name in lifetimes.keys() {
if let hir::ParamName::Plain(name) = name {
lifetime_names.insert(name.name);
lifetime_spans.push(name.span);
}
}
scope = s;
}
Scope::ObjectLifetimeDefault { ref s, .. }
| Scope::Elision { ref s, .. }
| Scope::TraitRefBoundary { ref s, .. } => {
scope = s;
}
_ => break,
}
}
break Some(&e[..]);
}
Elide::Forbid => break None,
};
self.insert_lifetime(lifetime_ref, lifetime);
}
break Ok(());
}
Scope::Elision { elide: Elide::Exact(l), .. } => {
let lifetime = l.shifted(late_depth);
for lifetime_ref in lifetime_refs {
self.insert_lifetime(lifetime_ref, lifetime);
}
return;
break Ok(());
}
Scope::Elision { elide: Elide::Error(ref e), ref s, .. } => {
let mut scope = s;
loop {
match scope {
Scope::Binder { ref lifetimes, s, .. } => {
// Collect named lifetimes for suggestions.
for name in lifetimes.keys() {
if let hir::ParamName::Plain(name) = name {
lifetime_names.insert(name.name);
lifetime_spans.push(name.span);
}
}
scope = s;
}
Scope::ObjectLifetimeDefault { ref s, .. }
| Scope::Elision { ref s, .. }
| Scope::TraitRefBoundary { ref s, .. } => {
scope = s;
}
_ => break,
}
}
break Err(Some(&e[..]));
}
Scope::Elision { elide: Elide::Forbid, .. } => break Err(None),
Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. } => {
@ -3128,6 +3132,14 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
};
let error = match error {
Ok(()) => {
self.report_elided_lifetime_in_ty(lifetime_refs);
return;
}
Err(error) => error,
};
// If we specifically need the `scope_for_path` map, then we're in the
// diagnostic pass and we don't want to emit more errors.
if self.map.scope_for_path.is_some() {
@ -3166,84 +3178,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
err.emit();
}
fn report_elision_failure(
&mut self,
db: &mut DiagnosticBuilder<'_>,
params: &[ElisionFailureInfo],
) -> bool /* add `'static` lifetime to lifetime list */ {
let mut m = String::new();
let len = params.len();
let elided_params: Vec<_> =
params.iter().cloned().filter(|info| info.lifetime_count > 0).collect();
let elided_len = elided_params.len();
for (i, info) in elided_params.into_iter().enumerate() {
let ElisionFailureInfo { parent, index, lifetime_count: n, have_bound_regions, span } =
info;
db.span_label(span, "");
let help_name = if let Some(ident) =
parent.and_then(|body| self.tcx.hir().body(body).params[index].pat.simple_ident())
{
format!("`{}`", ident)
} else {
format!("argument {}", index + 1)
};
m.push_str(
&(if n == 1 {
help_name
} else {
format!(
"one of {}'s {} {}lifetimes",
help_name,
n,
if have_bound_regions { "free " } else { "" }
)
})[..],
);
if elided_len == 2 && i == 0 {
m.push_str(" or ");
} else if i + 2 == elided_len {
m.push_str(", or ");
} else if i != elided_len - 1 {
m.push_str(", ");
}
}
if len == 0 {
db.help(
"this function's return type contains a borrowed value, \
but there is no value for it to be borrowed from",
);
true
} else if elided_len == 0 {
db.help(
"this function's return type contains a borrowed value with \
an elided lifetime, but the lifetime cannot be derived from \
the arguments",
);
true
} else if elided_len == 1 {
db.help(&format!(
"this function's return type contains a borrowed value, \
but the signature does not say which {} it is borrowed from",
m
));
false
} else {
db.help(&format!(
"this function's return type contains a borrowed value, \
but the signature does not say whether it is borrowed from {}",
m
));
false
}
}
fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
debug!("resolve_object_lifetime_default(lifetime_ref={:?})", lifetime_ref);
let mut late_depth = 0;
@ -3348,7 +3282,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
))
.emit();
}
hir::LifetimeName::Param(_) | hir::LifetimeName::Implicit => {
hir::LifetimeName::Param(_) | hir::LifetimeName::Implicit(_) => {
self.resolve_lifetime_ref(lt);
}
hir::LifetimeName::ImplicitObjectLifetimeDefault => {