Skip to content

Commit 8a93aef

Browse files
authored
Rollup merge of rust-lang#92183 - tmandry:issue-74256, r=estebank
Point at correct argument when async fn output type lifetime disagrees with signature Fixes most of rust-lang#74256. ## Problems fixed This PR fixes a couple of related problems in the error reporting code. ### Highlighting the wrong argument First, the error reporting code was looking at the desugared return type of an `async fn` to decide which parameter to highlight. For example, a function like ```rust async fn async_fn(self: &Struct, f: &u32) -> &u32 { f } ``` desugars to ```rust async fn async_fn<'a, 'b>(self: &'a Struct, f: &'b u32) -> impl Future<Output = &'a u32> + 'a + 'b { f } ``` Since `f: &'b u32` is returned but the output type is `&'a u32`, the error would occur when checking that `'a: 'b`. The reporting code would look to see if the "offending" lifetime `'b` was included in the return type, and because the code was looking at the desugared future type, it was included. So it defaulted to reporting that the source of the other lifetime `'a` (the `self` type) was the problem, when it was really the type of `f`. (Note that if it had chosen instead to look at `'a` first, it too would have been included in the output type, and it would have arbitrarily reported the error (correctly this time) on the type of `f`.) Looking at the actual future type isn't useful for this reason; it captures all input lifetimes. Using the written return type for `async fn` solves this problem and results in less confusing error messages for the user. This isn't a perfect fix, unfortunately; writing the "manually desugared" form of the above function still results in the wrong parameter being highlighted. Looking at the output type of every `impl Future` return type doesn't feel like a very principled approach, though it might work. The problem would remain for function signatures that look like the desugared one above but use different traits. There may be deeper changes required to pinpoint which part of each type is conflicting. ### Lying about await point capture causing lifetime conflicts The second issue fixed by this PR is the unnecessary complexity in `try_report_anon_anon_conflict`. It turns out that the root cause I suggested in rust-lang#76547 (comment) wasn't really the root cause. Adding special handling to report that a variable was captured over an await point only made the error messages less correct and pointed to a problem other than the one that actually occurred. Given the above discussion, it's easy to see why: `async fn`s capture all input lifetimes in their return type, so holding an argument across an await point should never cause a lifetime conflict! Removing the special handling simplified the code and improved the error messages (though they still aren't very good!) ## Future work * Fix error reporting on the "desugared" form of this code * Get the `suggest_adding_lifetime_params` suggestion firing on these examples * cc rust-lang#42703, I think r? `@estebank`
2 parents 69d25fc + 50ac0a3 commit 8a93aef

File tree

16 files changed

+233
-342
lines changed

16 files changed

+233
-342
lines changed

compiler/rustc_hir/src/hir.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -2743,6 +2743,10 @@ pub struct FnHeader {
27432743
}
27442744

27452745
impl FnHeader {
2746+
pub fn is_async(&self) -> bool {
2747+
matches!(&self.asyncness, IsAsync::Async)
2748+
}
2749+
27462750
pub fn is_const(&self) -> bool {
27472751
matches!(&self.constness, Constness::Const)
27482752
}
@@ -3184,7 +3188,7 @@ impl<'hir> Node<'hir> {
31843188
}
31853189
}
31863190

