Skip to content

Account for missing lifetime in opaque and trait object return types #72543

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 10 commits into from
May 31, 2020
Merged

Account for missing lifetime in opaque and trait object return types #72543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f49ebbb
Account for missing lifetime in opaque return type
estebank May 24, 2020
99d9ccd
Improve output of argument anonymous borrow missing annotation involv…
estebank May 24, 2020
a724d9a
Fix NLL output
estebank May 24, 2020
65f492b
Account for returned `dyn Trait` evaluating to `'static` lifetime
estebank May 26, 2020
731ea85
review comment: tweak wording and account for span overlap
estebank May 27, 2020
1d9472b
Update nll tests
estebank May 27, 2020
8f7ee34
Tweak type parameter errors to reduce verbosity
estebank May 27, 2020
224ad32
Account for enclosing item when suggesting new lifetime name
estebank May 27, 2020
6dcd744
Consider all possible one letter lifetimes in suggestion
estebank May 27, 2020
83f6f22
Tweak wording and spans of `'static` `dyn Trait`/`impl Trait` require…
estebank May 28, 2020
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
170 changes: 124 additions & 46 deletions src/librustc_infer/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use rustc_errors::{pluralize, struct_span_err};
use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::Node;
use rustc_hir::{Item, ItemKind, Node};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::{
self,
Expand Down Expand Up @@ -1682,49 +1682,92 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
bound_kind: GenericKind<'tcx>,
sub: Region<'tcx>,
) -> DiagnosticBuilder<'a> {
let hir = &self.tcx.hir();
// Attempt to obtain the span of the parameter so we can
// suggest adding an explicit lifetime bound to it.
let type_param_span = match (self.in_progress_tables, bound_kind) {
(Some(ref table), GenericKind::Param(ref param)) => {
let table_owner = table.borrow().hir_owner;
table_owner.and_then(|table_owner| {
let generics = self.tcx.generics_of(table_owner.to_def_id());
// Account for the case where `param` corresponds to `Self`,
// which doesn't have the expected type argument.
if !(generics.has_self && param.index == 0) {
let type_param = generics.type_param(param, self.tcx);
let hir = &self.tcx.hir();
type_param.def_id.as_local().map(|def_id| {
// Get the `hir::Param` to verify whether it already has any bounds.
// We do this to avoid suggesting code that ends up as `T: 'a'b`,
// instead we suggest `T: 'a + 'b` in that case.
let id = hir.as_local_hir_id(def_id);
let mut has_bounds = false;
if let Node::GenericParam(param) = hir.get(id) {
has_bounds = !param.bounds.is_empty();
}
let sp = hir.span(id);
// `sp` only covers `T`, change it so that it covers
// `T:` when appropriate
let is_impl_trait = bound_kind.to_string().starts_with("impl ");
let sp = if has_bounds && !is_impl_trait {
sp.to(self
.tcx
.sess
.source_map()
.next_point(self.tcx.sess.source_map().next_point(sp)))
} else {
sp
};
(sp, has_bounds, is_impl_trait)
})
let generics =
self.in_progress_tables.and_then(|table| table.borrow().hir_owner).map(|table_owner| {
let hir_id = hir.as_local_hir_id(table_owner);
let parent_id = hir.get_parent_item(hir_id);
(
// Parent item could be a `mod`, so we check the HIR before calling:
if let Some(Node::Item(Item {
kind: ItemKind::Trait(..) | ItemKind::Impl { .. },
..
})) = hir.find(parent_id)
{
Some(self.tcx.generics_of(hir.local_def_id(parent_id).to_def_id()))
} else {
None
}
})
},
self.tcx.generics_of(table_owner.to_def_id()),
)
});
let type_param_span = match (generics, bound_kind) {
(Some((_, ref generics)), GenericKind::Param(ref param)) => {
// Account for the case where `param` corresponds to `Self`,
// which doesn't have the expected type argument.
if !(generics.has_self && param.index == 0) {
let type_param = generics.type_param(param, self.tcx);
type_param.def_id.as_local().map(|def_id| {
// Get the `hir::Param` to verify whether it already has any bounds.
// We do this to avoid suggesting code that ends up as `T: 'a'b`,
// instead we suggest `T: 'a + 'b` in that case.
let id = hir.as_local_hir_id(def_id);
let mut has_bounds = false;
if let Node::GenericParam(param) = hir.get(id) {
has_bounds = !param.bounds.is_empty();
}
let sp = hir.span(id);
// `sp` only covers `T`, change it so that it covers
// `T:` when appropriate
let is_impl_trait = bound_kind.to_string().starts_with("impl ");
let sp = if has_bounds && !is_impl_trait {
sp.to(self
.tcx
.sess
.source_map()
.next_point(self.tcx.sess.source_map().next_point(sp)))
} else {
sp
};
(sp, has_bounds, is_impl_trait)
})
} else {
None
}
}
_ => None,
};
let new_lt = generics
.as_ref()
.and_then(|(parent_g, g)| {
let possible: Vec<_> = (b'a'..=b'z').map(|c| format!("'{}", c as char)).collect();
let mut lts_names = g
.params
.iter()
.filter(|p| matches!(p.kind, ty::GenericParamDefKind::Lifetime))
.map(|p| p.name.as_str())
.collect::<Vec<_>>();
if let Some(g) = parent_g {
lts_names.extend(
g.params
.iter()
.filter(|p| matches!(p.kind, ty::GenericParamDefKind::Lifetime))
.map(|p| p.name.as_str()),
);
}
let lts = lts_names.iter().map(|s| -> &str { &*s }).collect::<Vec<_>>();
possible.into_iter().find(|candidate| !lts.contains(&candidate.as_str()))
})
.unwrap_or("'lt".to_string());
let add_lt_sugg = generics
.as_ref()
.and_then(|(_, g)| g.params.first())
.and_then(|param| param.def_id.as_local())
.map(|def_id| {
(hir.span(hir.as_local_hir_id(def_id)).shrink_to_lo(), format!("{}, ", new_lt))
});

let labeled_user_string = match bound_kind {
GenericKind::Param(ref p) => format!("the parameter type `{}`", p),
Expand Down Expand Up @@ -1781,6 +1824,29 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

let new_binding_suggestion =
|err: &mut DiagnosticBuilder<'tcx>,
type_param_span: Option<(Span, bool, bool)>,
bound_kind: GenericKind<'tcx>| {
let msg = "consider introducing an explicit lifetime bound";
if let Some((sp, has_lifetimes, is_impl_trait)) = type_param_span {
let suggestion = if is_impl_trait {
(sp.shrink_to_hi(), format!(" + {}", new_lt))
} else {
let tail = if has_lifetimes { " +" } else { "" };
(sp, format!("{}: {}{}", bound_kind, new_lt, tail))
};
let mut sugg =
vec![suggestion, (span.shrink_to_hi(), format!(" + {}", new_lt))];
if let Some(lt) = add_lt_sugg {
sugg.push(lt);
sugg.rotate_right(1);
}
// `MaybeIncorrect` due to issue #41966.
err.multipart_suggestion(msg, sugg, Applicability::MaybeIncorrect);
}
};

let mut err = match *sub {
ty::ReEarlyBound(ty::EarlyBoundRegion { name, .. })
| ty::ReFree(ty::FreeRegion { bound_region: ty::BrNamed(_, name), .. }) => {
Expand Down Expand Up @@ -1822,17 +1888,28 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
"{} may not live long enough",
labeled_user_string
);
err.help(&format!(
"consider adding an explicit lifetime bound for `{}`",
bound_kind
));
note_and_explain_region(
self.tcx,
&mut err,
&format!("{} must be valid for ", labeled_user_string),
sub,
"...",
);
if let Some(infer::RelateParamBound(_, t)) = origin {
let t = self.resolve_vars_if_possible(&t);
match t.kind {
// We've got:
// fn get_later<G, T>(g: G, dest: &mut T) -> impl FnOnce() + '_
// suggest:
// fn get_later<'a, G: 'a, T>(g: G, dest: &mut T) -> impl FnOnce() + '_ + 'a
ty::Closure(_, _substs) | ty::Opaque(_, _substs) => {
new_binding_suggestion(&mut err, type_param_span, bound_kind);
}
_ => {
binding_suggestion(&mut err, type_param_span, bound_kind, new_lt);
}
}
}
err
}
};
Expand Down Expand Up @@ -1861,14 +1938,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
"...",
);

