Skip to content

Commit d90a4b8

Browse files
JohnTitorJohnTitor
authored
Rollup merge of #75372 - estebank:lt-sugg-in-type, r=lcnr
Fix suggestion to use lifetime in type and in assoc const _Do not merge until #75363 has landed, as it has the test case for this._ * Account for associated types * Associated `const`s can't have generics (fix #74264) * Do not suggest duplicate lifetimes and suggest `for<'a>` more (fix #72404)
2 parents d0414b5 + 6a3deb0 commit d90a4b8

10 files changed

+360
-38
lines changed

‎src/librustc_resolve/late/diagnostics.rs

+137-24
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
1616
use rustc_hir::PrimTy;
1717
use rustc_session::config::nightly_options;
1818
use rustc_span::hygiene::MacroKind;
19-
use rustc_span::symbol::{kw, sym, Ident};
20-
use rustc_span::{BytePos, Span};
19+
use rustc_span::symbol::{kw, sym, Ident, Symbol};
20+
use rustc_span::{BytePos, Span, DUMMY_SP};
2121

2222
use log::debug;
2323

@@ -33,6 +33,7 @@ enum AssocSuggestion {
3333
crate enum MissingLifetimeSpot<'tcx> {
3434
Generics(&'tcx hir::Generics<'tcx>),
3535
HigherRanked { span: Span, span_type: ForLifetimeSpanType },
36+
Static,
3637
}
3738

3839
crate enum ForLifetimeSpanType {
@@ -1195,6 +1196,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
11951196
https://doc.rust-lang.org/nomicon/hrtb.html",
11961197
);
11971198
}
1199+
_ => {}
11981200
}
11991201
}
12001202
if nightly_options::is_nightly_build()
@@ -1253,7 +1255,8 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
12531255
err: &mut DiagnosticBuilder<'_>,
12541256
span: Span,
12551257
count: usize,
1256-
lifetime_names: &FxHashSet<Ident>,
1258+
lifetime_names: &FxHashSet<Symbol>,
1259+
lifetime_spans: Vec<Span>,
12571260
params: &[ElisionFailureInfo],
12581261
) {
12591262
let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok();
@@ -1267,11 +1270,60 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
12671270
),
12681271
);
12691272

