Skip to content

Commit

Permalink
Auto merge of #49822 - matthewjasper:dropck-closures, r=nikomatsakis
Browse files Browse the repository at this point in the history
Access individual fields of tuples, closures and generators on drop.

Fixes #48623, by extending the change in #47917 to closures. Also does this for tuples and generators for consistency.

Enums are unchanged because there is now way to borrow `*enum.field` without borrowing `enum.field` at the moment, so any error would be reported when the enum goes out of scope. Unions aren't changed because unions they don't automatically drop their fields.

r? @nikomatsakis
  • Loading branch information
bors committed Apr 27, 2018
2 parents 686d0ae + 902bc0f commit ede7f94
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 40 deletions.
40 changes: 30 additions & 10 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc::ty::maps::Providers;
use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Location, Place};
use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue};
use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind};
use rustc::mir::ClosureRegionRequirements;
use rustc::mir::{ClosureRegionRequirements, Local};

use rustc_data_structures::control_flow_graph::dominators::Dominators;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -729,6 +729,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
erased_drop_place_ty: ty::Ty<'gcx>,
span: Span,
) {
let gcx = self.tcx.global_tcx();
let drop_field = |
mir: &mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
(index, field): (usize, ty::Ty<'gcx>),
| {
let field_ty = gcx.normalize_erasing_regions(mir.param_env, field);
let place = drop_place.clone().field(Field::new(index), field_ty);

mir.visit_terminator_drop(loc, term, flow_state, &place, field_ty, span);
};

match erased_drop_place_ty.sty {
// When a struct is being dropped, we need to check
// whether it has a destructor, if it does, then we can
Expand All @@ -737,22 +748,31 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// destructor but `bar` does not, we will only check for
// borrows of `x.foo` and not `x.bar`. See #47703.
ty::TyAdt(def, substs) if def.is_struct() && !def.has_dtor(self.tcx) => {
for (index, field) in def.all_fields().enumerate() {
let gcx = self.tcx.global_tcx();
let field_ty = field.ty(gcx, substs);
let field_ty = gcx.normalize_erasing_regions(self.param_env, field_ty);
let place = drop_place.clone().field(Field::new(index), field_ty);

self.visit_terminator_drop(loc, term, flow_state, &place, field_ty, span);
}
def.all_fields()
.map(|field| field.ty(gcx, substs))
.enumerate()
.for_each(|field| drop_field(self, field));
}
// Same as above, but for tuples.
ty::TyTuple(tys) => {
tys.iter().cloned().enumerate()
.for_each(|field| drop_field(self, field));
}
// Closures and generators also have disjoint fields, but they are only
// directly accessed in the body of the closure/generator.
ty::TyClosure(def, substs)
| ty::TyGenerator(def, substs, ..)
if *drop_place == Place::Local(Local::new(1)) && !self.mir.upvar_decls.is_empty()
=> {
substs.upvar_tys(def, self.tcx).enumerate()
.for_each(|field| drop_field(self, field));
}
_ => {
// We have now refined the type of the value being
// dropped (potentially) to just the type of a
// subfield; so check whether that field's type still
// "needs drop". If so, we assume that the destructor
// may access any data it likes (i.e., a Deep Write).
let gcx = self.tcx.global_tcx();
if erased_drop_place_ty.needs_drop(gcx, self.param_env) {
self.access_place(
ContextKind::Drop.new(loc),
Expand Down
21 changes: 21 additions & 0 deletions src/test/run-pass/issue-47703-tuple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2012 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(nll)]

struct WithDrop;

impl Drop for WithDrop {
fn drop(&mut self) {}
}

fn consume(x: (&mut (), WithDrop)) -> &mut () { x.0 }

fn main() {}
24 changes: 24 additions & 0 deletions src/test/run-pass/nll/issue-48623-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2012 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(nll)]

struct WithDrop;

impl Drop for WithDrop {
fn drop(&mut self) {}
}

fn reborrow_from_closure(r: &mut ()) -> &mut () {
let d = WithDrop;
(move || { d; &mut *r })()
}

fn main() {}
25 changes: 25 additions & 0 deletions src/test/run-pass/nll/issue-48623-generator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2012 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(nll)]
#![feature(generators, generator_trait)]

struct WithDrop;

impl Drop for WithDrop {
fn drop(&mut self) {}
}

fn reborrow_from_generator(r: &mut ()) {
let d = WithDrop;
move || { d; yield; &mut *r };
}

fn main() {}
32 changes: 2 additions & 30 deletions src/test/ui/generator/yield-while-iterating.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,6 @@ LL | for p in &x { //~ ERROR
LL | yield();
| ------- possible yield occurs here

error[E0597]: borrowed value does not live long enough
--> $DIR/yield-while-iterating.rs:50:17
|
LL | let mut b = || {
| _________________^
LL | | for p in &mut x {
LL | | yield p;
LL | | }
LL | | };
| | ^
| | |
| |_____temporary value only lives until here
| temporary value does not live long enough

error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
--> $DIR/yield-while-iterating.rs:67:20
|
Expand All @@ -35,21 +21,7 @@ LL | println!("{}", x[0]); //~ ERROR
LL | b.resume();
| - borrow later used here

error[E0597]: borrowed value does not live long enough
--> $DIR/yield-while-iterating.rs:62:17
|
LL | let mut b = || {
| _________________^
LL | | for p in &mut x {
LL | | yield p;
LL | | }
LL | | };
| | ^
| | |
| |_____temporary value only lives until here
| temporary value does not live long enough

error: aborting due to 4 previous errors
error: aborting due to 2 previous errors

Some errors occurred: E0502, E0597, E0626.
Some errors occurred: E0502, E0626.
For more information about an error, try `rustc --explain E0502`.

0 comments on commit ede7f94

Please sign in to comment.