未验证 提交 5193c5d6 编写于 作者: R Ralf Jung 提交者: GitHub

Rollup merge of #72598 - Aaron1011:feature/fnmut-capture-span, r=nikomatsakis

Display information about captured variable in `FnMut` error

Fixes #69446

When we encounter a region error involving an `FnMut` closure, we
display a specialized error message. However, we currently do not
tell the user which upvar was captured. This makes it difficult to
determine the cause of the error, especially when the closure is large.

This commit records marks constraints involving closure upvars
with `ConstraintCategory::ClosureUpvar`. When we decide to 'blame'
a `ConstraintCategory::Return`, we additionall store
the captured upvar if we found a `ConstraintCategory::ClosureUpvar` in
the path.

When generating an error message, we point to relevant spans if we have
closure upvar information available. We further customize the message if
an `async` closure is being returned, to make it clear that the captured
variable is being returned indirectly.
......@@ -56,6 +56,7 @@ pub const fn from_generator<T>(gen: T) -> impl Future<Output = T::Return>
where
T: Generator<ResumeTy, Yield = ()>,
{
#[rustc_diagnostic_item = "gen_future"]
struct GenFuture<T: Generator<ResumeTy, Yield = ()>>(T);
// We rely on the fact that async/await futures are immovable in order to create
......
......@@ -183,7 +183,7 @@ pub struct ClosureOutlivesRequirement<'tcx> {
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
#[derive(RustcEncodable, RustcDecodable, HashStable)]
pub enum ConstraintCategory {
Return,
Return(ReturnConstraint),
Yield,
UseAsConst,
UseAsStatic,
......@@ -199,6 +199,7 @@ pub enum ConstraintCategory {
SizedBound,
Assignment,
OpaqueType,
ClosureUpvar(hir::HirId),
/// A "boring" constraint (caused by the given location) is one that
/// the user probably doesn't want to see described in diagnostics,
......@@ -216,6 +217,13 @@ pub enum ConstraintCategory {
Internal,
}
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
#[derive(RustcEncodable, RustcDecodable, HashStable)]
pub enum ReturnConstraint {
Normal,
ClosureUpvar(hir::HirId),
}
/// The subject of a `ClosureOutlivesRequirement` -- that is, the thing
/// that must outlive some region.
#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
......
......@@ -824,7 +824,7 @@ pub(in crate::borrow_check) fn report_borrowed_value_does_not_live_long_enough(
category:
category
@
(ConstraintCategory::Return
(ConstraintCategory::Return(_)
| ConstraintCategory::CallArgument
| ConstraintCategory::OpaqueType),
from_closure: false,
......@@ -1147,7 +1147,7 @@ fn try_report_cannot_return_reference_to_local(
opt_place_desc: Option<&String>,
) -> Option<DiagnosticBuilder<'cx>> {
let return_kind = match category {
ConstraintCategory::Return => "return",
ConstraintCategory::Return(_) => "return",
ConstraintCategory::Yield => "yield",
_ => return None,
};
......@@ -1261,7 +1261,7 @@ fn report_escaping_closure_capture(
);
let msg = match category {
ConstraintCategory::Return | ConstraintCategory::OpaqueType => {
ConstraintCategory::Return(_) | ConstraintCategory::OpaqueType => {
format!("{} is returned here", kind)
}
ConstraintCategory::CallArgument => {
......
......@@ -5,9 +5,9 @@
error_reporting::nice_region_error::NiceRegionError,
error_reporting::unexpected_hidden_region_diagnostic, NLLRegionVariableOrigin,
};
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
use rustc_middle::ty::{self, RegionVid, Ty};
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, sym};
use rustc_span::Span;
use crate::util::borrowck_errors;
......@@ -26,7 +26,7 @@ fn description(&self) -> &'static str {
// Must end with a space. Allows for empty names to be provided.
match self {
ConstraintCategory::Assignment => "assignment ",
ConstraintCategory::Return => "returning this value ",
ConstraintCategory::Return(_) => "returning this value ",
ConstraintCategory::Yield => "yielding this value ",
ConstraintCategory::UseAsConst => "using this value as a constant ",
ConstraintCategory::UseAsStatic => "using this value as a static ",
......@@ -37,6 +37,7 @@ fn description(&self) -> &'static str {
ConstraintCategory::SizedBound => "proving this value is `Sized` ",
ConstraintCategory::CopyBound => "copying this value ",
ConstraintCategory::OpaqueType => "opaque type ",
ConstraintCategory::ClosureUpvar(_) => "closure capture ",
ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => "",
......@@ -306,8 +307,8 @@ pub(in crate::borrow_check) fn report_region_error(
};
let diag = match (category, fr_is_local, outlived_fr_is_local) {
(ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(fr) => {
self.report_fnmut_error(&errci)
(ConstraintCategory::Return(kind), true, false) if self.is_closure_fn_mut(fr) => {
self.report_fnmut_error(&errci, kind)
}
(ConstraintCategory::Assignment, true, false)
| (ConstraintCategory::CallArgument, true, false) => {
......@@ -347,7 +348,11 @@ pub(in crate::borrow_check) fn report_region_error(
/// executing...
/// = note: ...therefore, returned references to captured variables will escape the closure
/// ```
fn report_fnmut_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'tcx> {
fn report_fnmut_error(
&self,
errci: &ErrorConstraintInfo,
kind: ReturnConstraint,
) -> DiagnosticBuilder<'tcx> {
let ErrorConstraintInfo { outlived_fr, span, .. } = errci;
let mut diag = self
......@@ -356,19 +361,39 @@ fn report_fnmut_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'
.sess
.struct_span_err(*span, "captured variable cannot escape `FnMut` closure body");
// We should check if the return type of this closure is in fact a closure - in that
// case, we can special case the error further.
let return_type_is_closure =
self.regioncx.universal_regions().unnormalized_output_ty.is_closure();
let message = if return_type_is_closure {
"returns a closure that contains a reference to a captured variable, which then \
escapes the closure body"
} else {
"returns a reference to a captured variable which escapes the closure body"
let mut output_ty = self.regioncx.universal_regions().unnormalized_output_ty;
if let ty::Opaque(def_id, _) = output_ty.kind {
output_ty = self.infcx.tcx.type_of(def_id)
};
debug!("report_fnmut_error: output_ty={:?}", output_ty);
let message = match output_ty.kind {
ty::Closure(_, _) => {
"returns a closure that contains a reference to a captured variable, which then \
escapes the closure body"
}
ty::Adt(def, _) if self.infcx.tcx.is_diagnostic_item(sym::gen_future, def.did) => {
"returns an `async` block that contains a reference to a captured variable, which then \
escapes the closure body"
}
_ => "returns a reference to a captured variable which escapes the closure body",
};
diag.span_label(*span, message);
if let ReturnConstraint::ClosureUpvar(upvar) = kind {
let def_id = match self.regioncx.universal_regions().defining_ty {
DefiningTy::Closure(def_id, _) => def_id,
ty @ _ => bug!("unexpected DefiningTy {:?}", ty),
};
let upvar_def_span = self.infcx.tcx.hir().span(upvar);
let upvar_span = self.infcx.tcx.upvars_mentioned(def_id).unwrap()[&upvar].span;
diag.span_label(upvar_def_span, "variable defined here");
diag.span_label(upvar_span, "variable captured here");
}
match self.give_region_a_name(*outlived_fr).unwrap().source {
RegionNameSource::NamedEarlyBoundRegion(fr_span)
| RegionNameSource::NamedFreeRegion(fr_span)
......@@ -506,7 +531,7 @@ fn report_general_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder
outlived_fr_name.highlight_region_name(&mut diag);
match (category, outlived_fr_is_local, fr_is_local) {
(ConstraintCategory::Return, true, _) => {
(ConstraintCategory::Return(_), true, _) => {
diag.span_label(
*span,
format!(
......
......@@ -216,6 +216,7 @@ fn do_mir_borrowck<'a, 'tcx>(
&mut flow_inits,
&mdpe.move_data,
&borrow_set,
&upvars,
);
// Dump MIR results into a file, if that is enabled. This let us
......@@ -2314,30 +2315,7 @@ fn is_mutable(
/// be `self` in the current MIR, because that is the only time we directly access the fields
/// of a closure type.
pub fn is_upvar_field_projection(&self, place_ref: PlaceRef<'tcx>) -> Option<Field> {
let mut place_projection = place_ref.projection;
let mut by_ref = false;
if let [proj_base @ .., ProjectionElem::Deref] = place_projection {
place_projection = proj_base;
by_ref = true;
}
match place_projection {
[base @ .., ProjectionElem::Field(field, _ty)] => {
let tcx = self.infcx.tcx;
let base_ty = Place::ty_from(place_ref.local, base, self.body(), tcx).ty;
if (base_ty.is_closure() || base_ty.is_generator())
&& (!by_ref || self.upvars[field.index()].by_ref)
{
Some(*field)
} else {
None
}
}
_ => None,
}
path_utils::is_upvar_field_projection(self.infcx.tcx, &self.upvars, place_ref, self.body())
}
}
......
......@@ -39,6 +39,7 @@
renumber,
type_check::{self, MirTypeckRegionConstraints, MirTypeckResults},
universal_regions::UniversalRegions,
Upvar,
};
crate type PoloniusOutput = Output<RustcFacts>;
......@@ -166,6 +167,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>,
move_data: &MoveData<'tcx>,
borrow_set: &BorrowSet<'tcx>,
upvars: &[Upvar],
) -> NllOutput<'tcx> {
let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default());
......@@ -188,6 +190,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
flow_inits,
move_data,
elements,
upvars,
);
if let Some(all_facts) = &mut all_facts {
......
use crate::borrow_check::borrow_set::{BorrowData, BorrowSet, TwoPhaseActivation};
use crate::borrow_check::places_conflict;
use crate::borrow_check::AccessDepth;
use crate::borrow_check::Upvar;
use crate::dataflow::indexes::BorrowIndex;
use rustc_data_structures::graph::dominators::Dominators;
use rustc_middle::mir::BorrowKind;
use rustc_middle::mir::{BasicBlock, Body, Location, Place};
use rustc_middle::mir::{BasicBlock, Body, Field, Location, Place, PlaceRef, ProjectionElem};
use rustc_middle::ty::TyCtxt;
/// Returns `true` if the borrow represented by `kind` is
......@@ -135,3 +136,38 @@ pub(super) fn borrow_of_local_data(place: Place<'_>) -> bool {
// Any errors will be caught on the initial borrow
!place.is_indirect()
}
/// If `place` is a field projection, and the field is being projected from a closure type,
/// then returns the index of the field being projected. Note that this closure will always
/// be `self` in the current MIR, because that is the only time we directly access the fields
/// of a closure type.
pub(crate) fn is_upvar_field_projection(
tcx: TyCtxt<'tcx>,
upvars: &[Upvar],
place_ref: PlaceRef<'tcx>,
body: &Body<'tcx>,
) -> Option<Field> {
let mut place_projection = place_ref.projection;
let mut by_ref = false;
if let [proj_base @ .., ProjectionElem::Deref] = place_projection {
place_projection = proj_base;
by_ref = true;
}
match place_projection {
[base @ .., ProjectionElem::Field(field, _ty)] => {
let base_ty = Place::ty_from(place_ref.local, base, body, tcx).ty;
if (base_ty.is_closure() || base_ty.is_generator())
&& (!by_ref || upvars[field.index()].by_ref)
{
Some(*field)
} else {
None
}
}
_ => None,
}
}
......@@ -12,7 +12,7 @@
use rustc_infer::infer::{InferCtxt, NLLRegionVariableOrigin, RegionVariableOrigin};
use rustc_middle::mir::{
Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements,
ConstraintCategory, Local, Location,
ConstraintCategory, Local, Location, ReturnConstraint,
};
use rustc_middle::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable};
use rustc_span::Span;
......@@ -2017,7 +2017,7 @@ fn check_member_constraints(
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => false,
ConstraintCategory::TypeAnnotation
| ConstraintCategory::Return
| ConstraintCategory::Return(_)
| ConstraintCategory::Yield => true,
_ => constraint_sup_scc != target_scc,
}
......@@ -2042,7 +2042,7 @@ fn check_member_constraints(
if let Some(i) = best_choice {
if let Some(next) = categorized_path.get(i + 1) {
if categorized_path[i].0 == ConstraintCategory::Return
if matches!(categorized_path[i].0, ConstraintCategory::Return(_))
&& next.0 == ConstraintCategory::OpaqueType
{
// The return expression is being influenced by the return type being
......@@ -2050,6 +2050,18 @@ fn check_member_constraints(
return *next;
}
}
if categorized_path[i].0 == ConstraintCategory::Return(ReturnConstraint::Normal) {
let field = categorized_path.iter().find_map(|p| {
if let ConstraintCategory::ClosureUpvar(f) = p.0 { Some(f) } else { None }
});
if let Some(field) = field {
categorized_path[i].0 =
ConstraintCategory::Return(ReturnConstraint::ClosureUpvar(field));
}
}
return categorized_path[i];
}
......
......@@ -55,6 +55,7 @@
location::LocationTable,
member_constraints::MemberConstraintSet,
nll::ToRegionVid,
path_utils,
region_infer::values::{
LivenessValues, PlaceholderIndex, PlaceholderIndices, RegionValueElements,
},
......@@ -62,6 +63,7 @@
renumber,
type_check::free_region_relations::{CreateResult, UniversalRegionRelations},
universal_regions::{DefiningTy, UniversalRegions},
Upvar,
};
macro_rules! span_mirbug {
......@@ -132,6 +134,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
elements: &Rc<RegionValueElements>,
upvars: &[Upvar],
) -> MirTypeckResults<'tcx> {
let implicit_region_bound = infcx.tcx.mk_region(ty::ReVar(universal_regions.fr_fn_body));
let mut constraints = MirTypeckRegionConstraints {
......@@ -162,6 +165,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
borrow_set,
all_facts,
constraints: &mut constraints,
upvars,
};
let opaque_type_values = type_check_internal(
......@@ -577,7 +581,7 @@ fn sanitize_promoted(&mut self, promoted_body: &'b Body<'tcx>, location: Locatio
for constraint in constraints.outlives().iter() {
let mut constraint = *constraint;
constraint.locations = locations;
if let ConstraintCategory::Return
if let ConstraintCategory::Return(_)
| ConstraintCategory::UseAsConst
| ConstraintCategory::UseAsStatic = constraint.category
{
......@@ -827,6 +831,7 @@ struct BorrowCheckContext<'a, 'tcx> {
all_facts: &'a mut Option<AllFacts>,
borrow_set: &'a BorrowSet<'tcx>,
constraints: &'a mut MirTypeckRegionConstraints<'tcx>,
upvars: &'a [Upvar],
}
crate struct MirTypeckResults<'tcx> {
......@@ -1420,7 +1425,7 @@ fn check_stmt(&mut self, body: &Body<'tcx>, stmt: &Statement<'tcx>, location: Lo
ConstraintCategory::UseAsConst
}
} else {
ConstraintCategory::Return
ConstraintCategory::Return(ReturnConstraint::Normal)
}
}
Some(l) if !body.local_decls[l].is_user_variable() => {
......@@ -1703,7 +1708,7 @@ fn check_call_dest(
ConstraintCategory::UseAsConst
}
} else {
ConstraintCategory::Return
ConstraintCategory::Return(ReturnConstraint::Normal)
}
}
Some(l) if !body.local_decls[l].is_user_variable() => {
......@@ -2489,6 +2494,19 @@ fn add_reborrow_constraint(
);
let mut cursor = borrowed_place.projection.as_ref();
let tcx = self.infcx.tcx;
let field = path_utils::is_upvar_field_projection(
tcx,
&self.borrowck_context.upvars,
borrowed_place.as_ref(),
body,
);
let category = if let Some(field) = field {
ConstraintCategory::ClosureUpvar(self.borrowck_context.upvars[field.index()].var_hir_id)
} else {
ConstraintCategory::Boring
};
while let [proj_base @ .., elem] = cursor {
cursor = proj_base;
......@@ -2496,7 +2514,6 @@ fn add_reborrow_constraint(
match elem {
ProjectionElem::Deref => {
let tcx = self.infcx.tcx;
let base_ty = Place::ty_from(borrowed_place.local, proj_base, body, tcx).ty;
debug!("add_reborrow_constraint - base_ty = {:?}", base_ty);
......@@ -2506,7 +2523,7 @@ fn add_reborrow_constraint(
sup: ref_region.to_region_vid(),
sub: borrow_region.to_region_vid(),
locations: location.to_locations(),
category: ConstraintCategory::Boring,
category,
});
match mutbl {
......
// Regression test for issue #69446 - we should display
// which variable is captured
// edition:2018
use core::future::Future;
struct Foo;
impl Foo {
fn foo(&mut self) {}
}
async fn bar<T>(_: impl FnMut() -> T)
where
T: Future<Output = ()>,
{}
fn main() {
let mut x = Foo;
bar(move || async { //~ ERROR captured
x.foo();
});
}
error: captured variable cannot escape `FnMut` closure body
--> $DIR/issue-69446-fnmut-capture.rs:19:17
|
LL | let mut x = Foo;
| ----- variable defined here
LL | bar(move || async {
| _______________-_^
| | |
| | inferred to be a `FnMut` closure
LL | | x.foo();
| | - variable captured here
LL | | });
| |_____^ returns an `async` block that contains a reference to a captured variable, which then escapes the closure body
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
= note: ...therefore, they cannot allow references to captured variables to escape
error: aborting due to previous error
......@@ -21,10 +21,13 @@ LL | *y = 1;
error: captured variable cannot escape `FnMut` closure body
--> $DIR/borrowck-describe-lvalue.rs:264:16
|
LL | let mut x = 0;
| ----- variable defined here
LL | || {
| - inferred to be a `FnMut` closure
LL | / || {
LL | | let y = &mut x;
| | - variable captured here
LL | | &mut x;
LL | | *y = 1;
LL | | drop(y);
......
error: captured variable cannot escape `FnMut` closure body
--> $DIR/issue-40510-1.rs:7:9
|
LL | let mut x: Box<()> = Box::new(());
| ----- variable defined here
LL |
LL | || {
| - inferred to be a `FnMut` closure
LL | &mut x
| ^^^^^^ returns a reference to a captured variable which escapes the closure body
| ^^^^^-
| | |
| | variable captured here
| returns a reference to a captured variable which escapes the closure body
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
= note: ...therefore, they cannot allow references to captured variables to escape
......
error: captured variable cannot escape `FnMut` closure body
--> $DIR/issue-40510-3.rs:7:9
|
LL | let mut x: Vec<()> = Vec::new();
| ----- variable defined here
LL |
LL | || {
| - inferred to be a `FnMut` closure
LL | / || {
LL | | x.push(())
| | - variable captured here
LL | | }
| |_________^ returns a closure that contains a reference to a captured variable, which then escapes the closure body
|
......
error: captured variable cannot escape `FnMut` closure body
--> $DIR/issue-49824.rs:4:9
|
LL | let mut x = 0;
| ----- variable defined here
LL | || {
| - inferred to be a `FnMut` closure
LL | / || {
LL | |
LL | | let _y = &mut x;
| | - variable captured here
LL | | }
| |_________^ returns a closure that contains a reference to a captured variable, which then escapes the closure body
|
......
error: captured variable cannot escape `FnMut` closure body
--> $DIR/issue-53040.rs:3:8
|
LL | let mut v: Vec<()> = Vec::new();
| ----- variable defined here
LL | || &mut v;
| - ^^^^^^ returns a reference to a captured variable which escapes the closure body
| |
| - ^^^^^-
| | | |
| | | variable captured here
| | returns a reference to a captured variable which escapes the closure body
| inferred to be a `FnMut` closure
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
......
error: captured variable cannot escape `FnMut` closure body
--> $DIR/regions-return-ref-to-upvar-issue-17403.rs:7:24
|
LL | let mut x = 0;
| ----- variable defined here
LL | let mut f = || &mut x;
| - ^^^^^^ returns a reference to a captured variable which escapes the closure body
| |
| - ^^^^^-
| | | |
| | | variable captured here
| | returns a reference to a captured variable which escapes the closure body
| inferred to be a `FnMut` closure
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册