Skip to content

Commit 562f496

Browse files
estebankestebank
committed
Account for multiple impl/dyn Trait in return type when suggesting '_ 
1 parent a39c778 commit 562f496

12 files changed

+363
-77
lines changed

‎src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
2626
&self,
2727
region: Region<'tcx>,
2828
br: &ty::BoundRegion,
29-
) -> Option<(&hir::Ty<'_>, &hir::FnDecl<'_>)> {
29+
) -> Option<(&hir::Ty<'tcx>, &hir::FnDecl<'tcx>)> {
3030
if let Some(anon_reg) = self.tcx().is_suitable_region(region) {
3131
let def_id = anon_reg.def_id;
3232
if let Some(def_id) = def_id.as_local() {

‎src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//! where one region is named and the other is anonymous.
33
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
44
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
5-
use rustc_hir::{FnRetTy, TyKind};
5+
use rustc_hir::intravisit::Visitor;
6+
use rustc_hir::FnRetTy;
67
use rustc_middle::ty;
78

89
impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
@@ -80,12 +81,16 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
8081
}
8182

8283
if let FnRetTy::Return(ty) = &fndecl.output {
83-
let mut v = ty::TraitObjectVisitor(vec![]);
84-
rustc_hir::intravisit::walk_ty(&mut v, ty);
84+
let mut v = ty::TraitObjectVisitor(vec![], self.tcx().hir());
85+
v.visit_ty(ty);
8586

8687
debug!("try_report_named_anon_conflict: ret ty {:?}", ty);
8788
if sub == &ty::ReStatic
88-
&& (matches!(ty.kind, TyKind::OpaqueDef(_, _)) || v.0.len() == 1)
89+
&& v.0
90+
.into_iter()
91+
.filter(|t| t.span.desugaring_kind().is_none())
92+
.next()
93+
.is_some()
8994
{
9095
debug!("try_report_named_anon_conflict: impl Trait + 'static");
9196
// This is an `impl Trait` or `dyn Trait` return that evaluates de need of

‎src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
2626
);
2727
let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?;
2828
debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup);
29-
let fn_return = self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id)?;
30-
debug!("try_report_static_impl_trait: fn_return={:?}", fn_return);
29+
let fn_returns = self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id);
30+
if fn_returns.is_empty() {
31+
return None;
32+
}
33+
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
3134
if **sub_r == RegionKind::ReStatic {
3235
let sp = var_origin.span();
3336
let return_sp = sub_origin.span();
@@ -98,25 +101,26 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
98101
);
99102
}
100103

