Skip to content

Commit d37100e

Browse files
committed
Provide suggestion to dereference closure tail if appropriate
When encoutnering a case like ```rust //@ run-rustfix use std::collections::HashMap; fn main() { let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3]; let mut counts = HashMap::new(); for num in vs { let count = counts.entry(num).or_insert(0); *count += 1; } let _ = counts.iter().max_by_key(|(_, v)| v); ``` produce the following suggestion ``` error: lifetime may not live long enough --> $DIR/return-value-lifetime-error.rs:13:47 | LL | let _ = counts.iter().max_by_key(|(_, v)| v); | ------- ^ returning this value requires that `'1` must outlive `'2` | | | | | return type of closure is &'2 &i32 | has type `&'1 (&i32, &i32)` | help: dereference the return value | LL | let _ = counts.iter().max_by_key(|(_, v)| **v); | ++ ``` Fix #50195.
1 parent 76cf07d commit d37100e

File tree

6 files changed

+256
-0
lines changed

6 files changed

+256
-0
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -3607,6 +3607,7 @@ dependencies = [
36073607
"rustc_fluent_macro",
36083608
"rustc_graphviz",
36093609
"rustc_hir",
3610+
"rustc_hir_typeck",
36103611
"rustc_index",
36113612
"rustc_infer",
36123613
"rustc_lexer",

compiler/rustc_borrowck/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ rustc_errors = { path = "../rustc_errors" }
1313
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
1414
rustc_graphviz = { path = "../rustc_graphviz" }
1515
rustc_hir = { path = "../rustc_hir" }
16+
rustc_hir_typeck = { path = "../rustc_hir_typeck" }
1617
rustc_index = { path = "../rustc_index" }
1718
rustc_infer = { path = "../rustc_infer" }
1819
rustc_lexer = { path = "../rustc_lexer" }

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

+206
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_hir::GenericBound::Trait;
1010
use rustc_hir::QPath::Resolved;
1111
use rustc_hir::WherePredicate::BoundPredicate;
1212
use rustc_hir::{PolyTraitRef, TyKind, WhereBoundPredicate};
13+
use rustc_hir_typeck::{FnCtxt, TypeckRootCtxt};
1314
use rustc_infer::infer::{
1415
error_reporting::nice_region_error::{
1516
self, find_anon_type, find_param_with_region, suggest_adding_lifetime_params,
@@ -20,12 +21,17 @@ use rustc_infer::infer::{
2021
};
2122
use rustc_middle::hir::place::PlaceBase;
2223
use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
24+
use rustc_middle::traits::ObligationCause;
2325
use rustc_middle::ty::GenericArgs;
2426
use rustc_middle::ty::TypeVisitor;
2527
use rustc_middle::ty::{self, RegionVid, Ty};
2628
use rustc_middle::ty::{Region, TyCtxt};
2729
use rustc_span::symbol::{kw, Ident};
2830
use rustc_span::Span;
31+
use rustc_trait_selection::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
32+
use rustc_trait_selection::infer::InferCtxtExt;
33+
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
34+
use rustc_trait_selection::traits::Obligation;
2935

3036
use crate::borrowck_errors;
3137
use crate::session_diagnostics::{
@@ -810,6 +816,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
810816
self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr);
811817
self.suggest_adding_lifetime_params(&mut diag, *fr, *outlived_fr);
812818
self.suggest_move_on_borrowing_closure(&mut diag);
819+
self.suggest_deref_closure_value(&mut diag);
813820

814821
diag
815822
}
@@ -1039,6 +1046,205 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
10391046
suggest_adding_lifetime_params(self.infcx.tcx, sub, ty_sup, ty_sub, diag);
10401047
}
10411048

1049+
#[allow(rustc::diagnostic_outside_of_impl)]
1050+
#[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable
1051+
/// When encountering a lifetime error caused by the return type of a closure, check the
1052+
/// corresponding trait bound and see if dereferencing the closure return value would satisfy
1053+
/// them. If so, we produce a structured suggestion.
1054+
fn suggest_deref_closure_value(&self, diag: &mut Diag<'_>) {
1055+
let tcx = self.infcx.tcx;
1056+
let map = tcx.hir();
1057+
1058+
// Get the closure return value and type.
1059+
let body_id = map.body_owned_by(self.mir_def_id());
1060+
let body = &map.body(body_id);
1061+
let value = &body.value.peel_blocks();
1062+
let hir::Node::Expr(closure_expr) = tcx.hir_node_by_def_id(self.mir_def_id()) else {
1063+
return;
1064+
};
1065+
let fn_call_id = tcx.parent_hir_id(self.mir_hir_id());
1066+
let hir::Node::Expr(expr) = tcx.hir_node(fn_call_id) else { return };
1067+
let def_id = map.enclosing_body_owner(fn_call_id);
1068+
let tables = tcx.typeck(def_id);
1069+
let Some(return_value_ty) = tables.node_type_opt(value.hir_id) else { return };
1070+
let return_value_ty = self.infcx.resolve_vars_if_possible(return_value_ty);
1071+
1072+
// We don't use `ty.peel_refs()` to get the number of `*`s needed to get the root type.
1073+
let mut ty = return_value_ty;
1074+
let mut count = 0;
1075+
while let ty::Ref(_, t, _) = ty.kind() {
1076+
ty = *t;
1077+
count += 1;
1078+
}
1079+
if !self.infcx.type_is_copy_modulo_regions(self.param_env, ty) {
1080+
return;
1081+
}
1082+
1083+
// Build a new closure where the return type is an owned value, instead of a ref.
1084+
let Some(ty::Closure(did, args)) =
1085+
tables.node_type_opt(closure_expr.hir_id).as_ref().map(|ty| ty.kind())
1086+
else {
1087+
return;
1088+
};
1089+
let sig = args.as_closure().sig();
1090+
let closure_sig_as_fn_ptr_ty = Ty::new_fn_ptr(
1091+
tcx,
1092+
sig.map_bound(|s| {
1093+
let unsafety = hir::Unsafety::Normal;
1094+
use rustc_target::spec::abi;
1095+
tcx.mk_fn_sig(
1096+
[s.inputs()[0]],
1097+
s.output().peel_refs(),
1098+
s.c_variadic,
1099+
unsafety,
1100+
abi::Abi::Rust,
1101+
)
1102+
}),
1103+
);
1104+
let parent_args = GenericArgs::identity_for_item(
1105+
tcx,
1106+
tcx.typeck_root_def_id(self.mir_def_id().to_def_id()),
1107+
);
1108+
let closure_kind = args.as_closure().kind();
1109+
let closure_kind_ty = Ty::from_closure_kind(tcx, closure_kind);
1110+
let tupled_upvars_ty = self.infcx.next_ty_var(TypeVariableOrigin {
1111+
kind: TypeVariableOriginKind::ClosureSynthetic,
1112+
span: closure_expr.span,
1113+
});
1114+
let closure_args = ty::ClosureArgs::new(
1115+
tcx,
1116+
ty::ClosureArgsParts {
1117+
parent_args,
1118+
closure_kind_ty,
1119+
closure_sig_as_fn_ptr_ty,
1120+
tupled_upvars_ty,
1121+
},
1122+
);
1123+
let closure_ty = Ty::new_closure(tcx, *did, closure_args.args);
1124+
let closure_ty = tcx.erase_regions(closure_ty);
1125+
1126+
let hir::ExprKind::MethodCall(segment, rcvr, args, _) = expr.kind else { return };
1127+
let Some(pos) = args
1128+
.iter()
1129+
.enumerate()
1130+
.find(|(_, arg)| arg.hir_id == closure_expr.hir_id)
1131+
.map(|(i, _)| i)
1132+
else {
1133+
return;
1134+
};
1135+
// The found `Self` type of the method call.
1136+
let Some(possible_rcvr_ty) = tables.node_type_opt(rcvr.hir_id) else { return };
1137+
1138+
// The `MethodCall` expression is `Res::Err`, so we search for the method on the `rcvr_ty`.
1139+
let root_ctxt = TypeckRootCtxt::new(tcx, self.mir_def_id());
1140+
let fn_ctxt = FnCtxt::new(&root_ctxt, self.param_env, self.mir_def_id());
1141+
let Ok(method) =
1142+
fn_ctxt.lookup_method_for_diagnostic(possible_rcvr_ty, segment, expr.span, expr, rcvr)
1143+
else {
1144+
return;
1145+
};
1146+
1147+
// Get the arguments for the found method, only specifying that `Self` is the receiver type.
1148+
let args = GenericArgs::for_item(tcx, method.def_id, |param, _| {
1149+
if param.index == 0 {
1150+
possible_rcvr_ty.into()
1151+
} else {
1152+
self.infcx.var_for_def(expr.span, param)
1153+
}
1154+
});
1155+
1156+
let preds = tcx.predicates_of(method.def_id).instantiate(tcx, args);
1157+
// Get the type for the parameter corresponding to the argument the closure with the
1158+
// lifetime error we had.
1159+
let Some(input) = tcx
1160+
.fn_sig(method.def_id)
1161+
.instantiate_identity()
1162+
.inputs()
1163+
.skip_binder()
1164+
// Methods have a `self` arg, so `pos` is actually `+ 1` to match the method call arg.
1165+
.get(pos + 1)
1166+
else {
1167+
return;
1168+
};
1169+
1170+
let cause = ObligationCause::misc(expr.span, self.mir_def_id());
1171+
1172+
enum CanSuggest {
1173+
Yes,
1174+
No,
1175+
Maybe,
1176+
}
1177+
1178+
// Ok, the following is a HACK. We go over every predicate in the `fn` looking for the ones
1179+
// referencing the argument at hand, which is a closure with some bounds. In those, we
1180+
// re-verify that the closure we synthesized still matches the closure bound on the argument
1181+
// (this is likely unneeded) but *more importantly*, we look at the
1182+
// `<ClosureTy as FnOnce>::Output = ClosureRetTy` to confirm that the closure type we
1183+
// synthesized above *will* be accepted by the `where` bound corresponding to this
1184+
// argument. Put a different way, given `counts.iter().max_by_key(|(_, v)| v)`, we check
1185+
// that a new `ClosureTy` of `|(_, v)| { **v }` will be accepted by this method signature:
1186+
// ```
1187+
// fn max_by_key<B: Ord, F>(self, f: F) -> Option<Self::Item>
1188+
// where
1189+
// Self: Sized,
1190+
// F: FnMut(&Self::Item) -> B,
1191+
// ```
1192+
// Sadly, we can't use `ObligationCtxt` to do this, we need to modify things in place.
1193+
let mut can_suggest = CanSuggest::Maybe;
1194+
for pred in preds.predicates {
1195+
match tcx.liberate_late_bound_regions(self.mir_def_id().into(), pred.kind()) {
1196+
ty::ClauseKind::Trait(pred)
1197+
if self.infcx.can_eq(self.param_env, pred.self_ty(), *input)
1198+
&& [
1199+
tcx.lang_items().fn_trait(),
1200+
tcx.lang_items().fn_mut_trait(),
1201+
tcx.lang_items().fn_once_trait(),
1202+
]
1203+
.contains(&Some(pred.def_id())) =>
1204+
{
1205+
// This predicate is an `Fn*` trait and corresponds to the argument with the
1206+
// closure that failed the lifetime check. We verify that the arguments will
1207+
// continue to match (which didn't change, so they should, and this be a no-op).
1208+
let pred = pred.with_self_ty(tcx, closure_ty);
1209+
let o = Obligation::new(tcx, cause.clone(), self.param_env, pred);
1210+
if !self.infcx.predicate_may_hold(&o) {
1211+
// The closure we have doesn't have the right arguments for the trait bound
1212+
can_suggest = CanSuggest::No;
1213+
} else if let CanSuggest::Maybe = can_suggest {
1214+
// The closure has the right arguments
1215+
can_suggest = CanSuggest::Yes;
1216+
}
1217+
}
1218+
ty::ClauseKind::Projection(proj)
1219+
if self.infcx.can_eq(self.param_env, proj.projection_ty.self_ty(), *input)
1220+
&& tcx.lang_items().fn_once_output() == Some(proj.projection_ty.def_id) =>
1221+
{
1222+
// Verify that `<[closure@...] as FnOnce>::Output` matches the expected
1223+
// `Output` from the trait bound on the function called with the `[closure@...]`
1224+
// as argument.
1225+
let proj = proj.with_self_ty(tcx, closure_ty);
1226+
let o = Obligation::new(tcx, cause.clone(), self.param_env, proj);
1227+
if !self.infcx.predicate_may_hold(&o) {
1228+
// Return type doesn't match.
1229+
can_suggest = CanSuggest::No;
1230+
} else if let CanSuggest::Maybe = can_suggest {
1231+
// Return type matches, we can suggest dereferencing the closure's value.
1232+
can_suggest = CanSuggest::Yes;
1233+
}
1234+
}
1235+
_ => {}
1236+
}
1237+
}
1238+
if let CanSuggest::Yes = can_suggest {
1239+
diag.span_suggestion_verbose(
1240+
value.span.shrink_to_lo(),
1241+
"dereference the return value",
1242+
"*".repeat(count),
1243+
Applicability::MachineApplicable,
1244+
);
1245+
}
1246+
}
1247+
10421248
#[allow(rustc::diagnostic_outside_of_impl)]
10431249
#[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable
10441250
fn suggest_move_on_borrowing_closure(&self, diag: &mut Diag<'_>) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//@ run-rustfix
2+
use std::collections::HashMap;
3+
4+
fn main() {
5+
let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3];
6+
7+
let mut counts = HashMap::new();
8+
for num in vs {
9+
let count = counts.entry(num).or_insert(0);
10+
*count += 1;
11+
}
12+
13+
let _ = counts.iter().max_by_key(|(_, v)| **v);
14+
//~^ ERROR lifetime may not live long enough
15+
//~| HELP dereference the return value
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//@ run-rustfix
2+
use std::collections::HashMap;
3+
4+
fn main() {
5+
let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3];
6+
7+
let mut counts = HashMap::new();
8+
for num in vs {
9+
let count = counts.entry(num).or_insert(0);
10+
*count += 1;
11+
}
12+
13+
let _ = counts.iter().max_by_key(|(_, v)| v);
14+
//~^ ERROR lifetime may not live long enough
15+
//~| HELP dereference the return value
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: lifetime may not live long enough
2+
--> $DIR/return-value-lifetime-error.rs:13:47
3+
|
4+
LL | let _ = counts.iter().max_by_key(|(_, v)| v);
5+
| ------- ^ returning this value requires that `'1` must outlive `'2`
6+
| | |
7+
| | return type of closure is &'2 &i32
8+
| has type `&'1 (&i32, &i32)`
9+
|
10+
help: dereference the return value
11+
|
12+
LL | let _ = counts.iter().max_by_key(|(_, v)| **v);
13+
| ++
14+
15+
error: aborting due to 1 previous error
16+

0 commit comments

Comments
 (0)