3187-
pub fn fn_decl(&self) -> Option<&FnDecl<'hir>> {
3191+
pub fn fn_decl(&self) -> Option<&'hir FnDecl<'hir>> {
31883192
match self {
31893193
Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
31903194
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
@@ -3196,6 +3200,15 @@ impl<'hir> Node<'hir> {
31963200
}
31973201
}
31983202

3203+
pub fn fn_sig(&self) -> Option<&'hir FnSig<'hir>> {
3204+
match self {
3205+
Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
3206+
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
3207+
| Node::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig),
3208+
_ => None,
3209+
}
3210+
}
3211+
31993212
pub fn body_id(&self) -> Option<BodyId> {
32003213
match self {
32013214
Node::TraitItem(TraitItem {

compiler/rustc_infer/src/infer/error_reporting/mod.rs

+19-12
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@ use rustc_hir::def_id::DefId;
6565
use rustc_hir::lang_items::LangItem;
6666
use rustc_hir::{Item, ItemKind, Node};
6767
use rustc_middle::dep_graph::DepContext;
68-
use rustc_middle::ty::error::TypeError;
6968
use rustc_middle::ty::{
7069
self,
70+
error::TypeError,
7171
subst::{GenericArgKind, Subst, SubstsRef},
72-
Region, Ty, TyCtxt, TypeFoldable,
72+
Binder, Region, Ty, TyCtxt, TypeFoldable,
7373
};
7474
use rustc_span::{sym, BytePos, DesugaringKind, MultiSpan, Pos, Span};
7575
use rustc_target::spec::abi;
@@ -1771,7 +1771,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
17711771
self.note_error_origin(diag, cause, exp_found, terr);
17721772
}
17731773

1774-
pub fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
1774+
pub fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option<Binder<'tcx, Ty<'tcx>>> {
17751775
if let ty::Opaque(def_id, substs) = ty.kind() {
17761776
let future_trait = self.tcx.require_lang_item(LangItem::Future, None);
17771777
// Future::Output
@@ -1781,13 +1781,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
17811781

17821782
for (predicate, _) in bounds {
17831783
let predicate = predicate.subst(self.tcx, substs);
1784-
if let ty::PredicateKind::Projection(projection_predicate) =
1785-
predicate.kind().skip_binder()
1786-
{
1787-
if projection_predicate.projection_ty.item_def_id == item_def_id {
1788-
// We don't account for multiple `Future::Output = Ty` contraints.
1789-
return Some(projection_predicate.ty);
1790-
}
1784+
let output = predicate
1785+
.kind()
1786+
.map_bound(|kind| match kind {
1787+
ty::PredicateKind::Projection(projection_predicate)
1788+
if projection_predicate.projection_ty.item_def_id == item_def_id =>
1789+
{
1790+
Some(projection_predicate.ty)
1791+
}
1792+
_ => None,
1793+
})
1794+
.transpose();
1795+
if output.is_some() {
1796+
// We don't account for multiple `Future::Output = Ty` contraints.
1797+
return output;
17911798
}
17921799
}
17931800
}
@@ -1829,8 +1836,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
18291836
}
18301837

18311838
match (
1832-
self.get_impl_future_output_ty(exp_found.expected),
1833-
self.get_impl_future_output_ty(exp_found.found),
1839+
self.get_impl_future_output_ty(exp_found.expected).map(Binder::skip_binder),
1840+
self.get_impl_future_output_ty(exp_found.found).map(Binder::skip_binder),
18341841
) {
18351842
(Some(exp), Some(found)) if same_type_modulo_infer(exp, found) => match cause.code() {
18361843
ObligationCauseCode::IfExpression(box IfExpressionCause { then, .. }) => {

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/different_lifetimes.rs

+33-76
Original file line numberDiff line numberDiff line change
@@ -106,90 +106,47 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
106106
None => String::new(),
107107
};
108108

109-
let (span_1, span_2, main_label, span_label, future_return_type) =
110-
match (sup_is_ret_type, sub_is_ret_type) {
111-
(None, None) => {
112-
let (main_label_1, span_label_1) = if ty_sup.hir_id == ty_sub.hir_id {
113-
(
114-
"this type is declared with multiple lifetimes...".to_owned(),
115-
"...but data with one lifetime flows into the other here".to_owned(),
116-
)
117-
} else {
118-
(
119-
"these two types are declared with different lifetimes...".to_owned(),
120-
format!("...but data{} flows{} here", span_label_var1, span_label_var2),
121-
)
122-
};
123-
(ty_sup.span, ty_sub.span, main_label_1, span_label_1, None)
124-
}
109+
debug!(
110+
"try_report_anon_anon_conflict: sub_is_ret_type={:?} sup_is_ret_type={:?}",
111+
sub_is_ret_type, sup_is_ret_type
112+
);
125113

126-
(Some(ret_span), _) => {
127-
let sup_future = self.future_return_type(scope_def_id_sup);
128-
let (return_type, action) = if sup_future.is_some() {
129-
("returned future", "held across an await point")
130-
} else {
131-
("return type", "returned")
132-
};
114+
let mut err = struct_span_err!(self.tcx().sess, span, E0623, "lifetime mismatch");
133115

134-
(
135-
ty_sub.span,
136-
ret_span,
137-
format!(
138-
"this parameter and the {} are declared with different lifetimes...",
139-
return_type
140-
),
141-
format!("...but data{} is {} here", span_label_var1, action),
142-
sup_future,
143-
)
144-
}
145-
(_, Some(ret_span)) => {
146-
let sub_future = self.future_return_type(scope_def_id_sub);
147-
let (return_type, action) = if sub_future.is_some() {
148-
("returned future", "held across an await point")
149-
} else {
150-
("return type", "returned")
151-
};
116+
match (sup_is_ret_type, sub_is_ret_type) {
117+
(ret_capture @ Some(ret_span), _) | (_, ret_capture @ Some(ret_span)) => {
118+
let param_span =
119+
if sup_is_ret_type == ret_capture { ty_sub.span } else { ty_sup.span };
120+
121+
err.span_label(
122+
param_span,
123+
"this parameter and the return type are declared with different lifetimes...",
124+
);
125+
err.span_label(ret_span, "");
126+
err.span_label(span, format!("...but data{} is returned here", span_label_var1));
127+
}
152128

153-
(
129+
(None, None) => {
130+
if ty_sup.hir_id == ty_sub.hir_id {
131+
err.span_label(ty_sup.span, "this type is declared with multiple lifetimes...");
132+
err.span_label(ty_sub.span, "");
133+
err.span_label(span, "...but data with one lifetime flows into the other here");
134+
} else {
135+
err.span_label(
154136
ty_sup.span,
155-
ret_span,
156-
format!(
157-
"this parameter and the {} are declared with different lifetimes...",
158-
return_type
159-
),
160-
format!("...but data{} is {} here", span_label_var1, action),
161-
sub_future,
162-
)
137+
"these two types are declared with different lifetimes...",
138+
);
139+
err.span_label(ty_sub.span, "");
140+
err.span_label(
141+
span,
142+
format!("...but data{} flows{} here", span_label_var1, span_label_var2),
143+
);
163144
}
164-
};
165-
166-
let mut err = struct_span_err!(self.tcx().sess, span, E0623, "lifetime mismatch");
167-
168-
err.span_label(span_1, main_label);
169-
err.span_label(span_2, String::new());
170-
err.span_label(span, span_label);
145+
}
146+
}
171147

172148
self.suggest_adding_lifetime_params(sub, ty_sup, ty_sub, &mut err);
173149

174-
if let Some(t) = future_return_type {
175-
let snip = self
176-
.tcx()
177-
.sess
178-
.source_map()
179-
.span_to_snippet(t.span)
180-
.ok()
181-
.and_then(|s| match (&t.kind, s.as_str()) {
182-
(rustc_hir::TyKind::Tup(&[]), "") => Some("()".to_string()),
183-
(_, "") => None,
184-
_ => Some(s),
185-
})
186-
.unwrap_or_else(|| "{unnamed_type}".to_string());
187-
188-
err.span_label(
189-
t.span,
190-
&format!("this `async fn` implicitly returns an `impl Future<Output = {}>`", snip),
191-
);
192-
}
193150
err.emit();
194151
Some(ErrorReported)
195152
}

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/find_anon_type.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use rustc_hir as hir;
22
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
3-
use rustc_hir::Node;
43
use rustc_middle::hir::map::Map;
54
use rustc_middle::middle::resolve_lifetime as rl;
65
use rustc_middle::ty::{self, Region, TyCtxt};
@@ -24,25 +23,19 @@ pub(crate) fn find_anon_type<'tcx>(
2423
tcx: TyCtxt<'tcx>,
2524
region: Region<'tcx>,
2625
br: &ty::BoundRegionKind,
27-
) -> Option<(&'tcx hir::Ty<'tcx>, &'tcx hir::FnDecl<'tcx>)> {
26+
) -> Option<(&'tcx hir::Ty<'tcx>, &'tcx hir::FnSig<'tcx>)> {
2827
if let Some(anon_reg) = tcx.is_suitable_region(region) {
2928
let hir_id = tcx.hir().local_def_id_to_hir_id(anon_reg.def_id);
30-
let fndecl = match tcx.hir().get(hir_id) {
31-
Node::Item(&hir::Item { kind: hir::ItemKind::Fn(ref m, ..), .. })
32-
| Node::TraitItem(&hir::TraitItem {
33-
kind: hir::TraitItemKind::Fn(ref m, ..), ..
34-
})
35-
| Node::ImplItem(&hir::ImplItem { kind: hir::ImplItemKind::Fn(ref m, ..), .. }) => {
36-
&m.decl
37-
}
38-
_ => return None,
29+
let Some(fn_sig) = tcx.hir().get(hir_id).fn_sig() else {
30+
return None
3931
};
4032

41-
fndecl
33+
fn_sig
34+
.decl
4235
.inputs
4336
.iter()
4437
.find_map(|arg| find_component_for_bound_region(tcx, arg, br))
45-
.map(|ty| (ty, &**fndecl))
38+
.map(|ty| (ty, fn_sig))
4639
} else {
4740
None
4841
}

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/util.rs

+26-64
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
55
use rustc_hir as hir;
66
use rustc_hir::def_id::LocalDefId;
7-
use rustc_middle::ty::{self, DefIdTree, Region, Ty};
7+
use rustc_middle::ty::{self, Binder, DefIdTree, Region, Ty, TypeFoldable};
88
use rustc_span::Span;
99

1010
/// Information about the anonymous region we are searching for.
@@ -94,80 +94,42 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
9494
})
9595
}
9696

97-
pub(super) fn future_return_type(
98-
&self,
99-
local_def_id: LocalDefId,
100-
) -> Option<&rustc_hir::Ty<'_>> {
101-
if let Some(hir::IsAsync::Async) = self.asyncness(local_def_id) {
102-
if let rustc_middle::ty::Opaque(def_id, _) =
103-
self.tcx().type_of(local_def_id).fn_sig(self.tcx()).output().skip_binder().kind()
104-
{
105-
match self.tcx().hir().get_if_local(*def_id) {
106-
Some(hir::Node::Item(hir::Item {
107-
kind:
108-
hir::ItemKind::OpaqueTy(hir::OpaqueTy {
109-
bounds,
110-
origin: hir::OpaqueTyOrigin::AsyncFn(..),
111-
..
112-
}),
113-
..
114-
})) => {
115-
for b in bounds.iter() {
116-
if let hir::GenericBound::LangItemTrait(
117-
hir::LangItem::Future,
118-
_span,
119-
_hir_id,
120-
generic_args,
121-
) = b
122-
{
123-
for type_binding in generic_args.bindings.iter() {
124-
if type_binding.ident.name == rustc_span::sym::Output {
125-
if let hir::TypeBindingKind::Equality { ty } =
126-
type_binding.kind
127-
{
128-
return Some(ty);
129-
}
130-
}
131-
}
132-
}
133-
}
134-
}
135-
_ => {}
136-
}
137-
}
138-
}
139-
None
140-
}
141-
142-
pub(super) fn asyncness(&self, local_def_id: LocalDefId) -> Option<hir::IsAsync> {
143-
// similar to the asyncness fn in rustc_ty_utils::ty
144-
let hir_id = self.tcx().hir().local_def_id_to_hir_id(local_def_id);
145-
let node = self.tcx().hir().get(hir_id);
146-
let fn_kind = node.fn_kind()?;
147-
Some(fn_kind.asyncness())
148-
}
149-
15097
// Here, we check for the case where the anonymous region
151-
// is in the return type.
98+
// is in the return type as written by the user.
15299
// FIXME(#42703) - Need to handle certain cases here.
153100
pub(super) fn is_return_type_anon(
154101
&self,
155102
scope_def_id: LocalDefId,
156103
br: ty::BoundRegionKind,
157-
decl: &hir::FnDecl<'_>,
104+
hir_sig: &hir::FnSig<'_>,
158105
) -> Option<Span> {
159-
let ret_ty = self.tcx().type_of(scope_def_id);
160-
if let ty::FnDef(_, _) = ret_ty.kind() {
161-
let sig = ret_ty.fn_sig(self.tcx());
162-
let late_bound_regions =
163-
self.tcx().collect_referenced_late_bound_regions(&sig.output());
164-
if late_bound_regions.iter().any(|r| *r == br) {
165-
return Some(decl.output.span());
166-
}
106+
let fn_ty = self.tcx().type_of(scope_def_id);
107+
if let ty::FnDef(_, _) = fn_ty.kind() {
108+
let ret_ty = fn_ty.fn_sig(self.tcx()).output();
109+
let span = hir_sig.decl.output.span();
110+
let future_output = if hir_sig.header.is_async() {
111+
ret_ty.map_bound(|ty| self.infcx.get_impl_future_output_ty(ty)).transpose()
112+
} else {
113+
None
114+
};
115+
return match future_output {
116+
Some(output) if self.includes_region(output, br) => Some(span),
117+
None if self.includes_region(ret_ty, br) => Some(span),
118+
_ => None,
119+
};
167120
}
168121
None
169122
}
170123

124+
fn includes_region(
125+
&self,
126+
ty: Binder<'tcx, impl TypeFoldable<'tcx>>,
127+
region: ty::BoundRegionKind,
128+
) -> bool {
129+
let late_bound_regions = self.tcx().collect_referenced_late_bound_regions(&ty);
130+
late_bound_regions.iter().any(|r| *r == region)
131+
}
132+
171133
// Here we check for the case where anonymous region
172134
// corresponds to self and if yes, we display E0312.
173135
// FIXME(#42700) - Need to format self properly to

compiler/rustc_typeck/src/check/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
19151915
_ => return,
19161916
};
19171917
let mut add_label = true;
1918-
if let ty::Adt(def, _) = output_ty.kind() {
1918+
if let ty::Adt(def, _) = output_ty.skip_binder().kind() {
19191919
// no field access on enum type
19201920
if !def.is_enum() {
19211921
if def

compiler/rustc_typeck/src/check/method/suggest.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12901290
span: Span,
12911291
) {
12921292
let output_ty = match self.infcx.get_impl_future_output_ty(ty) {
1293-
Some(output_ty) => self.resolve_vars_if_possible(output_ty),
1293+
Some(output_ty) => self.resolve_vars_if_possible(output_ty).skip_binder(),
12941294
_ => return,
12951295
};
12961296
let method_exists = self.method_exists(item_name, output_ty, call.hir_id, true);

0 commit comments

Comments
 (0)