Skip to content

Commit c65e119

Browse files
davidtwcodavidtwco
committed
Improve errors for FnMut closures.
This commit improves the errors for `FnMut` closures where a reference to a captured variable is escaping.
1 parent 607243b commit c65e119

File tree

12 files changed

+204
-68
lines changed

12 files changed

+204
-68
lines changed

‎src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs‎

Lines changed: 133 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010

1111
use borrow_check::nll::constraints::{OutlivesConstraint};
1212
use borrow_check::nll::region_infer::RegionInferenceContext;
13+
use borrow_check::nll::region_infer::error_reporting::region_name::RegionNameSource;
1314
use borrow_check::nll::type_check::Locations;
15+
use borrow_check::nll::universal_regions::DefiningTy;
1416
use rustc::hir::def_id::DefId;
1517
use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
1618
use rustc::infer::InferCtxt;
@@ -263,6 +265,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
263265
debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}",
264266
fr_is_local, outlived_fr_is_local, category);
265267
match (category, fr_is_local, outlived_fr_is_local) {
268+
(ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(infcx, fr) =>
269+
self.report_fnmut_error(mir, infcx, mir_def_id, fr, outlived_fr, span,
270+
errors_buffer),
266271
(ConstraintCategory::Assignment, true, false) |
267272
(ConstraintCategory::CallArgument, true, false) =>
268273
self.report_escaping_data_error(mir, infcx, mir_def_id, fr, outlived_fr,
@@ -274,6 +279,75 @@ impl<'tcx> RegionInferenceContext<'tcx> {
274279
};
275280
}
276281

282+
/// Report a specialized error when `FnMut` closures return a reference to a captured variable.
283+
/// This function expects `fr` to be local and `outlived_fr` to not be local.
284+
///
285+
/// ```text
286+
/// error: captured variable cannot escape `FnMut` closure body
287+
/// --> $DIR/issue-53040.rs:15:8
288+
/// |
289+
/// LL | || &mut v;
290+
/// | -- ^^^^^^ creates a reference to a captured variable which escapes the closure body
291+
/// | |
292+
/// | inferred to be a `FnMut` closure
293+
/// |
294+
/// = note: `FnMut` closures only have access to their captured variables while they are
295+
/// executing...
296+
/// = note: ...therefore, returned references to captured variables will escape the closure
297+
/// ```
298+
fn report_fnmut_error(
299+
&self,
300+
mir: &Mir<'tcx>,
301+
infcx: &InferCtxt<'_, '_, 'tcx>,
302+
mir_def_id: DefId,
303+
_fr: RegionVid,
304+
outlived_fr: RegionVid,
305+
span: Span,
306+
errors_buffer: &mut Vec<Diagnostic>,
307+
) {
308+
let mut diag = infcx.tcx.sess.struct_span_err(
309+
span,
310+
"captured variable cannot escape `FnMut` closure body",
311+
);
312+
313+
diag.span_label(
314+
span,
315+
"creates a reference to a captured variable which escapes the closure body",
316+
);
317+
318+
match self.give_region_a_name(infcx, mir, mir_def_id, outlived_fr, &mut 1).source {
319+
RegionNameSource::NamedEarlyBoundRegion(fr_span) |
320+
RegionNameSource::NamedFreeRegion(fr_span) |
321+
RegionNameSource::SynthesizedFreeEnvRegion(fr_span, _) |
322+
RegionNameSource::CannotMatchHirTy(fr_span, _) |
323+
RegionNameSource::MatchedHirTy(fr_span) |
324+
RegionNameSource::MatchedAdtAndSegment(fr_span) |
325+
RegionNameSource::AnonRegionFromUpvar(fr_span, _) |
326+
RegionNameSource::AnonRegionFromOutput(fr_span, _, _) => {
327+
diag.span_label(fr_span, "inferred to be a `FnMut` closure");
328+
},
329+
_ => {},
330+
}
331+
332+
diag.note("`FnMut` closures only have access to their captured variables while they are \
333+
executing...");
334+
diag.note("...therefore, they cannot allow references to captured variables to escape");
335+
336+
diag.buffer(errors_buffer);
337+
}
338+
339+
/// Reports a error specifically for when data is escaping a closure.
340+
///
341+
/// ```text
342+
/// error: borrowed data escapes outside of function
343+
/// --> $DIR/lifetime-bound-will-change-warning.rs:44:5
344+
/// |
345+
/// LL | fn test2<'a>(x: &'a Box<Fn()+'a>) {
346+
/// | - `x` is a reference that is only valid in the function body
347+
/// LL | // but ref_obj will not, so warn.
348+
/// LL | ref_obj(x)
349+
/// | ^^^^^^^^^^ `x` escapes the function body here
350+
/// ```
277351
fn report_escaping_data_error(
278352
&self,
279353
mir: &Mir<'tcx>,
@@ -305,31 +379,46 @@ impl<'tcx> RegionInferenceContext<'tcx> {
305379
span, &format!("borrowed data escapes outside of {}", escapes_from),
306380
);
307381

308-
if let Some((outlived_fr_name, outlived_fr_span)) = outlived_fr_name_and_span {
309-
if let Some(name) = outlived_fr_name {
310-
diag.span_label(
311-
outlived_fr_span,
312-
format!("`{}` is declared here, outside of the {} body", name, escapes_from),
313-
);
314-
}
382+
if let Some((Some(outlived_fr_name), outlived_fr_span)) = outlived_fr_name_and_span {
383+
diag.span_label(
384+
outlived_fr_span,
385+
format!(
386+
"`{}` is declared here, outside of the {} body",
387+
outlived_fr_name, escapes_from
388+
),
389+
);
315390
}
316391

317-
if let Some((fr_name, fr_span)) = fr_name_and_span {
318-
if let Some(name) = fr_name {
319-
diag.span_label(
320-
fr_span,
321-
format!("`{}` is a reference that is only valid in the {} body",
322-
name, escapes_from),
323-
);
392+
if let Some((Some(fr_name), fr_span)) = fr_name_and_span {
393+
diag.span_label(
394+
fr_span,
395+
format!(
396+
"`{}` is a reference that is only valid in the {} body",
397+
fr_name, escapes_from
398+
),
399+
);
324400

325-
diag.span_label(span, format!("`{}` escapes the {} body here",
326-
name, escapes_from));
327-
}
401+
diag.span_label(span, format!("`{}` escapes the {} body here", fr_name, escapes_from));
328402
}
329403

330404
diag.buffer(errors_buffer);
331405
}
332406

407+
/// Reports a region inference error for the general case with named/synthesized lifetimes to
408+
/// explain what is happening.
409+
///
410+
/// ```text
411+
/// error: unsatisfied lifetime constraints
412+
/// --> $DIR/regions-creating-enums3.rs:17:5
413+
/// |
414+
/// LL | fn mk_add_bad1<'a,'b>(x: &'a ast<'a>, y: &'b ast<'b>) -> ast<'a> {
415+
/// | -- -- lifetime `'b` defined here
416+
/// | |
417+
/// | lifetime `'a` defined here
418+
/// LL | ast::add(x, y)
419+
/// | ^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'a` but it
420+
/// | is returning data with lifetime `'b`
421+
/// ```
333422
fn report_general_error(
334423
&self,
335424
mir: &Mir<'tcx>,
@@ -380,6 +469,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
380469
diag.buffer(errors_buffer);
381470
}
382471

472+
/// Adds a suggestion to errors where a `impl Trait` is returned.
473+
///
474+
/// ```text
475+
/// help: to allow this impl Trait to capture borrowed data with lifetime `'1`, add `'_` as
476+
/// a constraint
477+
/// |
478+
/// LL | fn iter_values_anon(&self) -> impl Iterator<Item=u32> + 'a {
479+
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
480+
/// ```
383481
fn add_static_impl_trait_suggestion(
384482
&self,
385483
infcx: &InferCtxt<'_, '_, 'tcx>,
@@ -500,4 +598,22 @@ impl<'tcx> RegionInferenceContext<'tcx> {
500598
.get(&(constraint.sup, constraint.sub));
501599
*opt_span_category.unwrap_or(&(constraint.category, mir.source_info(loc).span))
502600
}
601+
602+
/// Returns `true` if a closure is inferred to be an `FnMut` closure.
603+
crate fn is_closure_fn_mut(
604+
&self,
605+
infcx: &InferCtxt<'_, '_, 'tcx>,
606+
fr: RegionVid,
607+
) -> bool {
608+
if let Some(ty::ReFree(free_region)) = self.to_error_region(fr) {
609+
if let ty::BoundRegion::BrEnv = free_region.bound_region {
610+
if let DefiningTy::Closure(def_id, substs) = self.universal_regions.defining_ty {
611+
let closure_kind_ty = substs.closure_kind_ty(def_id, infcx.tcx);
612+
return Some(ty::ClosureKind::FnMut) == closure_kind_ty.to_opt_closure_kind();
613+
}
614+
}
615+
}
616+
617+
false
618+
}
503619
}

‎src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use syntax_pos::symbol::InternedString;
2727

2828
#[derive(Debug)]
2929
crate struct RegionName {
30-
name: InternedString,
31-
source: RegionNameSource,
30+
crate name: InternedString,
31+
crate source: RegionNameSource,
3232
}
3333

3434
#[derive(Debug)]

‎src/librustc_mir/borrow_check/nll/region_infer/error_reporting/var_name.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
5050
.defining_ty
5151
.upvar_tys(tcx)
5252
.position(|upvar_ty| {
53-
debug!(
54-
"get_upvar_index_for_region: upvar_ty = {:?}",
55-
upvar_ty,
56-
);
57-
tcx.any_free_region_meets(&upvar_ty, |r| r.to_region_vid() == fr)
53+
debug!("get_upvar_index_for_region: upvar_ty={:?}", upvar_ty);
54+
tcx.any_free_region_meets(&upvar_ty, |r| {
55+
let r = r.to_region_vid();
56+
debug!("get_upvar_index_for_region: r={:?} fr={:?}", r, fr);
57+
r == fr
58+
})
5859
})?;
5960

6061
let upvar_ty = self

‎src/test/ui/borrowck/borrowck-describe-lvalue.ast.nll.stderr‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,22 @@ LL | //[mir]~^ ERROR cannot borrow `x` as mutable more than o
2020
LL | *y = 1;
2121
| ------ first borrow later used here
2222

23-
error: unsatisfied lifetime constraints
23+
error: captured variable cannot escape `FnMut` closure body
2424
--> $DIR/borrowck-describe-lvalue.rs:305:16
2525
|
2626
LL | || {
27-
| --
28-
| ||
29-
| |return type of closure is [closure@$DIR/borrowck-describe-lvalue.rs:305:16: 311:18 x:&'2 mut i32]
30-
| lifetime `'1` represents this closure's body
31-
LL | / || { //[mir]~ ERROR unsatisfied lifetime constraints
27+
| - inferred to be a `FnMut` closure
28+
LL | / || { //[mir]~ ERROR captured variable cannot escape `FnMut` closure body
3229
LL | | let y = &mut x;
3330
LL | | &mut x; //[ast]~ ERROR cannot borrow `**x` as mutable more than once at a time
3431
LL | | //[mir]~^ ERROR cannot borrow `x` as mutable more than once at a time
3532
LL | | *y = 1;
3633
LL | | drop(y);
3734
LL | | }
38-
| |_________________^ returning this value requires that `'1` must outlive `'2`
35+
| |_________________^ creates a reference to a captured variable which escapes the closure body
3936
|
40-
= note: closure implements `FnMut`, so references to captured variables can't escape the closure
37+
= note: `FnMut` closures only have access to their captured variables while they are executing...
38+
= note: ...therefore, they cannot allow references to captured variables to escape
4139

4240
error[E0503]: cannot use `f.x` because it was mutably borrowed
4341
--> $DIR/borrowck-describe-lvalue.rs:53:9

‎src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,22 @@ LL | //[mir]~^ ERROR cannot borrow `x` as mutable more than o
2020
LL | *y = 1;
2121
| ------ first borrow later used here
2222

23-
error: unsatisfied lifetime constraints
23+
error: captured variable cannot escape `FnMut` closure body
2424
--> $DIR/borrowck-describe-lvalue.rs:305:16
2525
|
2626
LL | || {
27-
| --
28-
| ||
29-
| |return type of closure is [closure@$DIR/borrowck-describe-lvalue.rs:305:16: 311:18 x:&'2 mut i32]
30-
| lifetime `'1` represents this closure's body
31-
LL | / || { //[mir]~ ERROR unsatisfied lifetime constraints
27+
| - inferred to be a `FnMut` closure
28+
LL | / || { //[mir]~ ERROR captured variable cannot escape `FnMut` closure body
3229
LL | | let y = &mut x;
3330
LL | | &mut x; //[ast]~ ERROR cannot borrow `**x` as mutable more than once at a time
3431
LL | | //[mir]~^ ERROR cannot borrow `x` as mutable more than once at a time
3532
LL | | *y = 1;
3633
LL | | drop(y);
3734
LL | | }
38-
| |_________________^ returning this value requires that `'1` must outlive `'2`
35+
| |_________________^ creates a reference to a captured variable which escapes the closure body
3936
|
40-
= note: closure implements `FnMut`, so references to captured variables can't escape the closure
37+
= note: `FnMut` closures only have access to their captured variables while they are executing...
38+
= note: ...therefore, they cannot allow references to captured variables to escape
4139

4240
error[E0503]: cannot use `f.x` because it was mutably borrowed
4341
--> $DIR/borrowck-describe-lvalue.rs:53:9

‎src/test/ui/borrowck/borrowck-describe-lvalue.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ fn main() {
302302
// FIXME(#49824) -- the free region error below should probably not be there
303303
let mut x = 0;
304304
|| {
305-
|| { //[mir]~ ERROR unsatisfied lifetime constraints
305+
|| { //[mir]~ ERROR captured variable cannot escape `FnMut` closure body
306306
let y = &mut x;
307307
&mut x; //[ast]~ ERROR cannot borrow `**x` as mutable more than once at a time
308308
//[mir]~^ ERROR cannot borrow `x` as mutable more than once at a time

‎src/test/ui/issues/issue-40510-1.nll.stderr‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
1-
error: unsatisfied lifetime constraints
1+
error: captured variable cannot escape `FnMut` closure body
22
--> $DIR/issue-40510-1.rs:18:9
33
|
44
LL | || {
5-
| --
6-
| ||
7-
| |return type of closure is &'2 mut std::boxed::Box<()>
8-
| lifetime `'1` represents this closure's body
5+
| - inferred to be a `FnMut` closure
96
LL | &mut x
10-
| ^^^^^^ returning this value requires that `'1` must outlive `'2`
7+
| ^^^^^^ creates a reference to a captured variable which escapes the closure body
118
|
12-
= note: closure implements `FnMut`, so references to captured variables can't escape the closure
9+
= note: `FnMut` closures only have access to their captured variables while they are executing...
10+
= note: ...therefore, they cannot allow references to captured variables to escape
1311

1412
error: aborting due to previous error
1513

‎src/test/ui/issues/issue-40510-3.nll.stderr‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
1-
error: unsatisfied lifetime constraints
1+
error: captured variable cannot escape `FnMut` closure body
22
--> $DIR/issue-40510-3.rs:18:9
33
|
44
LL | || {
5-
| --
6-
| ||
7-
| |return type of closure is [closure@$DIR/issue-40510-3.rs:18:9: 20:10 x:&'2 mut std::vec::Vec<()>]
8-
| lifetime `'1` represents this closure's body
5+
| - inferred to be a `FnMut` closure
96
LL | / || {
107
LL | | x.push(())
118
LL | | }
12-
| |_________^ returning this value requires that `'1` must outlive `'2`
9+
| |_________^ creates a reference to a captured variable which escapes the closure body
1310
|
14-
= note: closure implements `FnMut`, so references to captured variables can't escape the closure
11+
= note: `FnMut` closures only have access to their captured variables while they are executing...
12+
= note: ...therefore, they cannot allow references to captured variables to escape
1513

1614
error: aborting due to previous error
1715

‎src/test/ui/issues/issue-49824.nll.stderr‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
1-
error: unsatisfied lifetime constraints
1+
error: captured variable cannot escape `FnMut` closure body
22
--> $DIR/issue-49824.rs:22:9
33
|
44
LL | || {
5-
| --
6-
| ||
7-
| |return type of closure is [closure@$DIR/issue-49824.rs:22:9: 24:10 x:&'2 mut i32]
8-
| lifetime `'1` represents this closure's body
5+
| - inferred to be a `FnMut` closure
96
LL | / || {
107
LL | | let _y = &mut x;
118
LL | | }
12-
| |_________^ returning this value requires that `'1` must outlive `'2`
9+
| |_________^ creates a reference to a captured variable which escapes the closure body
1310
|
14-
= note: closure implements `FnMut`, so references to captured variables can't escape the closure
11+
= note: `FnMut` closures only have access to their captured variables while they are executing...
12+
= note: ...therefore, they cannot allow references to captured variables to escape
1513

1614
error: aborting due to previous error
1715

‎src/test/ui/nll/issue-53040.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(nll)]
12+
13+
fn main() {
14+
let mut v: Vec<()> = Vec::new();
15+
|| &mut v;
16+
}

0 commit comments

Comments
 (0)