Skip to content

Access individual fields of tuples, closures and generators on drop. #49822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, ..)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generators are a bit funny -- I'm not sure if upvar_tys is correct for that case?

Maybe @Zoxc can comment how to get the list of fields for a generator type, else I'll dig in later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is correct here. upvar_tys gives you just the upvars for a generator. Pre-transform generators have an additional u32 state field, but it's only used when constructing a generator and it is never accessed.

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`.