debug!("report_sub_sup_conflict: var_origin={:?}", var_origin);
debug!("report_sub_sup_conflict: sub_region={:?}", sub_region);
debug!("report_sub_sup_conflict: sub_origin={:?}", sub_origin);
debug!("report_sub_sup_conflict: sup_region={:?}", sup_region);
debug!("report_sub_sup_conflict: sup_origin={:?}", sup_origin);

if let (&infer::Subtype(ref sup_trace), &infer::Subtype(ref sub_trace)) =
(&sup_origin, &sub_origin)
{
debug!("report_sub_sup_conflict: var_origin={:?}", var_origin);
debug!("report_sub_sup_conflict: sub_region={:?}", sub_region);
debug!("report_sub_sup_conflict: sub_origin={:?}", sub_origin);
debug!("report_sub_sup_conflict: sup_region={:?}", sup_region);
debug!("report_sub_sup_conflict: sup_origin={:?}", sup_origin);
debug!("report_sub_sup_conflict: sup_trace={:?}", sup_trace);
debug!("report_sub_sup_conflict: sub_trace={:?}", sub_trace);
debug!("report_sub_sup_conflict: sup_trace.values={:?}", sup_trace.values);
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_infer/infer/error_reporting/nice_region_error/different_lifetimes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,14 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
(Some(ret_span), _) => (
ty_sub.span,
ret_span,
"this parameter and the return type are declared \
with different lifetimes..."
"this parameter and the return type are declared with different lifetimes..."
.to_owned(),
format!("...but data{} is returned here", span_label_var1),
),
(_, Some(ret_span)) => (
ty_sup.span,
ret_span,
"this parameter and the return type are declared \
with different lifetimes..."
"this parameter and the return type are declared with different lifetimes..."
.to_owned(),
format!("...but data{} is returned here", span_label_var1),
),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/nice_region_error/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ impl<'cx, 'tcx> NiceRegionError<'cx, 'tcx> {
diag.emit();
ErrorReported
})
.or_else(|| self.try_report_impl_not_conforming_to_trait())
.or_else(|| self.try_report_anon_anon_conflict())
.or_else(|| self.try_report_static_impl_trait())
.or_else(|| self.try_report_impl_not_conforming_to_trait())
}

