change how we report err_out_of_scope borrowck errors

Also, remove the explicit code detecting borrows over a yield.  It
turns out not to be necessary -- any such borrow winds up with a
lifetime that is part of the generator type, and therefore which will
outlive the generator expression itself, which yields an
`err_out_of_scope`. So instead we intercept those errors and display
them in a nicer way.
This commit is contained in:
Niko Matsakis 2017-07-15 06:52:49 -04:00 committed by John Kåre Alsaker
parent 188cdf499f
commit 3fdc3fa1ec
17 changed files with 452 additions and 134 deletions

View File

@ -1143,7 +1143,7 @@ fn region_maps<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
Rc::new(maps)
}
struct YieldFinder(bool);
struct YieldFinder(Option<Span>);
impl<'tcx> Visitor<'tcx> for YieldFinder {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
@ -1156,52 +1156,57 @@ impl<'tcx> Visitor<'tcx> for YieldFinder {
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
if let hir::ExprYield(..) = expr.node {
self.0 = true;
if self.0.is_none() {
self.0 = Some(expr.span);
}
}
intravisit::walk_expr(self, expr);
}
}
pub fn extent_has_yield<'a, 'gcx: 'a+'tcx, 'tcx: 'a>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
extent: CodeExtent) -> bool {
let mut finder = YieldFinder(false);
impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
/// Checks whether the given code extent contains a `yield`. If so,
/// returns `Some(span)` with the span of the "first" yield we find.
pub fn yield_in_extent(self, extent: CodeExtent) -> Option<Span> {
let mut finder = YieldFinder(None);
match extent {
CodeExtent::DestructionScope(node_id) |
CodeExtent::Misc(node_id) => {
match tcx.hir.get(node_id) {
Node::NodeItem(_) |
Node::NodeTraitItem(_) |
Node::NodeImplItem(_) => {
let body = tcx.hir.body(tcx.hir.body_owned_by(node_id));
intravisit::walk_body(&mut finder, body);
match extent {
CodeExtent::DestructionScope(node_id) |
CodeExtent::Misc(node_id) => {
match self.hir.get(node_id) {
Node::NodeItem(_) |
Node::NodeTraitItem(_) |
Node::NodeImplItem(_) => {
let body = self.hir.body(self.hir.body_owned_by(node_id));
intravisit::walk_body(&mut finder, body);
}
Node::NodeExpr(expr) => intravisit::walk_expr(&mut finder, expr),
Node::NodeStmt(stmt) => intravisit::walk_stmt(&mut finder, stmt),
Node::NodeBlock(block) => intravisit::walk_block(&mut finder, block),
_ => bug!(),
}
}
CodeExtent::CallSiteScope(body_id) |
CodeExtent::ParameterScope(body_id) => {
intravisit::walk_body(&mut finder, self.hir.body(body_id))
}
CodeExtent::Remainder(r) => {
if let Node::NodeBlock(block) = self.hir.get(r.block) {
for stmt in &block.stmts[(r.first_statement_index as usize + 1)..] {
intravisit::walk_stmt(&mut finder, stmt);
}
block.expr.as_ref().map(|e| intravisit::walk_expr(&mut finder, e));
} else {
bug!()
}
Node::NodeExpr(expr) => intravisit::walk_expr(&mut finder, expr),
Node::NodeStmt(stmt) => intravisit::walk_stmt(&mut finder, stmt),
Node::NodeBlock(block) => intravisit::walk_block(&mut finder, block),
_ => bug!(),
}
}
CodeExtent::CallSiteScope(body_id) |
CodeExtent::ParameterScope(body_id) => {
intravisit::walk_body(&mut finder, tcx.hir.body(body_id))
}
CodeExtent::Remainder(r) => {
if let Node::NodeBlock(block) = tcx.hir.get(r.block) {
for stmt in &block.stmts[(r.first_statement_index as usize + 1)..] {
intravisit::walk_stmt(&mut finder, stmt);
}
block.expr.as_ref().map(|e| intravisit::walk_expr(&mut finder, e));
} else {
bug!()
}
}
finder.0
}
finder.0
}
pub fn provide(providers: &mut Providers) {

View File

@ -48,9 +48,8 @@ pub fn gather_loans_in_fn<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
move_error_collector: move_error::MoveErrorCollector::new(),
};
let body = glcx.bccx.tcx.hir.body(body);
euv::ExprUseVisitor::new(&mut glcx, bccx.tcx, param_env, &bccx.region_maps, bccx.tables)
.consume_body(body);
.consume_body(bccx.body);
glcx.report_potential_errors();
let GatherLoanCtxt { all_loans, move_data, .. } = glcx;

View File

@ -98,9 +98,8 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) {
let body_id = tcx.hir.body_owned_by(owner_id);
let tables = tcx.typeck_tables_of(owner_def_id);
let region_maps = tcx.region_maps(owner_def_id);
let mut bccx = &mut BorrowckCtxt { tcx, tables, region_maps, owner_def_id };
let body = bccx.tcx.hir.body(body_id);
let body = tcx.hir.body(body_id);
let mut bccx = &mut BorrowckCtxt { tcx, tables, region_maps, owner_def_id, body };
// Eventually, borrowck will always read the MIR, but at the
// moment we do not. So, for now, we always force MIR to be
@ -128,10 +127,9 @@ fn build_borrowck_dataflow_data<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>,
{
// Check the body of fn items.
let tcx = this.tcx;
let body = tcx.hir.body(body_id);
let id_range = {
let mut visitor = intravisit::IdRangeComputingVisitor::new(&tcx.hir);
visitor.visit_body(body);
visitor.visit_body(this.body);
visitor.result()
};
let (all_loans, move_data) =
@ -140,7 +138,7 @@ fn build_borrowck_dataflow_data<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>,
let mut loan_dfcx =
DataFlowContext::new(this.tcx,
"borrowck",
Some(body),
Some(this.body),
cfg,
LoanDataFlowOperator,
id_range,
@ -151,13 +149,13 @@ fn build_borrowck_dataflow_data<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>,
loan.kill_scope.node_id(), loan_idx);
}
loan_dfcx.add_kills_from_flow_exits(cfg);
loan_dfcx.propagate(cfg, body);
loan_dfcx.propagate(cfg, this.body);
let flowed_moves = move_data::FlowedMoveData::new(move_data,
this,
cfg,
id_range,
body);
this.body);
AnalysisData { all_loans: all_loans,
loans: loan_dfcx,
@ -176,7 +174,8 @@ pub fn build_borrowck_dataflow_data_for_fn<'a, 'tcx>(
let owner_def_id = tcx.hir.local_def_id(owner_id);
let tables = tcx.typeck_tables_of(owner_def_id);
let region_maps = tcx.region_maps(owner_def_id);
let mut bccx = BorrowckCtxt { tcx, tables, region_maps, owner_def_id };
let body = tcx.hir.body(body_id);
let mut bccx = BorrowckCtxt { tcx, tables, region_maps, owner_def_id, body };
let dataflow_data = build_borrowck_dataflow_data(&mut bccx, cfg, body_id);
(bccx, dataflow_data)
@ -195,6 +194,8 @@ pub struct BorrowckCtxt<'a, 'tcx: 'a> {
region_maps: Rc<RegionMaps>,
owner_def_id: DefId,
body: &'tcx hir::Body,
}
///////////////////////////////////////////////////////////////////////////
@ -751,9 +752,85 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
format!("`{}`", self.loan_path_to_string(&lp))
}
};
let mut db = struct_span_err!(self.tcx.sess, error_span, E0597, "{} does not live long enough", msg);
let (value_kind, value_msg) = match err.cmt.cat {
// When you have a borrow that lives across a yield,
// that reference winds up captured in the generator
// type. Regionck then constraints it to live as long
// as the generator itself. If that borrow is borrowing
// data owned by the generator, this winds up resulting in
// an `err_out_of_scope` error:
//
// ```
// {
// let g = || {
// let a = &3; // this borrow is forced to ... -+
// yield (); // |
// println!("{}", a); // |
// }; // |
// } <----------------------... live until here --------+
// ```
//
// To detect this case, we look for cases where the
// `super_scope` (lifetime of the value) is within the
// body, but the `sub_scope` is not.
debug!("err_out_of_scope: self.body.is_generator = {:?}",
self.body.is_generator);
let maybe_borrow_across_yield = if self.body.is_generator {
let body_extent = region::CodeExtent::Misc(self.body.id().node_id);
debug!("err_out_of_scope: body_extent = {:?}", body_extent);
debug!("err_out_of_scope: super_scope = {:?}", super_scope);
debug!("err_out_of_scope: sub_scope = {:?}", sub_scope);
match (super_scope, sub_scope) {
(&ty::RegionKind::ReScope(value_extent),
&ty::RegionKind::ReScope(loan_extent)) => {
if {
// value_extent <= body_extent &&
self.region_maps.is_subscope_of(value_extent, body_extent) &&
// body_extent <= loan_extent
self.region_maps.is_subscope_of(body_extent, loan_extent)
} {
// We now know that this is a case
// that fits the bill described above:
// a borrow of something whose scope
// is within the generator, but the
// borrow is for a scope outside the
// generator.
//
// Now look within the scope of the of
// the value being borrowed (in the
// example above, that would be the
// block remainder that starts with
// `let a`) for a yield. We can cite
// that for the user.
self.tcx.yield_in_extent(value_extent)
} else {
None
}
}
_ => None,
}
} else {
None
};
if let Some(yield_span) = maybe_borrow_across_yield {
debug!("err_out_of_scope: opt_yield_span = {:?}", yield_span);
struct_span_err!(self.tcx.sess,
error_span,
E0624,
"borrow may still be in use when generator yields")
.span_label(yield_span, "possible yield occurs here")
.emit();
return;
}
let mut db = struct_span_err!(self.tcx.sess,
error_span,
E0597,
"{} does not live long enough",
msg);
let (value_kind, value_msg) = match err.cmt.cat {
mc::Categorization::Rvalue(..) =>
("temporary value", "temporary value created here"),
_ =>

View File

@ -1183,6 +1183,82 @@ x.x = Some(&y);
```
"##,
E0624: r##"
This error occurs because a borrow in a generator persists across a
yield point.
```compile_fail,E0624
let mut b = || {
let a = &3; // <-- This borrow...
yield (); // ...is still in scope here, when the yield occurs.
println!("{}", a);
};
b.resume();
```
At present, it is not permitted to have a yield that occurs while a
borrow is still in scope. To resolve this error, the borrow must
either be "contained" to a smaller scope that does not overlap the
yield or else eliminated in another way. So, for example, we might
resolve the previous example by removing the borrow and just storing
the integer by value:
```
let mut b = || {
let a = 3;
yield ();
println!("{}", a);
};
b.resume();
```
This is a very simple case, of course. In more complex cases, we may
wish to have more than one reference to the value that was borrowed --
in those cases, something like the `Rc` or `Arc` types may be useful.
This error also frequently arises with iteration:
```compile_fail,E0624
let mut b = || {
let v = vec![1,2,3];
for &x in &v { // <-- borrow of `v` is still in scope...
yield x; // ...when this yield occurs.
}
};
b.resume();
```
Such cases can sometimes be resolved by iterating "by value" (or using
`into_iter()`) to avoid borrowing:
```
let mut b = || {
let v = vec![1,2,3];
for x in v { // <-- Take ownership of the values instead!
yield x; // <-- Now yield is OK.
}
};
b.resume();
```
If taking ownership is not an option, using indices can work too:
```
let mut b = || {
let v = vec![1,2,3];
let len = v.len(); // (*)
for i in 0..len {
let x = v[i]; // (*)
yield x; // <-- Now yield is OK.
}
};
b.resume();
// (*) -- Unfortunately, these temporaries are currently required.
// See <https://github.com/rust-lang/rust/issues/43122>.
```
"##,
}
register_diagnostics! {

View File

@ -12,7 +12,7 @@ use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap};
use rustc::hir::{self, Body, Pat, PatKind, Expr};
use rustc::hir::def_id::DefId;
use rustc::ty::Ty;
use rustc::middle::region::{RegionMaps, CodeExtent, extent_has_yield};
use rustc::middle::region::{RegionMaps, CodeExtent};
use util::nodemap::FxHashSet;
use std::rc::Rc;
use super::FnCtxt;
@ -27,7 +27,7 @@ impl<'a, 'gcx, 'tcx> InteriorVisitor<'a, 'gcx, 'tcx> {
fn record(&mut self, ty: Ty<'tcx>, scope: Option<CodeExtent>, expr: Option<&'tcx Expr>) {
use syntax_pos::DUMMY_SP;
if scope.map(|s| extent_has_yield(self.fcx.tcx, s)).unwrap_or(true) {
if scope.map(|s| self.fcx.tcx.yield_in_extent(s).is_some()).unwrap_or(true) {
if self.fcx.tcx.sess.verbose() {
if let Some(s) = scope {
self.fcx.tcx.sess.span_warn(s.span(&self.fcx.tcx.hir).unwrap_or(DUMMY_SP),

View File

@ -0,0 +1,28 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(generators, generator_trait)]
use std::ops::{State, Generator};
use std::cell::Cell;
fn foo(x: &i32) {
// In this case, a reference to `b` escapes the generator, but not
// because of a yield. We see that there is no yield in the scope of
// `b` and give the more generic error message.
let mut a = &3;
let mut b = move || {
yield();
let b = 5;
a = &b; //~ ERROR
};
}
fn main() { }

View File

@ -0,0 +1,12 @@
error[E0597]: `b` does not live long enough
--> $DIR/ref-escapes-but-not-over-yield.rs:25:5
|
24 | a = &b; //~ ERROR
| - borrow occurs here
25 | };
| ^ `b` dropped here while still borrowed
26 | }
| - borrowed value needs to live until here
error: aborting due to previous error

View File

@ -15,10 +15,10 @@ fn foo(_a: (), _b: &bool) {}
// Some examples that probably *could* be accepted, but which we reject for now.
fn bar() {
|| {
let b = true;
foo(yield, &b);
}; //~ ERROR `b` does not live long enough
|| {
let b = true;
foo(yield, &b); //~ ERROR
};
}
fn main() { }

View File

@ -0,0 +1,10 @@
error[E0624]: borrow may still be in use when generator yields
--> $DIR/yield-in-args-rev.rs:20:15
|
20 | \t\tfoo(yield, &b); //~ ERROR
| \t\t ----- ^
| \t\t |
| \t\t possible yield occurs here
error: aborting due to previous error

View File

@ -13,8 +13,8 @@
fn foo(_b: &bool, _a: ()) {}
fn main() {
|| {
let b = true;
foo(&b, yield);
}; //~ ERROR `b` does not live long enough
|| {
let b = true;
foo(&b, yield); //~ ERROR
};
}

View File

@ -0,0 +1,8 @@
error[E0624]: borrow may still be in use when generator yields
--> $DIR/yield-in-args.rs:18:8
|
18 | \t\tfoo(&b, yield); //~ ERROR
| \t\t ^ ----- possible yield occurs here
error: aborting due to previous error

View File

@ -13,89 +13,16 @@
use std::ops::{State, Generator};
use std::cell::Cell;
fn borrow_local_inline() {
// Not OK to yield with a borrow of a temporary.
//
// (This error occurs because the region shows up in the type of
// `b` and gets extended by region inference.)
let mut b = move || {
let a = &3;
yield();
println!("{}", a);
}; //~ ERROR E0597
b.resume();
}
fn borrow_local_inline_done() {
// No error here -- `a` is not in scope at the point of `yield`.
let mut b = move || {
{
let a = &3;
}
yield();
};
b.resume();
}
fn borrow_local() {
// Not OK to yield with a borrow of a temporary.
//
// (This error occurs because the region shows up in the type of
// `b` and gets extended by region inference.)
let mut b = move || {
let a = 3;
{
let b = &a;
yield();
println!("{}", b);
}
}; //~ ERROR E0597
b.resume();
}
fn reborrow_shared_ref(x: &i32) {
// This is OK -- we have a borrow live over the yield, but it's of
// data that outlives the generator.
let mut b = move || {
let a = &*x;
yield();
println!("{}", a);
};
b.resume();
}
fn reborrow_mutable_ref(x: &mut i32) {
// This is OK -- we have a borrow live over the yield, but it's of
// data that outlives the generator.
let mut b = move || {
let a = &mut *x;
yield();
println!("{}", a);
};
b.resume();
}
fn reborrow_mutable_ref_2(x: &mut i32) {
// ...but not OK to go on using `x`.
let mut b = || {
let a = &mut *x;
yield();
println!("{}", a);
};
println!("{}", x); //~ ERROR E0501
b.resume();
}
fn yield_during_iter_owned_data(x: Vec<i32>) {
// The generator owns `x`, so we error out when yielding with a
// reference to it. This winds up becoming a rather confusing
// regionck error -- in particular, we would freeze with the
// reference in scope, and it doesn't live long enough.
let _b = move || {
for p in &x {
for p in &x { //~ ERROR
yield();
}
}; //~ ERROR E0597
};
}
fn yield_during_iter_borrowed_slice(x: &[i32]) {
@ -137,7 +64,20 @@ fn yield_during_iter_borrowed_slice_4() {
yield p;
}
};
println!("{}", x[0]); //~ ERROR cannot borrow `x` as immutable
println!("{}", x[0]); //~ ERROR
b.resume();
}
fn yield_during_range_iter() {
// Should be OK.
let mut b = || {
let v = vec![1,2,3];
let len = v.len();
for i in 0..len {
let x = v[i];
yield x;
}
};
b.resume();
}

View File

@ -0,0 +1,24 @@
error[E0624]: borrow may still be in use when generator yields
--> $DIR/yield-while-iterating.rs:22:19
|
22 | for p in &x { //~ ERROR
| ^
23 | yield();
| ------- possible yield occurs here
error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
--> $DIR/yield-while-iterating.rs:67:20
|
62 | let mut b = || {
| -- mutable borrow occurs here
63 | for p in &mut x {
| - previous borrow occurs due to use of `x` in closure
...
67 | println!("{}", x[0]); //~ ERROR
| ^ immutable borrow occurs here
68 | b.resume();
69 | }
| - mutable borrow ends here
error: aborting due to 2 previous errors

View File

@ -0,0 +1,56 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(generators, generator_trait)]
use std::ops::{State, Generator};
use std::cell::Cell;
fn borrow_local_inline() {
// Not OK to yield with a borrow of a temporary.
//
// (This error occurs because the region shows up in the type of
// `b` and gets extended by region inference.)
let mut b = move || {
let a = &3; //~ ERROR
yield();
println!("{}", a);
};
b.resume();
}
fn borrow_local_inline_done() {
// No error here -- `a` is not in scope at the point of `yield`.
let mut b = move || {
{
let a = &3;
}
yield();
};
b.resume();
}
fn borrow_local() {
// Not OK to yield with a borrow of a temporary.
//
// (This error occurs because the region shows up in the type of
// `b` and gets extended by region inference.)
let mut b = move || {
let a = 3;
{
let b = &a; //~ ERROR
yield();
println!("{}", b);
}
};
b.resume();
}
fn main() { }

View File

@ -0,0 +1,18 @@
error[E0624]: borrow may still be in use when generator yields
--> $DIR/yield-while-local-borrowed.rs:22:18
|
22 | let a = &3; //~ ERROR
| ^
23 | yield();
| ------- possible yield occurs here
error[E0624]: borrow may still be in use when generator yields
--> $DIR/yield-while-local-borrowed.rs:48:22
|
48 | let b = &a; //~ ERROR
| ^
49 | yield();
| ------- possible yield occurs here
error: aborting due to 2 previous errors

View File

@ -0,0 +1,49 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(generators, generator_trait)]
use std::ops::{State, Generator};
use std::cell::Cell;
fn reborrow_shared_ref(x: &i32) {
// This is OK -- we have a borrow live over the yield, but it's of
// data that outlives the generator.
let mut b = move || {
let a = &*x;
yield();
println!("{}", a);
};
b.resume();
}
fn reborrow_mutable_ref(x: &mut i32) {
// This is OK -- we have a borrow live over the yield, but it's of
// data that outlives the generator.
let mut b = move || {
let a = &mut *x;
yield();
println!("{}", a);
};
b.resume();
}
fn reborrow_mutable_ref_2(x: &mut i32) {
// ...but not OK to go on using `x`.
let mut b = || {
let a = &mut *x;
yield();
println!("{}", a);
};
println!("{}", x); //~ ERROR
b.resume();
}
fn main() { }

View File

@ -0,0 +1,16 @@
error[E0501]: cannot borrow `x` as immutable because previous closure requires unique access
--> $DIR/yield-while-ref-reborrowed.rs:45:20
|
40 | let mut b = || {
| -- closure construction occurs here
41 | let a = &mut *x;
| - previous borrow occurs due to use of `x` in closure
...
45 | println!("{}", x); //~ ERROR
| ^ borrow occurs here
46 | b.resume();
47 | }
| - borrow from closure ends here
error: aborting due to previous error