101-
// only apply this suggestion onto functions with
102-
// explicit non-desugar'able return.
103-
if fn_return.span.desugaring_kind().is_none() {
104-
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
105-
106-
let consider = "consider changing the";
107-
let declare = "to declare that the";
108-
let arg = match param_info.param.pat.simple_ident() {
109-
Some(simple_ident) => format!("argument `{}`", simple_ident),
110-
None => "the argument".to_string(),
111-
};
112-
let explicit =
113-
format!("you can add an explicit `{}` lifetime bound", lifetime_name);
114-
let explicit_static =
115-
format!("explicit `'static` bound to the lifetime of {}", arg);
116-
let captures = format!("captures data from {}", arg);
117-
let add_static_bound =
118-
"alternatively, add an explicit `'static` bound to this reference";
119-
let plus_lt = format!(" + {}", lifetime_name);
104+
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
105+
let consider = "consider changing the";
106+
let declare = "to declare that the";
107+
let arg = match param_info.param.pat.simple_ident() {
108+
Some(simple_ident) => format!("argument `{}`", simple_ident),
109+
None => "the argument".to_string(),
110+
};
111+
let explicit =
112+
format!("you can add an explicit `{}` lifetime bound", lifetime_name);
113+
let explicit_static =
114+
format!("explicit `'static` bound to the lifetime of {}", arg);
115+
let captures = format!("captures data from {}", arg);
116+
let add_static_bound =
117+
"alternatively, add an explicit `'static` bound to this reference";
118+
let plus_lt = format!(" + {}", lifetime_name);
119+
for fn_return in fn_returns {
120+
if fn_return.span.desugaring_kind().is_some() {
121+
// Skip `async` desugaring `impl Future`.
122+
continue;
123+
}
120124
match fn_return.kind {
121125
TyKind::OpaqueDef(item_id, _) => {
122126
let item = self.tcx().hir().item(item_id.id);
@@ -143,7 +147,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
143147
err.span_suggestion_verbose(
144148
span,
145149
&format!("{} `impl Trait`'s {}", consider, explicit_static),
146-
lifetime_name,
150+
lifetime_name.clone(),
147151
Applicability::MaybeIncorrect,
148152
);
149153
err.span_suggestion_verbose(
@@ -152,6 +156,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
152156
param_info.param_ty.to_string(),
153157
Applicability::MaybeIncorrect,
154158
);
159+
} else if let Some(_) = opaque
160+
.bounds
161+
.iter()
162+
.filter_map(|arg| match arg {
163+
GenericBound::Outlives(Lifetime { name, span, .. })
164+
if name.ident().to_string() == lifetime_name =>
165+
{
166+
Some(*span)
167+
}
168+
_ => None,
169+
})
170+
.next()
171+
{
155172
} else {
156173
err.span_suggestion_verbose(
157174
fn_return.span.shrink_to_hi(),
@@ -161,10 +178,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
161178
captures = captures,
162179
explicit = explicit,
163180
),
164-
plus_lt,
181+
plus_lt.clone(),
165182
Applicability::MaybeIncorrect,
166183
);
167-
};
184+
}
168185
}
169186
TyKind::TraitObject(_, lt) => match lt.name {
170187
LifetimeName::ImplicitObjectLifetimeDefault => {
@@ -176,15 +193,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
176193
captures = captures,
177194
explicit = explicit,
178195
),
179-
plus_lt,
196+
plus_lt.clone(),
180197
Applicability::MaybeIncorrect,
181198
);
182199
}
183-
_ => {
200+
name if name.ident().to_string() != lifetime_name => {
201+
// With this check we avoid suggesting redundant bounds. This
202+
// would happen if there are nested impl/dyn traits and only
203+
// one of them has the bound we'd suggest already there, like
204+
// in `impl Foo<X = dyn Bar> + '_`.
184205
err.span_suggestion_verbose(
185206
lt.span,
186207
&format!("{} trait object's {}", consider, explicit_static),
187-
lifetime_name,
208+
lifetime_name.clone(),
188209
Applicability::MaybeIncorrect,
189210
);
190211
err.span_suggestion_verbose(
@@ -194,6 +215,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
194215
Applicability::MaybeIncorrect,
195216
);
196217
}
218+
_ => {}
197219
},
198220
_ => {}
199221
}

‎src/librustc_middle/ty/context.rs

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use rustc_hir as hir;
3737
use rustc_hir::def::{DefKind, Res};
3838
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId, LOCAL_CRATE};
3939
use rustc_hir::definitions::{DefPathHash, Definitions};
40+
use rustc_hir::intravisit::Visitor;
4041
use rustc_hir::lang_items::{self, PanicLocationLangItem};
4142
use rustc_hir::{HirId, ItemKind, ItemLocalId, ItemLocalMap, ItemLocalSet, Node, TraitCandidate};
4243
use rustc_index::vec::{Idx, IndexVec};
@@ -1405,10 +1406,7 @@ impl<'tcx> TyCtxt<'tcx> {
14051406
})
14061407
}
14071408

1408-
pub fn return_type_impl_or_dyn_trait(
1409-
&self,
1410-
scope_def_id: DefId,
1411-
) -> Option<&'tcx hir::Ty<'tcx>> {
1409+
pub fn return_type_impl_or_dyn_trait(&self, scope_def_id: DefId) -> Vec<&'tcx hir::Ty<'tcx>> {
14121410
let hir_id = self.hir().as_local_hir_id(scope_def_id.expect_local());
14131411
let hir_output = match self.hir().get(hir_id) {
14141412
Node::Item(hir::Item {
@@ -1444,30 +1442,12 @@ impl<'tcx> TyCtxt<'tcx> {
14441442
),
14451443
..
14461444
}) => ty,
1447-
_ => return None,
1445+
_ => return vec![],
14481446
};
14491447

1450-
let ret_ty = self.type_of(scope_def_id);
1451-
match ret_ty.kind {
1452-
ty::FnDef(_, _) => {
1453-
let sig = ret_ty.fn_sig(*self);
1454-
let output = self.erase_late_bound_regions(&sig.output());
1455-
if output.is_impl_trait() {
1456-
let fn_decl = self.hir().fn_decl_by_hir_id(hir_id).unwrap();
1457-
if let hir::FnRetTy::Return(ty) = fn_decl.output {
1458-
return Some(ty);
1459-
}
1460-
} else {
1461-
let mut v = TraitObjectVisitor(vec![]);
1462-
rustc_hir::intravisit::walk_ty(&mut v, hir_output);
1463-
if v.0.len() == 1 {
1464-
return Some(v.0[0]);
1465-
}
1466-
}
1467-
None
1468-
}
1469-
_ => None,
1470-
}
1448+
let mut v = TraitObjectVisitor(vec![], self.hir());
1449+
v.visit_ty(hir_output);
1450+
v.0
14711451
}
14721452

14731453
pub fn return_type_impl_trait(&self, scope_def_id: DefId) -> Option<(Ty<'tcx>, Span)> {

‎src/librustc_middle/ty/diagnostics.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,9 @@ pub fn suggest_constraining_type_param(
236236
}
237237
}
238238

239-
pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>);
239+
/// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.
240+
pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>, pub crate::hir::map::Map<'tcx>);
241+
240242
impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
241243
type Map = rustc_hir::intravisit::ErasedMap<'v>;
242244