pub fn regions(&self) -> Option<(Span, ty::Region<'tcx>, ty::Region<'tcx>)> {
Expand Down
37 changes: 15 additions & 22 deletions src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
// where the anonymous region appears (there must always be one; we
// only introduced anonymous regions in parameters) as well as a
// version new_ty of its type where the anonymous region is replaced
// with the named one.//scope_def_id
let (named, anon, anon_param_info, region_info) = if self.is_named_region(sub)
// with the named one.
let (named, anon, anon_param_info, region_info) = if sub.has_name()
&& self.tcx().is_suitable_region(sup).is_some()
&& self.find_param_with_region(sup, sub).is_some()
{
Expand All @@ -32,7 +32,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
self.find_param_with_region(sup, sub).unwrap(),
self.tcx().is_suitable_region(sup).unwrap(),
)
} else if self.is_named_region(sup)
} else if sup.has_name()
&& self.tcx().is_suitable_region(sub).is_some()
&& self.find_param_with_region(sub, sup).is_some()
{
Expand Down Expand Up @@ -74,15 +74,21 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}

if let Some((_, fndecl)) = self.find_anon_type(anon, &br) {
if self.is_return_type_anon(scope_def_id, br, fndecl).is_some()
|| self.is_self_anon(is_first, scope_def_id)
{
let is_self_anon = self.is_self_anon(is_first, scope_def_id);
if is_self_anon {
return None;
}

if let FnRetTy::Return(ty) = &fndecl.output {
if let (TyKind::Def(_, _), ty::ReStatic) = (&ty.kind, sub) {
// This is an impl Trait return that evaluates de need of 'static.
// We handle this case better in `static_impl_trait`.
let mut v = ty::TraitObjectVisitor(vec![]);
rustc_hir::intravisit::walk_ty(&mut v, ty);

debug!("try_report_named_anon_conflict: ret ty {:?}", ty);
if sub == &ty::ReStatic && (matches!(ty.kind, TyKind::Def(_, _)) || v.0.len() == 1)
{
debug!("try_report_named_anon_conflict: impl Trait + 'static");
// This is an `impl Trait` or `dyn Trait` return that evaluates de need of
// `'static`. We handle this case better in `static_impl_trait`.
return None;
}
}
Expand Down Expand Up @@ -114,17 +120,4 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {

Some(diag)
}

// This method returns whether the given Region is Named
pub(super) fn is_named_region(&self, region: ty::Region<'tcx>) -> bool {
match *region {
ty::ReStatic => true,
ty::ReFree(ref free_region) => match free_region.bound_region {
ty::BrNamed(..) => true,
_ => false,
},
ty::ReEarlyBound(ebr) => ebr.has_name(),
_ => false,
}
}
}
Loading