Skip to content

Kind-check closure freevars against destination builtin bounds, take 2. #7238

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

Closed
wants to merge 3 commits into from
Closed
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
77 changes: 56 additions & 21 deletions src/librustc/middle/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ pub fn check_crate(tcx: ty::ctxt,
tcx.sess.abort_if_errors();
}

type check_fn = @fn(Context, @freevar_entry);

fn check_struct_safe_for_destructor(cx: Context,
span: span,
struct_did: def_id) {
Expand Down Expand Up @@ -162,30 +160,43 @@ fn check_item(item: @item, (cx, visitor): (Context, visit::vt<Context>)) {
// Yields the appropriate function to check the kind of closed over
// variables. `id` is the node_id for some expression that creates the
// closure.
fn with_appropriate_checker(cx: Context, id: node_id, b: &fn(check_fn)) {
fn check_for_uniq(cx: Context, fv: @freevar_entry) {
fn with_appropriate_checker(cx: Context, id: node_id,
b: &fn(checker: &fn(Context, @freevar_entry))) {
fn check_for_uniq(cx: Context, fv: @freevar_entry, bounds: ty::BuiltinBounds) {
// all captured data must be owned, regardless of whether it is
// moved in or copied in.
let id = ast_util::def_id_of_def(fv.def).node;
let var_t = ty::node_id_to_type(cx.tcx, id);

// FIXME(#3569): Once closure capabilities are restricted based on their
// incoming bounds, make this check conditional based on the bounds.
if !check_owned(cx, var_t, fv.span) { return; }

// check that only immutable variables are implicitly copied in
check_imm_free_var(cx, fv.def, fv.span);

check_freevar_bounds(cx, fv.span, var_t, bounds);
}

fn check_for_box(cx: Context, fv: @freevar_entry) {
fn check_for_box(cx: Context, fv: @freevar_entry, bounds: ty::BuiltinBounds) {
// all captured data must be owned
let id = ast_util::def_id_of_def(fv.def).node;
let var_t = ty::node_id_to_type(cx.tcx, id);

// FIXME(#3569): Once closure capabilities are restricted based on their
// incoming bounds, make this check conditional based on the bounds.
if !check_durable(cx.tcx, var_t, fv.span) { return; }

// check that only immutable variables are implicitly copied in
check_imm_free_var(cx, fv.def, fv.span);

check_freevar_bounds(cx, fv.span, var_t, bounds);
}

fn check_for_block(_cx: Context, _fv: @freevar_entry) {
// no restrictions
fn check_for_block(cx: Context, fv: @freevar_entry, bounds: ty::BuiltinBounds) {
let id = ast_util::def_id_of_def(fv.def).node;
let var_t = ty::node_id_to_type(cx.tcx, id);
check_freevar_bounds(cx, fv.span, var_t, bounds);
}

fn check_for_bare(cx: Context, fv: @freevar_entry) {
Expand All @@ -196,14 +207,14 @@ fn with_appropriate_checker(cx: Context, id: node_id, b: &fn(check_fn)) {

let fty = ty::node_id_to_type(cx.tcx, id);
match ty::get(fty).sty {
ty::ty_closure(ty::ClosureTy {sigil: OwnedSigil, _}) => {
b(check_for_uniq)
ty::ty_closure(ty::ClosureTy {sigil: OwnedSigil, bounds: bounds, _}) => {
b(|cx, fv| check_for_uniq(cx, fv, bounds))
}
ty::ty_closure(ty::ClosureTy {sigil: ManagedSigil, _}) => {
b(check_for_box)
ty::ty_closure(ty::ClosureTy {sigil: ManagedSigil, bounds: bounds, _}) => {
b(|cx, fv| check_for_box(cx, fv, bounds))
}
ty::ty_closure(ty::ClosureTy {sigil: BorrowedSigil, _}) => {
b(check_for_block)
ty::ty_closure(ty::ClosureTy {sigil: BorrowedSigil, bounds: bounds, _}) => {
b(|cx, fv| check_for_block(cx, fv, bounds))
}
ty::ty_bare_fn(_) => {
b(check_for_bare)
Expand Down Expand Up @@ -271,7 +282,7 @@ pub fn check_expr(e: @expr, (cx, v): (Context, visit::vt<Context>)) {
type_param_defs.repr(cx.tcx));
}
for ts.iter().zip(type_param_defs.iter()).advance |(&ty, type_param_def)| {
check_bounds(cx, type_parameter_id, e.span, ty, type_param_def)
check_typaram_bounds(cx, type_parameter_id, e.span, ty, type_param_def)
}
}
}
Expand Down Expand Up @@ -314,7 +325,7 @@ fn check_ty(aty: @Ty, (cx, v): (Context, visit::vt<Context>)) {
let type_param_defs =
ty::lookup_item_type(cx.tcx, did).generics.type_param_defs;
for ts.iter().zip(type_param_defs.iter()).advance |(&ty, type_param_def)| {
check_bounds(cx, aty.id, aty.span, ty, type_param_def)
check_typaram_bounds(cx, aty.id, aty.span, ty, type_param_def)
}
}
}
Expand All @@ -323,19 +334,26 @@ fn check_ty(aty: @Ty, (cx, v): (Context, visit::vt<Context>)) {
visit::visit_ty(aty, (cx, v));
}

pub fn check_bounds(cx: Context,
_type_parameter_id: node_id,
sp: span,
ty: ty::t,
type_param_def: &ty::TypeParameterDef)
pub fn check_builtin_bounds(cx: Context, ty: ty::t, bounds: ty::BuiltinBounds)
-> ty::BuiltinBounds // returns the missing bounds
{
let kind = ty::type_contents(cx.tcx, ty);
let mut missing = ty::EmptyBuiltinBounds();
for type_param_def.bounds.builtin_bounds.each |bound| {
for bounds.each |bound| {
if !kind.meets_bound(cx.tcx, bound) {
missing.add(bound);
}
}
missing
}

pub fn check_typaram_bounds(cx: Context,
_type_parameter_id: node_id,
sp: span,
ty: ty::t,
type_param_def: &ty::TypeParameterDef)
{
let missing = check_builtin_bounds(cx, ty, type_param_def.bounds.builtin_bounds);
if !missing.is_empty() {
cx.tcx.sess.span_err(
sp,
Expand All @@ -346,6 +364,23 @@ pub fn check_bounds(cx: Context,
}
}

pub fn check_freevar_bounds(cx: Context, sp: span, ty: ty::t,
bounds: ty::BuiltinBounds)
{
let missing = check_builtin_bounds(cx, ty, bounds);
if !missing.is_empty() {
cx.tcx.sess.span_err(
sp,
fmt!("cannot capture variable of type `%s`, which does not fulfill \
`%s`, in a bounded closure",
ty_to_str(cx.tcx, ty), missing.user_string(cx.tcx)));
cx.tcx.sess.span_note(
sp,
fmt!("this closure's environment must satisfy `%s`",
bounds.user_string(cx.tcx)));
}
}

fn is_nullary_variant(cx: Context, ex: @expr) -> bool {
match ex.node {
expr_path(_) => {
Expand Down
27 changes: 20 additions & 7 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1850,7 +1850,7 @@ impl TypeContents {
}

pub fn noncopyable(_cx: ctxt) -> TypeContents {
TC_DTOR + TC_BORROWED_MUT + TC_ONCE_CLOSURE + TC_OWNED_CLOSURE +
TC_DTOR + TC_BORROWED_MUT + TC_ONCE_CLOSURE + TC_NONCOPY_TRAIT +
TC_EMPTY_ENUM
}

Expand Down Expand Up @@ -1899,13 +1899,19 @@ impl TypeContents {
}

pub fn needs_drop(&self, cx: ctxt) -> bool {
if self.intersects(TC_NONCOPY_TRAIT) {
// Currently all noncopyable existentials are 2nd-class types
// behind owned pointers. With dynamically-sized types, remove
// this assertion.
assert!(self.intersects(TC_OWNED_POINTER));
}
let tc = TC_MANAGED + TC_DTOR + TypeContents::owned(cx);
self.intersects(tc)
}

pub fn owned(_cx: ctxt) -> TypeContents {
//! Any kind of owned contents.
TC_OWNED_CLOSURE + TC_OWNED_POINTER + TC_OWNED_VEC
TC_OWNED_POINTER + TC_OWNED_VEC
}
}

Expand Down Expand Up @@ -1939,8 +1945,8 @@ static TC_OWNED_POINTER: TypeContents = TypeContents{bits: 0b0000_0000_0010};
/// Contains an owned vector ~[] or owned string ~str
static TC_OWNED_VEC: TypeContents = TypeContents{bits: 0b0000_0000_0100};

/// Contains a ~fn() or a ~Trait, which is non-copyable.
static TC_OWNED_CLOSURE: TypeContents = TypeContents{bits: 0b0000_0000_1000};
/// Contains a non-copyable ~fn() or a ~Trait (NOT a ~fn:Copy() or ~Trait:Copy).
static TC_NONCOPY_TRAIT: TypeContents = TypeContents{bits: 0b0000_0000_1000};

/// Type with a destructor
static TC_DTOR: TypeContents = TypeContents{bits: 0b0000_0001_0000};
Expand Down Expand Up @@ -2055,7 +2061,8 @@ pub fn type_contents(cx: ctxt, ty: t) -> TypeContents {
}

ty_trait(_, _, UniqTraitStore, _) => {
TC_OWNED_CLOSURE
// FIXME(#3569): Make this conditional on the trait's bounds.
TC_NONCOPY_TRAIT + TC_OWNED_POINTER
}

ty_trait(_, _, BoxTraitStore, mutbl) => {
Expand Down Expand Up @@ -2179,7 +2186,9 @@ pub fn type_contents(cx: ctxt, ty: t) -> TypeContents {
match sigil {
ast::BorrowedSigil => TC_BORROWED_POINTER,
ast::ManagedSigil => TC_MANAGED,
ast::OwnedSigil => TC_OWNED_CLOSURE
// FIXME(#3569): Looks like noncopyability should depend
// on the bounds, but I don't think this case ever comes up.
ast::OwnedSigil => TC_NONCOPY_TRAIT + TC_OWNED_POINTER,
}
}

Expand Down Expand Up @@ -2253,7 +2262,11 @@ pub fn type_contents(cx: ctxt, ty: t) -> TypeContents {
let st = match cty.sigil {
ast::BorrowedSigil => TC_BORROWED_POINTER,
ast::ManagedSigil => TC_MANAGED,
ast::OwnedSigil => TC_OWNED_CLOSURE
ast::OwnedSigil => if cty.bounds.contains_elem(BoundCopy) {
TC_OWNED_POINTER
} else {
TC_OWNED_POINTER + TC_NONCOPY_TRAIT
}
};
let rt = borrowed_contents(cty.region, m_imm);
let ot = match cty.onceness {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2013 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.

struct X {
field: @fn:Copy(),
}

fn foo(blk: @fn()) -> X {
return X { field: blk }; //~ ERROR expected bounds `Copy` but found no bounds
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2013 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.

use std::comm;

// If this were legal you could use it to copy captured noncopyables.
// Issue (#2828)

fn foo(blk: ~fn:Copy()) {
blk();
}

fn main() {
let (p,c) = comm::stream();
do foo {
c.send(()); //~ ERROR does not fulfill `Copy`
}
p.recv();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2013 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.

fn bar(blk: &fn:'static()) {
}

fn foo(x: &()) {
do bar {
let _ = x; //~ ERROR does not fulfill `'static`
}
}

fn main() {
}
7 changes: 6 additions & 1 deletion src/test/compile-fail/closure-bounds-subtype.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

fn take_any(_: &fn()) {
}

Expand All @@ -7,6 +8,9 @@ fn take_copyable(_: &fn:Copy()) {
fn take_copyable_owned(_: &fn:Copy+Owned()) {
}

fn take_const_owned(_: &fn:Const+Owned()) {
}

fn give_any(f: &fn()) {
take_any(f);
take_copyable(f); //~ ERROR expected bounds `Copy` but found no bounds
Expand All @@ -29,6 +33,7 @@ fn give_copyable_owned(f: &fn:Copy+Owned()) {
take_any(f);
take_copyable(f);
take_copyable_owned(f);
take_const_owned(f); //~ ERROR expected bounds `Owned+Const` but found bounds `Copy+Owned`
}

fn main() {}
fn main() {}
23 changes: 23 additions & 0 deletions src/test/run-pass/closure-bounds-can-capture-chan.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2013 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.

use std::comm;

fn foo(blk: ~fn:Owned()) {
blk();
}

fn main() {
let (p,c) = comm::stream();
do foo {
c.send(());
}
p.recv();
}
23 changes: 23 additions & 0 deletions src/test/run-pass/closure-bounds-copyable-squiggle-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2013 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.

// Tests correct copying of heap closures' environments.

fn foo(x: ~fn:Copy()) -> (~fn(), ~fn()) {
(copy x, x)
}
fn main() {
let v = ~[~[1,2,3],~[4,5,6]]; // shouldn't get double-freed
let (f1,f2) = do foo {
assert!(v.len() == 2);
};
f1();
f2();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2013 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.

// Tests correct copying of heap closures' environments.

fn bar<T: Copy>(x: T) -> (T, T) {
(copy x, x)
}
fn foo(x: ~fn:Copy()) -> (~fn(), ~fn()) {
bar(x)
}
fn main() {
let v = ~[~[1,2,3],~[4,5,6]]; // shouldn't get double-freed
let (f1,f2) = do foo {
assert!(v.len() == 2);
};
f1();
f2();
}