Skip to content

Treat generators as if they have an arbitrary destructor #49943

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 2 commits into from
May 2, 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: 32 additions & 8 deletions src/librustc_traits/dropck_outlives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,38 @@ fn dtorck_constraint_for_ty<'a, 'gcx, 'tcx>(
.map(|ty| dtorck_constraint_for_ty(tcx, span, for_ty, depth + 1, ty))
.collect(),

ty::TyGenerator(def_id, substs, _) => {
// Note that the interior types are ignored here.
// Any type reachable inside the interior must also be reachable
// through the upvars.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should be kept in some form. It serves as justification for why we can get away with only reporting the upvars instead of actually calculating all the types which may get dropped inside.

Copy link
Member Author

Choose a reason for hiding this comment

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

(okay, addressed this in followup commit)

substs
.upvar_tys(def_id, tcx)
.map(|ty| dtorck_constraint_for_ty(tcx, span, for_ty, depth + 1, ty))
.collect()
ty::TyGenerator(def_id, substs, _interior) => {
// rust-lang/rust#49918: types can be constructed, stored
// in the interior, and sit idle when generator yields
// (and is subsequently dropped).
//
// It would be nice to descend into interior of a
// generator to determine what effects dropping it might
// have (by looking at any drop effects associated with
// its interior).
//
// However, the interior's representation uses things like
// TyGeneratorWitness that explicitly assume they are not
// traversed in such a manner. So instead, we will
// simplify things for now by treating all generators as
// if they were like trait objects, where its upvars must
// all be alive for the generator's (potential)
// destructor.
//
// In particular, skipping over `_interior` is safe
// because any side-effects from dropping `_interior` can
// only take place through references with lifetimes
// derived from lifetimes attached to the upvars, and we
// *do* incorporate the upvars here.

let constraint = DtorckConstraint {
outlives: substs.upvar_tys(def_id, tcx).map(|t| t.into()).collect(),
dtorck_types: vec![],
overflows: vec![],
};
debug!("dtorck_constraint: generator {:?} => {:?}", def_id, constraint);

Ok(constraint)
}

ty::TyAdt(def, substs) => {
Expand Down
38 changes: 27 additions & 11 deletions src/test/ui/generator/borrowing.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,30 @@
error: compilation successful
--> $DIR/borrowing.rs:15:1
error[E0597]: `a` does not live long enough
--> $DIR/borrowing.rs:18:18
|
LL | / fn main() { #![rustc_error] // rust-lang/rust#49855
LL | | let _b = {
LL | | let a = 3;
LL | | unsafe { (|| yield &a).resume() }
... |
LL | | };
LL | | }
| |_^
LL | unsafe { (|| yield &a).resume() }
| ^^^^^^^^^^^^^
| |
| borrowed value does not live long enough
| borrow may end up in a temporary, created here
LL | //~^ ERROR: `a` does not live long enough
LL | };
| -- temporary later dropped here, potentially using the reference
| |
| borrowed value only lives until here

error: aborting due to previous error
error[E0597]: `a` does not live long enough
--> $DIR/borrowing.rs:24:9
|
LL | / || {
LL | | yield &a
LL | | //~^ ERROR: `a` does not live long enough
LL | | }
| |_________^ borrowed value does not live long enough
LL | };
| - borrowed value only lives until here
LL | }
| - borrow later used here, when `_b` is dropped

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0597`.
4 changes: 2 additions & 2 deletions src/test/ui/generator/borrowing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generators, generator_trait, rustc_attrs)]
#![feature(generators, generator_trait)]

use std::ops::Generator;

fn main() { #![rustc_error] // rust-lang/rust#49855
fn main() {
let _b = {
let a = 3;
unsafe { (|| yield &a).resume() }
Expand Down
26 changes: 16 additions & 10 deletions src/test/ui/generator/dropck.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
error: compilation successful
--> $DIR/dropck.rs:16:1
error[E0597]: `ref_` does not live long enough
--> $DIR/dropck.rs:22:11
|
LL | / fn main() { #![rustc_error] // rust-lang/rust#49855
LL | | let (cell, mut gen);
LL | | cell = Box::new(RefCell::new(0));
LL | | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut())));
... |
LL | | // drops the RefCell and then the Ref, leading to use-after-free
LL | | }
| |_^
LL | gen = || {
| ___________^
LL | | // but the generator can use it to drop a `Ref<'a, i32>`.
LL | | let _d = ref_.take(); //~ ERROR `ref_` does not live long enough
LL | | yield;
LL | | };
| |_____^ borrowed value does not live long enough
...
LL | }
| -
| |
| borrowed value only lives until here
| borrow later used here, when `gen` is dropped

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
5 changes: 3 additions & 2 deletions src/test/ui/generator/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generators, generator_trait, box_leak, rustc_attrs)]
#![feature(generators, generator_trait, box_leak)]

use std::cell::RefCell;
use std::ops::Generator;

fn main() { #![rustc_error] // rust-lang/rust#49855
fn main() {
let (cell, mut gen);
cell = Box::new(RefCell::new(0));
let ref_ = Box::leak(Box::new(Some(cell.borrow_mut())));
//~^ ERROR `*cell` does not live long enough [E0597]
// the upvar is the non-dropck `&mut Option<Ref<'a, i32>>`.
gen = || {
// but the generator can use it to drop a `Ref<'a, i32>`.
Expand Down
15 changes: 13 additions & 2 deletions src/test/ui/generator/dropck.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
error[E0597]: `*cell` does not live long enough
--> $DIR/dropck.rs:19:40
|
LL | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut())));
| ^^^^ borrowed value does not live long enough
...
LL | }
| - `*cell` dropped here while still borrowed
|
= note: values in a scope are dropped in the opposite order they are created

error[E0597]: `ref_` does not live long enough
--> $DIR/dropck.rs:23:18
--> $DIR/dropck.rs:24:18
|
LL | gen = || {
| -- capture occurs here
Expand All @@ -12,6 +23,6 @@ LL | }
|
= note: values in a scope are dropped in the opposite order they are created

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

For more information about this error, try `rustc --explain E0597`.
18 changes: 13 additions & 5 deletions src/test/ui/generator/ref-escapes-but-not-over-yield.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
error[E0597]: `b` does not live long enough
--> $DIR/ref-escapes-but-not-over-yield.rs:24:13
|
LL | a = &b;
| ^^ borrowed value does not live long enough
LL | //~^ ERROR `b` does not live long enough
LL | };
| - borrowed value only lives until here
LL | let mut b = move || {
| _________________-
LL | | yield();
LL | | let b = 5;
LL | | a = &b;
| | ^^ borrowed value does not live long enough
LL | | //~^ ERROR `b` does not live long enough
LL | | };
| | -
| | |
| | borrowed value only lives until here
| |_____temporary later dropped here, potentially using the reference
| borrow may end up in a temporary, created here

error: aborting due to previous error

Expand Down