mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #123990 - compiler-errors:suggest-closure-ret-deref, r=oli-obk

Browse Source 
Make `suggest_deref_closure_return` more idiomatic/easier to understand

The only functional change here really is just making it not use a fresh type variable for upvars. I'll point that out in the code.

The rest of the changes are just stylistic, because reading this code was really confusing me (variable names were vague, ways of accessing types were unidiomatic, order of operations was kind of strange, etc).

This is stacked on #123989.

r? oli-obk since you approved #122213
This commit is contained in:
Guillaume Gomez 2024-04-16 15:19:14 +02:00 committed by GitHub
commit 24cdb7ed56
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 63 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

135
compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View File

@ -26,7 +26,6 @@
use rustc_middle::ty::{Region, TyCtxt}; use rustc_middle::ty::{Region, TyCtxt};
use rustc_span::symbol::{kw, Ident}; use rustc_span::symbol::{kw, Ident};
use rustc_span::Span; use rustc_span::Span;
use rustc_trait_selection::infer::type_variable::TypeVariableOrigin;
use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::{Obligation, ObligationCtxt}; use rustc_trait_selection::traits::{Obligation, ObligationCtxt};
@ -813,7 +812,7 @@ fn report_general_error(&self, errci: &ErrorConstraintInfo<'tcx>) -> Diag<'tcx>
self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr); self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr);
self.suggest_adding_lifetime_params(&mut diag, *fr, *outlived_fr); self.suggest_adding_lifetime_params(&mut diag, *fr, *outlived_fr);
self.suggest_move_on_borrowing_closure(&mut diag); self.suggest_move_on_borrowing_closure(&mut diag);
self.suggest_deref_closure_value(&mut diag); self.suggest_deref_closure_return(&mut diag);
diag diag
} }
@ -1048,115 +1047,107 @@ fn suggest_adding_lifetime_params(&self, diag: &mut Diag<'_>, sub: RegionVid, su
/// When encountering a lifetime error caused by the return type of a closure, check the /// When encountering a lifetime error caused by the return type of a closure, check the
/// corresponding trait bound and see if dereferencing the closure return value would satisfy /// corresponding trait bound and see if dereferencing the closure return value would satisfy
/// them. If so, we produce a structured suggestion. /// them. If so, we produce a structured suggestion.
fn suggest_deref_closure_value(&self, diag: &mut Diag<'_>) { fn suggest_deref_closure_return(&self, diag: &mut Diag<'_>) {
let tcx = self.infcx.tcx; let tcx = self.infcx.tcx;
let map = tcx.hir();
// Get the closure return value and type. // Get the closure return value and type.
let body_id = map.body_owned_by(self.mir_def_id()); let closure_def_id = self.mir_def_id();
let body = &map.body(body_id); let hir::Node::Expr(
let value = &body.value.peel_blocks(); closure_expr @ hir::Expr {
let hir::Node::Expr(closure_expr) = tcx.hir_node_by_def_id(self.mir_def_id()) else { kind: hir::ExprKind::Closure(hir::Closure { body, .. }), ..
},
) = tcx.hir_node_by_def_id(closure_def_id)
else {
return; return;
}; };
let fn_call_id = tcx.parent_hir_id(self.mir_hir_id()); let ty::Closure(_, args) = *tcx.type_of(closure_def_id).instantiate_identity().kind()
let hir::Node::Expr(expr) = tcx.hir_node(fn_call_id) else { return }; else {
let def_id = map.enclosing_body_owner(fn_call_id); return;
let tables = tcx.typeck(def_id); };
let Some(return_value_ty) = tables.node_type_opt(value.hir_id) else { return }; let args = args.as_closure();
let return_value_ty = self.infcx.resolve_vars_if_possible(return_value_ty);
// Make sure that the parent expression is a method call.
let parent_expr_id = tcx.parent_hir_id(self.mir_hir_id());
let hir::Node::Expr(
parent_expr @ hir::Expr {
kind: hir::ExprKind::MethodCall(_, rcvr, call_args, _), ..
},
) = tcx.hir_node(parent_expr_id)
else {
return;
};
let typeck_results = tcx.typeck(self.mir_def_id());
// We don't use `ty.peel_refs()` to get the number of `*`s needed to get the root type. // We don't use `ty.peel_refs()` to get the number of `*`s needed to get the root type.
let mut ty = return_value_ty; let liberated_sig = tcx.liberate_late_bound_regions(closure_def_id.to_def_id(), args.sig());
let mut peeled_ty = liberated_sig.output();
let mut count = 0; let mut count = 0;
while let ty::Ref(_, t, _) = ty.kind() { while let ty::Ref(_, ref_ty, _) = *peeled_ty.kind() {
ty = *t; peeled_ty = ref_ty;
count += 1; count += 1;
} }
if !self.infcx.type_is_copy_modulo_regions(self.param_env, ty) { if !self.infcx.type_is_copy_modulo_regions(self.param_env, peeled_ty) {
return; return;
} }
// Build a new closure where the return type is an owned value, instead of a ref. // Build a new closure where the return type is an owned value, instead of a ref.
let Some(ty::Closure(did, args)) =
tables.node_type_opt(closure_expr.hir_id).as_ref().map(|ty| ty.kind())
else {
return;
};
let sig = args.as_closure().sig();
let closure_sig_as_fn_ptr_ty = Ty::new_fn_ptr( let closure_sig_as_fn_ptr_ty = Ty::new_fn_ptr(
tcx, tcx,
sig.map_bound(|s| { ty::Binder::dummy(tcx.mk_fn_sig(
let unsafety = hir::Unsafety::Normal; liberated_sig.inputs().iter().copied(),
use rustc_target::spec::abi; peeled_ty,
tcx.mk_fn_sig( liberated_sig.c_variadic,
[s.inputs()[0]], hir::Unsafety::Normal,
s.output().peel_refs(), rustc_target::spec::abi::Abi::Rust,
s.c_variadic, )),
unsafety,
abi::Abi::Rust,
)
}),
); );
let parent_args = GenericArgs::identity_for_item( let closure_ty = Ty::new_closure(
tcx, tcx,
tcx.typeck_root_def_id(self.mir_def_id().to_def_id()), closure_def_id.to_def_id(),
ty::ClosureArgs::new(
tcx,
ty::ClosureArgsParts {
parent_args: args.parent_args(),
closure_kind_ty: args.kind_ty(),
tupled_upvars_ty: args.tupled_upvars_ty(),
closure_sig_as_fn_ptr_ty,
},
)
.args,
); );
let closure_kind = args.as_closure().kind();
let closure_kind_ty = Ty::from_closure_kind(tcx, closure_kind);
let tupled_upvars_ty = self
.infcx
.next_ty_var(TypeVariableOrigin { param_def_id: None, span: closure_expr.span });
let closure_args = ty::ClosureArgs::new(
tcx,
ty::ClosureArgsParts {
parent_args,
closure_kind_ty,
closure_sig_as_fn_ptr_ty,
tupled_upvars_ty,
},
);
let closure_ty = Ty::new_closure(tcx, *did, closure_args.args);
let closure_ty = tcx.erase_regions(closure_ty);
let hir::ExprKind::MethodCall(_, rcvr, args, _) = expr.kind else { return }; let Some((closure_arg_pos, _)) =
let Some(pos) = args call_args.iter().enumerate().find(|(_, arg)| arg.hir_id == closure_expr.hir_id)
.iter()
.enumerate()
.find(|(_, arg)| arg.hir_id == closure_expr.hir_id)
.map(|(i, _)| i)
else { else {
return; return;
}; };
// The found `Self` type of the method call.
let Some(possible_rcvr_ty) = tables.node_type_opt(rcvr.hir_id) else { return };
let Some(method_def_id) = tables.type_dependent_def_id(expr.hir_id) else { return };
// Get the type for the parameter corresponding to the argument the closure with the // Get the type for the parameter corresponding to the argument the closure with the
// lifetime error we had. // lifetime error we had.
let Some(input) = tcx let Some(method_def_id) = typeck_results.type_dependent_def_id(parent_expr.hir_id) else {
return;
};
let Some(input_arg) = tcx
.fn_sig(method_def_id) .fn_sig(method_def_id)
.instantiate_identity() .skip_binder()
.inputs() .inputs()
.skip_binder() .skip_binder()
// Methods have a `self` arg, so `pos` is actually `+ 1` to match the method call arg. // Methods have a `self` arg, so `pos` is actually `+ 1` to match the method call arg.
.get(pos + 1) .get(closure_arg_pos + 1)
else { else {
return; return;
}; };
// If this isn't a param, then we can't substitute a new closure.
trace!(?input); let ty::Param(closure_param) = input_arg.kind() else { return };
let ty::Param(closure_param) = input.kind() else { return };
// Get the arguments for the found method, only specifying that `Self` is the receiver type. // Get the arguments for the found method, only specifying that `Self` is the receiver type.
let Some(possible_rcvr_ty) = typeck_results.node_type_opt(rcvr.hir_id) else { return };
let args = GenericArgs::for_item(tcx, method_def_id, |param, _| { let args = GenericArgs::for_item(tcx, method_def_id, |param, _| {
if param.index == 0 { if param.index == 0 {
possible_rcvr_ty.into() possible_rcvr_ty.into()
} else if param.index == closure_param.index { } else if param.index == closure_param.index {
closure_ty.into() closure_ty.into()
} else { } else {
self.infcx.var_for_def(expr.span, param) self.infcx.var_for_def(parent_expr.span, param)
} }
}); });
@ -1170,7 +1161,7 @@ fn suggest_deref_closure_value(&self, diag: &mut Diag<'_>) {
if ocx.select_all_or_error().is_empty() { if ocx.select_all_or_error().is_empty() {
diag.span_suggestion_verbose( diag.span_suggestion_verbose(
value.span.shrink_to_lo(), tcx.hir().body(*body).value.peel_blocks().span.shrink_to_lo(),
"dereference the return value", "dereference the return value",
"*".repeat(count), "*".repeat(count),
Applicability::MachineApplicable, Applicability::MachineApplicable,