1270-
let suggest_existing = |err: &mut DiagnosticBuilder<'_>, sugg| {
1273+
let suggest_existing = |err: &mut DiagnosticBuilder<'_>,
1274+
name: &str,
1275+
formatter: &dyn Fn(&str) -> String| {
1276+
if let Some(MissingLifetimeSpot::HigherRanked { span: for_span, span_type }) =
1277+
self.missing_named_lifetime_spots.iter().rev().next()
1278+
{
1279+
// When we have `struct S<'a>(&'a dyn Fn(&X) -> &X);` we want to not only suggest
1280+
// using `'a`, but also introduce the concept of HRLTs by suggesting
1281+
// `struct S<'a>(&'a dyn for<'b> Fn(&X) -> &'b X);`. (#72404)
1282+
let mut introduce_suggestion = vec![];
1283+
1284+
let a_to_z_repeat_n = |n| {
1285+
(b'a'..=b'z').map(move |c| {
1286+
let mut s = '\''.to_string();
1287+
s.extend(std::iter::repeat(char::from(c)).take(n));
1288+
s
1289+
})
1290+
};
1291+
1292+
// If all single char lifetime names are present, we wrap around and double the chars.
1293+
let lt_name = (1..)
1294+
.flat_map(a_to_z_repeat_n)
1295+
.find(|lt| !lifetime_names.contains(&Symbol::intern(&lt)))
1296+
.unwrap();
1297+
let msg = format!(
1298+
"consider making the {} lifetime-generic with a new `{}` lifetime",
1299+
span_type.descr(),
1300+
lt_name,
1301+
);
1302+
err.note(
1303+
"for more information on higher-ranked polymorphism, visit \
1304+
https://doc.rust-lang.org/nomicon/hrtb.html",
1305+
);
1306+
let for_sugg = span_type.suggestion(&lt_name);
1307+
for param in params {
1308+
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span) {
1309+
if snippet.starts_with('&') && !snippet.starts_with("&'") {
1310+
introduce_suggestion
1311+
.push((param.span, format!("&{} {}", lt_name, &snippet[1..])));
1312+
} else if snippet.starts_with("&'_ ") {
1313+
introduce_suggestion
1314+
.push((param.span, format!("&{} {}", lt_name, &snippet[4..])));
1315+
}
1316+
}
1317+
}
1318+
introduce_suggestion.push((*for_span, for_sugg.to_string()));
1319+
introduce_suggestion.push((span, formatter(&lt_name)));
1320+
err.multipart_suggestion(&msg, introduce_suggestion, Applicability::MaybeIncorrect);
1321+
}
1322+
12711323
err.span_suggestion_verbose(
12721324
span,
12731325
&format!("consider using the `{}` lifetime", lifetime_names.iter().next().unwrap()),
1274-
sugg,
1326+
formatter(name),
12751327
Applicability::MaybeIncorrect,
12761328
);
12771329
};
@@ -1282,6 +1334,15 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
12821334
let should_break;
12831335
introduce_suggestion.push(match missing {
12841336
MissingLifetimeSpot::Generics(generics) => {
1337+
if generics.span == DUMMY_SP {
1338+
// Account for malformed generics in the HIR. This shouldn't happen,
1339+
// but if we make a mistake elsewhere, mainly by keeping something in
1340+
// `missing_named_lifetime_spots` that we shouldn't, like associated
1341+
// `const`s or making a mistake in the AST lowering we would provide
1342+
// non-sensical suggestions. Guard against that by skipping these.
1343+
// (#74264)
1344+
continue;
1345+
}
12851346
msg = "consider introducing a named lifetime parameter".to_string();
12861347
should_break = true;
12871348
if let Some(param) = generics.params.iter().find(|p| match p.kind {
@@ -1308,6 +1369,42 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
13081369
);
13091370
(*span, span_type.suggestion("'a"))
13101371
}
1372+
MissingLifetimeSpot::Static => {
1373+
let (span, sugg) = match snippet.as_deref() {
1374+
Some("&") => (span.shrink_to_hi(), "'static ".to_owned()),
1375+
Some("'_") => (span, "'static".to_owned()),
1376+
Some(snippet) if !snippet.ends_with('>') => {
1377+
if snippet == "" {
1378+
(
1379+
span,
1380+
std::iter::repeat("'static")
1381+
.take(count)
1382+
.collect::<Vec<_>>()
1383+
.join(", "),
1384+
)
1385+
} else {
1386+
(
1387+
span.shrink_to_hi(),
1388+
format!(
1389+
"<{}>",
1390+
std::iter::repeat("'static")
1391+
.take(count)
1392+
.collect::<Vec<_>>()
1393+
.join(", ")
1394+
),
1395+
)
1396+
}
1397+
}
1398+
_ => continue,
1399+
};
1400+
err.span_suggestion_verbose(
1401+
span,
1402+
"consider using the `'static` lifetime",
1403+
sugg.to_string(),
1404+
Applicability::MaybeIncorrect,
1405+
);
1406+
continue;
1407+
}
13111408
});
13121409
for param in params {
13131410
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span) {
@@ -1328,41 +1425,57 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
13281425
}
13291426
};
13301427

