1
Fork 0

Rollup merge of #115882 - aliemjay:diag-name-region-1, r=compiler-errors

improve the suggestion of `generic_bound_failure`

- Fixes #115375
- suggest the bound in the correct scope: trait or impl header vs assoc item. See `tests/ui/suggestions/lifetimes/type-param-bound-scope.rs`
- don't suggest a lifetime name that conflicts with the other late-bound regions of the function:
```rust
type Inv<'a> = *mut &'a ();
fn check_bound<'a, T: 'a>(_: T, _: Inv<'a>) {}
fn test<'a, T>(_: &'a str, t: T, lt: Inv<'_>) { // suggests a new name `'a`
    check_bound(t, lt); //~ ERROR
}
```
This commit is contained in:
Matthias Krüger 2023-10-09 16:26:00 +02:00 committed by GitHub
commit 389747c41d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
82 changed files with 1181 additions and 746 deletions

View file

@ -59,20 +59,19 @@ use crate::traits::{
};
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_errors::{error_code, Applicability, DiagnosticBuilder, DiagnosticStyledString};
use rustc_errors::{pluralize, struct_span_err, Diagnostic, ErrorGuaranteed, IntoDiagnosticArg};
use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::Visitor;
use rustc_hir::lang_items::LangItem;
use rustc_hir::Node;
use rustc_middle::dep_graph::DepContext;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::relate::{self, RelateResult, TypeRelation};
use rustc_middle::ty::{
self, error::TypeError, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable,
TypeVisitable, TypeVisitableExt,
self, error::TypeError, IsSuggestable, List, Region, Ty, TyCtxt, TypeFoldable,
TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
};
use rustc_span::{sym, symbol::kw, BytePos, DesugaringKind, Pos, Span};
use rustc_target::spec::abi;
@ -2317,126 +2316,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
bound_kind: GenericKind<'tcx>,
sub: Region<'tcx>,
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
// Attempt to obtain the span of the parameter so we can
// suggest adding an explicit lifetime bound to it.
let generics = self.tcx.generics_of(generic_param_scope);
// type_param_span is (span, has_bounds)
let mut is_synthetic = false;
let mut ast_generics = None;
let type_param_span = match bound_kind {
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);
is_synthetic = type_param.kind.is_synthetic();
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 hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
ast_generics = self.tcx.hir().get_generics(hir_id.owner.def_id);
let bounds =
ast_generics.and_then(|g| g.bounds_span_for_suggestions(def_id));
// `sp` only covers `T`, change it so that it covers
// `T:` when appropriate
if let Some(span) = bounds {
(span, true)
} else {
let sp = self.tcx.def_span(def_id);
(sp.shrink_to_hi(), false)
}
})
} else {
None
}
}
_ => None,
};
let new_lt = {
let mut possible = (b'a'..=b'z').map(|c| format!("'{}", c as char));
let lts_names =
iter::successors(Some(generics), |g| g.parent.map(|p| self.tcx.generics_of(p)))
.flat_map(|g| &g.params)
.filter(|p| matches!(p.kind, ty::GenericParamDefKind::Lifetime))
.map(|p| p.name.as_str())
.collect::<Vec<_>>();
possible
.find(|candidate| !lts_names.contains(&&candidate[..]))
.unwrap_or("'lt".to_string())
};
let mut add_lt_suggs: Vec<Option<_>> = vec![];
if is_synthetic {
if let Some(ast_generics) = ast_generics {
let named_lifetime_param_exist = ast_generics.params.iter().any(|p| {
matches!(
p.kind,
hir::GenericParamKind::Lifetime { kind: hir::LifetimeParamKind::Explicit }
)
});
if named_lifetime_param_exist && let [param, ..] = ast_generics.params
{
add_lt_suggs.push(Some((
self.tcx.def_span(param.def_id).shrink_to_lo(),
format!("{new_lt}, "),
)));
} else {
add_lt_suggs
.push(Some((ast_generics.span.shrink_to_hi(), format!("<{new_lt}>"))));
}
}
} else {
if let [param, ..] = &generics.params[..] && let Some(def_id) = param.def_id.as_local()
{
add_lt_suggs
.push(Some((self.tcx.def_span(def_id).shrink_to_lo(), format!("{new_lt}, "))));
}
}
if let Some(ast_generics) = ast_generics {
for p in ast_generics.params {
if p.is_elided_lifetime() {
if self
.tcx
.sess
.source_map()
.span_to_prev_source(p.span.shrink_to_hi())
.ok()
.is_some_and(|s| *s.as_bytes().last().unwrap() == b'&')
{
add_lt_suggs
.push(Some(
(
p.span.shrink_to_hi(),
if let Ok(snip) = self.tcx.sess.source_map().span_to_next_source(p.span)
&& snip.starts_with(' ')
{
new_lt.to_string()
} else {
format!("{new_lt} ")
}
)
));
} else {
add_lt_suggs.push(Some((p.span.shrink_to_hi(), format!("<{new_lt}>"))));
}
}
}
}
let labeled_user_string = match bound_kind {
GenericKind::Param(ref p) => format!("the parameter type `{p}`"),
GenericKind::Alias(ref p) => match p.kind(self.tcx) {
ty::AliasKind::Projection | ty::AliasKind::Inherent => {
format!("the associated type `{p}`")
}
ty::AliasKind::Weak => format!("the type alias `{p}`"),
ty::AliasKind::Opaque => format!("the opaque type `{p}`"),
},
};
if let Some(SubregionOrigin::CompareImplItemObligation {
span,
impl_item_def_id,
@ -2451,209 +2330,218 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
);
}
fn binding_suggestion<'tcx, S: fmt::Display>(
err: &mut Diagnostic,
type_param_span: Option<(Span, bool)>,
bound_kind: GenericKind<'tcx>,
sub: S,
add_lt_suggs: Vec<Option<(Span, String)>>,
) {
let msg = "consider adding an explicit lifetime bound";
if let Some((sp, has_lifetimes)) = type_param_span {
let suggestion =
if has_lifetimes { format!(" + {sub}") } else { format!(": {sub}") };
let mut suggestions = vec![(sp, suggestion)];
for add_lt_sugg in add_lt_suggs.into_iter().flatten() {
suggestions.push(add_lt_sugg);
let labeled_user_string = match bound_kind {
GenericKind::Param(ref p) => format!("the parameter type `{p}`"),
GenericKind::Alias(ref p) => match p.kind(self.tcx) {
ty::AliasKind::Projection | ty::AliasKind::Inherent => {
format!("the associated type `{p}`")
}
err.multipart_suggestion_verbose(
format!("{msg}..."),
suggestions,
Applicability::MaybeIncorrect, // Issue #41966
);
} else {
let consider = format!("{msg} `{bound_kind}: {sub}`...");
err.help(consider);
ty::AliasKind::Weak => format!("the type alias `{p}`"),
ty::AliasKind::Opaque => format!("the opaque type `{p}`"),
},
};
let mut err = self.tcx.sess.struct_span_err_with_code(
span,
format!("{labeled_user_string} may not live long enough"),
match sub.kind() {
ty::ReEarlyBound(_) | ty::ReFree(_) if sub.has_name() => error_code!(E0309),
ty::ReStatic => error_code!(E0310),
_ => error_code!(E0311),
},
);
'_explain: {
let (description, span) = match sub.kind() {
ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReStatic => {
msg_span_from_named_region(self.tcx, sub, Some(span))
}
_ => (format!("lifetime `{sub}`"), Some(span)),
};
let prefix = format!("{labeled_user_string} must be valid for ");
label_msg_span(&mut err, &prefix, description, span, "...");
if let Some(origin) = origin {
self.note_region_origin(&mut err, &origin);
}
}
let new_binding_suggestion =
|err: &mut Diagnostic, type_param_span: Option<(Span, bool)>| {
let msg = "consider introducing an explicit lifetime bound";
if let Some((sp, has_lifetimes)) = type_param_span {
let suggestion =
if has_lifetimes { format!(" + {new_lt}") } else { format!(": {new_lt}") };
let mut sugg =
vec![(sp, suggestion), (span.shrink_to_hi(), format!(" + {new_lt}"))];
for lt in add_lt_suggs.clone().into_iter().flatten() {
sugg.push(lt);
sugg.rotate_right(1);
}
// `MaybeIncorrect` due to issue #41966.
err.multipart_suggestion(msg, sugg, Applicability::MaybeIncorrect);
'suggestion: {
let msg = "consider adding an explicit lifetime bound";
if (bound_kind, sub).has_infer_regions()
|| (bound_kind, sub).has_placeholders()
|| !bound_kind.is_suggestable(self.tcx, false)
{
let lt_name = sub.get_name_or_anon().to_string();
err.help(format!("{msg} `{bound_kind}: {lt_name}`..."));
break 'suggestion;
}
let mut generic_param_scope = generic_param_scope;
while self.tcx.def_kind(generic_param_scope) == DefKind::OpaqueTy {
generic_param_scope = self.tcx.local_parent(generic_param_scope);
}
// type_param_sugg_span is (span, has_bounds)
let (type_scope, type_param_sugg_span) = match bound_kind {
GenericKind::Param(ref param) => {
let generics = self.tcx.generics_of(generic_param_scope);
let def_id = generics.type_param(param, self.tcx).def_id.expect_local();
let scope = self.tcx.local_def_id_to_hir_id(def_id).owner.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 hir_generics = self.tcx.hir().get_generics(scope).unwrap();
let sugg_span = match hir_generics.bounds_span_for_suggestions(def_id) {
Some(span) => Some((span, true)),
// If `param` corresponds to `Self`, no usable suggestion span.
None if generics.has_self && param.index == 0 => None,
None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false)),
};
(scope, sugg_span)
}
_ => (generic_param_scope, None),
};
let suggestion_scope = {
let lifetime_scope = match sub.kind() {
ty::ReStatic => hir::def_id::CRATE_DEF_ID,
_ => match self.tcx.is_suitable_region(sub) {
Some(info) => info.def_id,
None => generic_param_scope,
},
};
match self.tcx.is_descendant_of(type_scope.into(), lifetime_scope.into()) {
true => type_scope,
false => lifetime_scope,
}
};
#[derive(Debug)]
enum SubOrigin<'hir> {
GAT(&'hir hir::Generics<'hir>),
Impl,
Trait,
Fn,
Unknown,
}
let sub_origin = 'origin: {
match *sub {
ty::ReEarlyBound(ty::EarlyBoundRegion { def_id, .. }) => {
let node = self.tcx.hir().get_if_local(def_id).unwrap();
match node {
Node::GenericParam(param) => {
for h in self.tcx.hir().parent_iter(param.hir_id) {
break 'origin match h.1 {
Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Type(..),
generics,
..
})
| Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Type(..),
generics,
..
}) => SubOrigin::GAT(generics),
Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(..),
..
})
| Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(..),
..
})
| Node::Item(hir::Item {
kind: hir::ItemKind::Fn(..), ..
}) => SubOrigin::Fn,
Node::Item(hir::Item {
kind: hir::ItemKind::Trait(..),
..
}) => SubOrigin::Trait,
Node::Item(hir::Item {
kind: hir::ItemKind::Impl(..), ..
}) => SubOrigin::Impl,
_ => continue,
};
}
}
_ => {}
}
}
_ => {}
}
SubOrigin::Unknown
};
debug!(?sub_origin);
let mut suggs = vec![];
let lt_name = self.suggest_name_region(sub, &mut suggs);
let mut err = match (*sub, sub_origin) {
// In the case of GATs, we have to be careful. If we a type parameter `T` on an impl,
// but a lifetime `'a` on an associated type, then we might need to suggest adding
// `where T: 'a`. Importantly, this is on the GAT span, not on the `T` declaration.
(ty::ReEarlyBound(ty::EarlyBoundRegion { name: _, .. }), SubOrigin::GAT(generics)) => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0309,
"{} may not live long enough",
labeled_user_string
);
let pred = format!("{bound_kind}: {sub}");
if let Some((sp, has_lifetimes)) = type_param_sugg_span
&& suggestion_scope == type_scope
{
let suggestion =
if has_lifetimes { format!(" + {lt_name}") } else { format!(": {lt_name}") };
suggs.push((sp, suggestion))
} else {
let generics = self.tcx.hir().get_generics(suggestion_scope).unwrap();
let pred = format!("{bound_kind}: {lt_name}");
let suggestion = format!("{} {}", generics.add_where_or_trailing_comma(), pred,);
err.span_suggestion(
generics.tail_span_for_predicate_suggestion(),
"consider adding a where clause",
suggestion,
Applicability::MaybeIncorrect,
);
err
}
(
ty::ReEarlyBound(ty::EarlyBoundRegion { name, .. })
| ty::ReFree(ty::FreeRegion { bound_region: ty::BrNamed(_, name), .. }),
_,
) if name != kw::UnderscoreLifetime => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0309,
"{} may not live long enough",
labeled_user_string
);
// Explicitly use the name instead of `sub`'s `Display` impl. The `Display` impl
// for the bound is not suitable for suggestions when `-Zverbose` is set because it
// uses `Debug` output, so we handle it specially here so that suggestions are
// always correct.
binding_suggestion(&mut err, type_param_span, bound_kind, name, vec![]);
err
suggs.push((generics.tail_span_for_predicate_suggestion(), suggestion))
}
(ty::ReStatic, _) => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0310,
"{} may not live long enough",
labeled_user_string
);
binding_suggestion(&mut err, type_param_span, bound_kind, "'static", vec![]);
err
}
err.multipart_suggestion_verbose(
format!("{msg}"),
suggs,
Applicability::MaybeIncorrect, // Issue #41966
);
}
_ => {
// If not, be less specific.
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0311,
"{} may not live long enough",
labeled_user_string
);
note_and_explain_region(
self.tcx,
&mut err,
&format!("{labeled_user_string} must be valid for "),
sub,
"...",
None,
);
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(..) | ty::Alias(ty::Opaque, ..) => {
new_binding_suggestion(&mut err, type_param_span);
err
}
pub fn suggest_name_region(
&self,
lifetime: Region<'tcx>,
add_lt_suggs: &mut Vec<(Span, String)>,
) -> String {
struct LifetimeReplaceVisitor<'tcx, 'a> {
tcx: TyCtxt<'tcx>,
needle: hir::LifetimeName,
new_lt: &'a str,
add_lt_suggs: &'a mut Vec<(Span, String)>,
}
impl<'hir, 'tcx> hir::intravisit::Visitor<'hir> for LifetimeReplaceVisitor<'tcx, '_> {
fn visit_lifetime(&mut self, lt: &'hir hir::Lifetime) {
if lt.res == self.needle {
let (pos, span) = lt.suggestion_position();
let new_lt = &self.new_lt;
let sugg = match pos {
hir::LifetimeSuggestionPosition::Normal => format!("{new_lt}"),
hir::LifetimeSuggestionPosition::Ampersand => format!("{new_lt} "),
hir::LifetimeSuggestionPosition::ElidedPath => format!("<{new_lt}>"),
hir::LifetimeSuggestionPosition::ElidedPathArgument => {
format!("{new_lt}, ")
}
_ => {
binding_suggestion(
&mut err,
type_param_span,
bound_kind,
new_lt,
add_lt_suggs,
);
}
}
hir::LifetimeSuggestionPosition::ObjectDefault => format!("+ {new_lt}"),
};
self.add_lt_suggs.push((span, sugg));
}
err
}
fn visit_ty(&mut self, ty: &'hir hir::Ty<'hir>) {
let hir::TyKind::OpaqueDef(item_id, _, _) = ty.kind else {
return hir::intravisit::walk_ty(self, ty);
};
let opaque_ty = self.tcx.hir().item(item_id).expect_opaque_ty();
if let Some(&(_, b)) =
opaque_ty.lifetime_mapping.iter().find(|&(a, _)| a.res == self.needle)
{
let prev_needle =
std::mem::replace(&mut self.needle, hir::LifetimeName::Param(b));
for bound in opaque_ty.bounds {
self.visit_param_bound(bound);
}
self.needle = prev_needle;
}
}
}
let (lifetime_def_id, lifetime_scope) = match self.tcx.is_suitable_region(lifetime) {
Some(info) if !lifetime.has_name() => {
(info.boundregion.get_id().unwrap().expect_local(), info.def_id)
}
_ => return lifetime.get_name_or_anon().to_string(),
};
if let Some(origin) = origin {
self.note_region_origin(&mut err, &origin);
let new_lt = {
let generics = self.tcx.generics_of(lifetime_scope);
let mut used_names =
iter::successors(Some(generics), |g| g.parent.map(|p| self.tcx.generics_of(p)))
.flat_map(|g| &g.params)
.filter(|p| matches!(p.kind, ty::GenericParamDefKind::Lifetime))
.map(|p| p.name)
.collect::<Vec<_>>();
if let Some(hir_id) = self.tcx.opt_local_def_id_to_hir_id(lifetime_scope) {
// consider late-bound lifetimes ...
used_names.extend(self.tcx.late_bound_vars(hir_id).into_iter().filter_map(|p| {
match p {
ty::BoundVariableKind::Region(lt) => lt.get_name(),
_ => None,
}
}))
}
(b'a'..=b'z')
.map(|c| format!("'{}", c as char))
.find(|candidate| !used_names.iter().any(|e| e.as_str() == candidate))
.unwrap_or("'lt".to_string())
};
let mut visitor = LifetimeReplaceVisitor {
tcx: self.tcx,
needle: hir::LifetimeName::Param(lifetime_def_id),
add_lt_suggs,
new_lt: &new_lt,
};
match self.tcx.hir().expect_owner(lifetime_scope) {
hir::OwnerNode::Item(i) => visitor.visit_item(i),
hir::OwnerNode::ForeignItem(i) => visitor.visit_foreign_item(i),
hir::OwnerNode::ImplItem(i) => visitor.visit_impl_item(i),
hir::OwnerNode::TraitItem(i) => visitor.visit_trait_item(i),
hir::OwnerNode::Crate(_) => bug!("OwnerNode::Crate doesn't not have generics"),
}
err
let ast_generics = self.tcx.hir().get_generics(lifetime_scope).unwrap();
let sugg = ast_generics
.span_for_lifetime_suggestion()
.map(|span| (span, format!("{new_lt}, ")))
.unwrap_or_else(|| (ast_generics.span, format!("<{new_lt}>")));
add_lt_suggs.push(sugg);
new_lt
}
fn report_sub_sup_conflict(

View file

@ -1057,16 +1057,21 @@ impl<'tcx> TyCtxt<'tcx> {
}
/// Returns the `DefId` and the `BoundRegionKind` corresponding to the given region.
pub fn is_suitable_region(self, region: Region<'tcx>) -> Option<FreeRegionInfo> {
let (suitable_region_binding_scope, bound_region) = match *region {
ty::ReFree(ref free_region) => {
(free_region.scope.expect_local(), free_region.bound_region)
pub fn is_suitable_region(self, mut region: Region<'tcx>) -> Option<FreeRegionInfo> {
let (suitable_region_binding_scope, bound_region) = loop {
let def_id = match region.kind() {
ty::ReFree(fr) => fr.bound_region.get_id()?.as_local()?,
ty::ReEarlyBound(ebr) => ebr.def_id.expect_local(),
_ => return None, // not a free region
};
let scope = self.local_parent(def_id);
if self.def_kind(scope) == DefKind::OpaqueTy {
// Lifetime params of opaque types are synthetic and thus irrelevant to
// diagnostics. Map them back to their origin!
region = self.map_rpit_lifetime_to_fn_lifetime(def_id);
continue;
}
ty::ReEarlyBound(ref ebr) => (
self.local_parent(ebr.def_id.expect_local()),
ty::BoundRegionKind::BrNamed(ebr.def_id, ebr.name),
),
_ => return None, // not a free region
break (scope, ty::BrNamed(def_id.into(), self.item_name(def_id.into())));
};
let is_impl_item = match self.hir().find_by_def_id(suitable_region_binding_scope) {
@ -1074,7 +1079,7 @@ impl<'tcx> TyCtxt<'tcx> {
Some(Node::ImplItem(..)) => {
self.is_bound_region_in_impl_item(suitable_region_binding_scope)
}
_ => return None,
_ => false,
};
Some(FreeRegionInfo {