@@ -245,15 +247,24 @@ impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
245247
}
246248

247249
fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
248-
if let hir::TyKind::TraitObject(
249-
_,
250-
hir::Lifetime {
251-
name: hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
252-
..
253-
},
254-
) = ty.kind
255-
{
256-
self.0.push(ty);
250+
match ty.kind {
251+
hir::TyKind::TraitObject(
252+
_,
253+
hir::Lifetime {
254+
name:
255+
hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
256+
..
257+
},
258+
) => {
259+
self.0.push(ty);
260+
}
261+
hir::TyKind::OpaqueDef(item_id, _) => {
262+
self.0.push(ty);
263+
let item = self.1.expect_item(item_id.id);
264+
hir::intravisit::walk_item(self, item);
265+
}
266+
_ => {}
257267
}
268+
hir::intravisit::walk_ty(self, ty);
258269
}
259270
}

‎src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,15 @@ LL | fn foo<'a>(x: &i32) -> impl Copy + 'a { x }
5353
| help: add explicit lifetime `'a` to the type of `x`: `&'a i32`
5454

5555
error: lifetime may not live long enough
56-
--> $DIR/must_outlive_least_region_or_bound.rs:33:69
56+
--> $DIR/must_outlive_least_region_or_bound.rs:30:24
57+
|
58+
LL | fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
59+
| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ opaque type requires that `'1` must outlive `'static`
60+
| |
61+
| let's call the lifetime of this reference `'1`
62+
63+
error: lifetime may not live long enough
64+
--> $DIR/must_outlive_least_region_or_bound.rs:37:69
5765
|
5866
LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
5967
| -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static`
@@ -62,7 +70,7 @@ LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
6270
= help: consider replacing `'a` with `'static`
6371

6472
error: lifetime may not live long enough
65-
--> $DIR/must_outlive_least_region_or_bound.rs:38:61
73+
--> $DIR/must_outlive_least_region_or_bound.rs:42:61
6674
|
6775
LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32) {
6876
| -- -- lifetime `'b` defined here ^^^^^^^^^^^^^^^^ opaque type requires that `'b` must outlive `'a`
@@ -72,14 +80,14 @@ LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32
7280
= help: consider adding the following bound: `'b: 'a`
7381

7482
error[E0310]: the parameter type `T` may not live long enough
75-
--> $DIR/must_outlive_least_region_or_bound.rs:43:51
83+
--> $DIR/must_outlive_least_region_or_bound.rs:47:51
7684
|
7785
LL | fn ty_param_wont_outlive_static<T:Debug>(x: T) -> impl Debug + 'static {
7886
| ^^^^^^^^^^^^^^^^^^^^
7987
|
8088
= help: consider adding an explicit lifetime bound `T: 'static`...
8189

82-
error: aborting due to 8 previous errors
90+
error: aborting due to 9 previous errors
8391

8492
Some errors have detailed explanations: E0310, E0621.
8593
For more information about an error, try `rustc --explain E0310`.

‎src/test/ui/impl-trait/must_outlive_least_region_or_bound.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ fn elided4(x: &i32) -> Box<dyn Debug + 'static> { Box::new(x) }
2727
fn explicit4<'a>(x: &'a i32) -> Box<dyn Debug + 'static> { Box::new(x) }
2828
//~^ ERROR cannot infer an appropriate lifetime
2929

30+
fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
31+
//~^ ERROR cannot infer an appropriate lifetime
32+
//~| ERROR cannot infer an appropriate lifetime
33+
3034
trait LifetimeTrait<'a> {}
3135
impl<'a> LifetimeTrait<'a> for &'a i32 {}
3236

0 commit comments

Comments
 (0)