1331-
match (lifetime_names.len(), lifetime_names.iter().next(), snippet.as_deref()) {
1332-
(1, Some(name), Some("&")) => {
1333-
suggest_existing(err, format!("&{} ", name));
1428+
let lifetime_names: Vec<_> = lifetime_names.into_iter().collect();
1429+
match (&lifetime_names[..], snippet.as_deref()) {
1430+
([name], Some("&")) => {
1431+
suggest_existing(err, &name.as_str()[..], &|name| format!("&{} ", name));
13341432
}
1335-
(1, Some(name), Some("'_")) => {
1336-
suggest_existing(err, name.to_string());
1433+
([name], Some("'_")) => {
1434+
suggest_existing(err, &name.as_str()[..], &|n| n.to_string());
13371435
}
1338-
(1, Some(name), Some("")) => {
1339-
suggest_existing(err, format!("{}, ", name).repeat(count));
1436+
([name], Some("")) => {
1437+
suggest_existing(err, &name.as_str()[..], &|n| format!("{}, ", n).repeat(count));
13401438
}
1341-
(1, Some(name), Some(snippet)) if !snippet.ends_with('>') => {
1342-
suggest_existing(
1343-
err,
1439+
([name], Some(snippet)) if !snippet.ends_with('>') => {
1440+
let f = |name: &str| {
13441441
format!(
13451442
"{}<{}>",
13461443
snippet,
13471444
std::iter::repeat(name.to_string())
13481445
.take(count)
13491446
.collect::<Vec<_>>()
13501447
.join(", ")
1351-
),
1352-
);
1448+
)
1449+
};
1450+
suggest_existing(err, &name.as_str()[..], &f);
13531451
}
1354-
(0, _, Some("&")) if count == 1 => {
1452+
([], Some("&")) if count == 1 => {
13551453
suggest_new(err, "&'a ");
13561454
}
1357-
(0, _, Some("'_")) if count == 1 => {
1455+
([], Some("'_")) if count == 1 => {
13581456
suggest_new(err, "'a");
13591457
}
1360-
(0, _, Some(snippet)) if !snippet.ends_with('>') && count == 1 => {
1361-
suggest_new(err, &format!("{}<'a>", snippet));
1458+
([], Some(snippet)) if !snippet.ends_with('>') => {
1459+
if snippet == "" {
1460+
// This happens when we have `type Bar<'a> = Foo<T>` where we point at the space
1461+
// before `T`. We will suggest `type Bar<'a> = Foo<'a, T>`.
1462+
suggest_new(
1463+
err,
1464+
&std::iter::repeat("'a, ").take(count).collect::<Vec<_>>().join(""),
1465+
);
1466+
} else {
1467+
suggest_new(
1468+
err,
1469+
&format!(
1470+
"{}<{}>",
1471+
snippet,
1472+
std::iter::repeat("'a").take(count).collect::<Vec<_>>().join(", ")
1473+
),
1474+
);
1475+
}
13621476
}
1363-
(n, ..) if n > 1 => {
1364-
let spans: Vec<Span> = lifetime_names.iter().map(|lt| lt.span).collect();
1365-
err.span_note(spans, "these named lifetimes are available to use");
1477+
(lts, ..) if lts.len() > 1 => {
1478+
err.span_note(lifetime_spans, "these named lifetimes are available to use");
13661479
if Some("") == snippet.as_deref() {
13671480
// This happens when we have `Foo<T>` where we point at the space before `T`,
13681481
// but this can be confusing so we give a suggestion with placeholders.

‎src/librustc_resolve/late/lifetimes.rs

+35-12
Original file line numberDiff line numberDiff line change
@@ -711,18 +711,20 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
711711

712712
fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) {
713713
use self::hir::TraitItemKind::*;
714-
self.missing_named_lifetime_spots.push((&trait_item.generics).into());
715714
match trait_item.kind {
716715
Fn(ref sig, _) => {
716+
self.missing_named_lifetime_spots.push((&trait_item.generics).into());
717717
let tcx = self.tcx;
718718
self.visit_early_late(
719719
Some(tcx.hir().get_parent_item(trait_item.hir_id)),
720720
&sig.decl,
721721
&trait_item.generics,
722722
|this| intravisit::walk_trait_item(this, trait_item),
723723
);
724+
self.missing_named_lifetime_spots.pop();
724725
}
725726
Type(bounds, ref ty) => {
727+
self.missing_named_lifetime_spots.push((&trait_item.generics).into());
726728
let generics = &trait_item.generics;
727729
let mut index = self.next_early_index();
728730
debug!("visit_ty: index = {}", index);
@@ -757,31 +759,35 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
757759
this.visit_ty(ty);
758760
}
759761
});
762+
self.missing_named_lifetime_spots.pop();
760763
}
761764
Const(_, _) => {
762765
// Only methods and types support generics.
763766
assert!(trait_item.generics.params.is_empty());
767+
self.missing_named_lifetime_spots.push(MissingLifetimeSpot::Static);
764768
intravisit::walk_trait_item(self, trait_item);
769+
self.missing_named_lifetime_spots.pop();
765770
}
766771
}
767-
self.missing_named_lifetime_spots.pop();
768772
}
769773

770774
fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) {
771775
use self::hir::ImplItemKind::*;
772-
self.missing_named_lifetime_spots.push((&impl_item.generics).into());
773776
match impl_item.kind {
774777
Fn(ref sig, _) => {
778+
self.missing_named_lifetime_spots.push((&impl_item.generics).into());
775779
let tcx = self.tcx;
776780
self.visit_early_late(
777781
Some(tcx.hir().get_parent_item(impl_item.hir_id)),
778782
&sig.decl,
779783
&impl_item.generics,
780784
|this| intravisit::walk_impl_item(this, impl_item),
781-
)
785+
);
786+
self.missing_named_lifetime_spots.pop();
782787
}
783788
TyAlias(ref ty) => {
784789
let generics = &impl_item.generics;
790+
self.missing_named_lifetime_spots.push(generics.into());
785791
let mut index = self.next_early_index();
786792
let mut non_lifetime_count = 0;
787793
debug!("visit_ty: index = {}", index);
@@ -810,14 +816,16 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
810816
this.visit_generics(generics);
811817
this.visit_ty(ty);
812818
});
819+
self.missing_named_lifetime_spots.pop();
813820
}
814821
Const(_, _) => {
815822
// Only methods and types support generics.
816823
assert!(impl_item.generics.params.is_empty());
824+
self.missing_named_lifetime_spots.push(MissingLifetimeSpot::Static);
817825
intravisit::walk_impl_item(self, impl_item);
826+
self.missing_named_lifetime_spots.pop();
818827
}
819828
}
820-
self.missing_named_lifetime_spots.pop();
821829
}
822830

823831
fn visit_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
@@ -2315,6 +2323,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23152323
let mut late_depth = 0;
23162324
let mut scope = self.scope;
23172325
let mut lifetime_names = FxHashSet::default();
2326+
let mut lifetime_spans = vec![];
23182327
let error = loop {
23192328
match *scope {
23202329
// Do not assign any resolution, it will be inferred.
@@ -2326,7 +2335,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23262335
// collect named lifetimes for suggestions
23272336
for name in lifetimes.keys() {
23282337
if let hir::ParamName::Plain(name) = name {
2329-
lifetime_names.insert(*name);
2338+
lifetime_names.insert(name.name);
2339+
lifetime_spans.push(name.span);
23302340
}
23312341
}
23322342
late_depth += 1;
@@ -2344,12 +2354,24 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23442354
}
23452355
Elide::Exact(l) => l.shifted(late_depth),
23462356
Elide::Error(ref e) => {
2347-
if let Scope::Binder { ref lifetimes, .. } = s {
2348-
// collect named lifetimes for suggestions
2349-
for name in lifetimes.keys() {
2350-
if let hir::ParamName::Plain(name) = name {
2351-
lifetime_names.insert(*name);
2357+
let mut scope = s;
2358+
loop {
2359+
match scope {
2360+
Scope::Binder { ref lifetimes, s, .. } => {
2361+
// Collect named lifetimes for suggestions.
2362+
for name in lifetimes.keys() {
2363+
if let hir::ParamName::Plain(name) = name {
2364+
lifetime_names.insert(name.name);
2365+
lifetime_spans.push(name.span);
2366+
}
2367+
}
2368+
scope = s;
2369+
}
2370+
Scope::ObjectLifetimeDefault { ref s, .. }
2371+
| Scope::Elision { ref s, .. } => {
2372+
scope = s;
23522373
}
2374+
_ => break,
23532375
}
23542376
}
23552377
break Some(e);
@@ -2373,14 +2395,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23732395
if let Some(params) = error {
23742396
// If there's no lifetime available, suggest `'static`.
23752397
if self.report_elision_failure(&mut err, params) && lifetime_names.is_empty() {
2376-
lifetime_names.insert(Ident::with_dummy_span(kw::StaticLifetime));
2398+
lifetime_names.insert(kw::StaticLifetime);
23772399
}
23782400
}
23792401
self.add_missing_lifetime_specifiers_label(
23802402
&mut err,
23812403
span,
23822404
lifetime_refs.len(),
23832405
&lifetime_names,
2406+
lifetime_spans,
23842407
error.map(|p| &p[..]).unwrap_or(&[]),
23852408
);
23862409
err.emit();

‎src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ LL | fn elision<T: Fn() -> &i32>() {
55
| ^ expected named lifetime parameter
66
|
77
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
8+
= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
9+
help: consider making the bound lifetime-generic with a new `'a` lifetime
10+
|
11+
LL | fn elision<T: for<'a> Fn() -> &'a i32>() {
12+
| ^^^^^^^ ^^^
813
help: consider using the `'static` lifetime
914
|
1015
LL | fn elision<T: Fn() -> &'static i32>() {

0 commit comments

Comments
 (0)