From 2c7903d59946e6a091bdcbb5f2ff51021c57a1d2 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Mon, 17 Jun 2013 18:02:17 -0400 Subject: [PATCH 1/6] Consume once fns when calling them (#2549). --- src/librustc/middle/moves.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/librustc/middle/moves.rs b/src/librustc/middle/moves.rs index 2217e632d14..4f31107620f 100644 --- a/src/librustc/middle/moves.rs +++ b/src/librustc/middle/moves.rs @@ -183,6 +183,7 @@ struct VisitContext { move_maps: MoveMaps } +#[deriving(Eq)] enum UseMode { Move, // This value or something owned by it is moved. Read // Read no matter what the type. @@ -335,7 +336,27 @@ impl VisitContext { } expr_call(callee, ref args, _) => { // callee(args) - self.use_expr(callee, Read, visitor); + // Figure out whether the called function is consumed. + let mode = match ty::get(ty::expr_ty(self.tcx, callee)).sty { + ty::ty_closure(ref cty) => { + match cty.onceness { + Once => Move, + Many => Read, + } + }, + ty::ty_bare_fn(*) => Read, + ref x => + self.tcx.sess.span_bug(callee.span, + fmt!("non-function type in moves for expr_call: %?", x)), + }; + // Note we're not using consume_expr, which uses type_moves_by_default + // to determine the mode, for this. The reason is that while stack + // closures should be noncopyable, they shouldn't move by default; + // calling a closure should only consume it if it's once. + if mode == Move { + self.move_maps.moves_map.insert(callee.id); + } + self.use_expr(callee, mode, visitor); self.use_fn_args(callee.id, *args, visitor); } From 1496216db6e0ab266df6dcbda31d9fab495afcf9 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Mon, 17 Jun 2013 19:06:36 -0400 Subject: [PATCH 2/6] Permit moving out of captured upvars in once fns. Close #2549. --- .../borrowck/gather_loans/gather_moves.rs | 20 +++++++++++++++++-- src/librustc/middle/mem_categorization.rs | 10 +++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index 5431a0a2998..d982be684a2 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -101,9 +101,7 @@ fn check_is_legal_to_move_from(bccx: @BorrowckCtxt, cmt0: mc::cmt, cmt: mc::cmt) -> bool { match cmt.cat { - mc::cat_stack_upvar(*) | mc::cat_implicit_self(*) | - mc::cat_copied_upvar(*) | mc::cat_deref(_, _, mc::region_ptr(*)) | mc::cat_deref(_, _, mc::gc_ptr(*)) | mc::cat_deref(_, _, mc::unsafe_ptr(*)) => { @@ -114,6 +112,24 @@ fn check_is_legal_to_move_from(bccx: @BorrowckCtxt, false } + // These are separate from the above cases for a better error message. + mc::cat_stack_upvar(*) | + mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, _ }) => { + bccx.span_err( + cmt0.span, + fmt!("cannot move out of %s \ + (unless the destination closure type is `once fn')", + bccx.cmt_to_str(cmt))); + false + } + + // Can move out of captured upvars only if the destination closure + // type is 'once'. 1-shot stack closures emit the copied_upvar form + // (see mem_categorization.rs). + mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Once, _ }) => { + true + } + // It seems strange to allow a move out of a static item, // but what happens in practice is that you have a // reference to a constant with a type that should be diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 14764e7bc37..d020cf651a4 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -78,7 +78,7 @@ pub enum categorization { } #[deriving(Eq)] -struct CopiedUpvar { +pub struct CopiedUpvar { upvar_id: ast::node_id, onceness: ast::Onceness, } @@ -497,9 +497,8 @@ impl mem_categorization_ctxt { let ty = ty::node_id_to_type(self.tcx, fn_node_id); match ty::get(ty).sty { ty::ty_closure(ref closure_ty) => { - let sigil = closure_ty.sigil; - match sigil { - ast::BorrowedSigil => { + match (closure_ty.sigil, closure_ty.onceness) { + (ast::BorrowedSigil, ast::Many) => { let upvar_cmt = self.cat_def(id, span, expr_ty, *inner); @cmt_ { @@ -510,7 +509,8 @@ impl mem_categorization_ctxt { ty:upvar_cmt.ty } } - ast::OwnedSigil | ast::ManagedSigil => { + (ast::BorrowedSigil, ast::Once) | + (ast::OwnedSigil, _) | (ast::ManagedSigil, _) => { // FIXME #2152 allow mutation of moved upvars @cmt_ { id:id, From c1f037e6ac75e05e6bc3f05ec6cab9de8c971e81 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Mon, 17 Jun 2013 19:25:06 -0400 Subject: [PATCH 3/6] Add tests for once functions (#2549). --- .../once-cant-call-twice-on-heap.rs | 29 +++++++++++++++++++ .../once-cant-call-twice-on-stack.rs | 29 +++++++++++++++++++ .../once-cant-copy-stack-once-fn-copy.rs | 20 +++++++++++++ .../once-cant-move-out-of-non-once-on-heap.rs | 29 +++++++++++++++++++ ...once-cant-move-out-of-non-once-on-stack.rs | 29 +++++++++++++++++++ src/test/run-pass/once-move-out-on-heap.rs | 27 +++++++++++++++++ src/test/run-pass/once-move-out-on-stack.rs | 27 +++++++++++++++++ 7 files changed, 190 insertions(+) create mode 100644 src/test/compile-fail/once-cant-call-twice-on-heap.rs create mode 100644 src/test/compile-fail/once-cant-call-twice-on-stack.rs create mode 100644 src/test/compile-fail/once-cant-copy-stack-once-fn-copy.rs create mode 100644 src/test/compile-fail/once-cant-move-out-of-non-once-on-heap.rs create mode 100644 src/test/compile-fail/once-cant-move-out-of-non-once-on-stack.rs create mode 100644 src/test/run-pass/once-move-out-on-heap.rs create mode 100644 src/test/run-pass/once-move-out-on-stack.rs diff --git a/src/test/compile-fail/once-cant-call-twice-on-heap.rs b/src/test/compile-fail/once-cant-call-twice-on-heap.rs new file mode 100644 index 00000000000..4436675d69a --- /dev/null +++ b/src/test/compile-fail/once-cant-call-twice-on-heap.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. +// This program would segfault if it were legal. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: ~once fn()) { + blk(); + blk(); //~ ERROR use of moved value +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); + } +} diff --git a/src/test/compile-fail/once-cant-call-twice-on-stack.rs b/src/test/compile-fail/once-cant-call-twice-on-stack.rs new file mode 100644 index 00000000000..7048f1a7a76 --- /dev/null +++ b/src/test/compile-fail/once-cant-call-twice-on-stack.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. +// This program would segfault if it were legal. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: &once fn()) { + blk(); + blk(); //~ ERROR use of moved value +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); + } +} diff --git a/src/test/compile-fail/once-cant-copy-stack-once-fn-copy.rs b/src/test/compile-fail/once-cant-copy-stack-once-fn-copy.rs new file mode 100644 index 00000000000..6f524c0068b --- /dev/null +++ b/src/test/compile-fail/once-cant-copy-stack-once-fn-copy.rs @@ -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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Though it should be legal to copy a heap-allocated "once fn:Copy", +// stack closures are not deep-copied, so (counterintuitively) it should be +// illegal to copy them. + +fn foo<'r>(blk: &'r once fn:Copy()) -> (&'r once fn:Copy(), &'r once fn:Copy()) { + (copy blk, blk) //~ ERROR copying a value of non-copyable type +} + +fn main() { +} diff --git a/src/test/compile-fail/once-cant-move-out-of-non-once-on-heap.rs b/src/test/compile-fail/once-cant-move-out-of-non-once-on-heap.rs new file mode 100644 index 00000000000..61f158cec27 --- /dev/null +++ b/src/test/compile-fail/once-cant-move-out-of-non-once-on-heap.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. +// This program would segfault if it were legal. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: ~fn()) { + blk(); + blk(); +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); //~ ERROR cannot move out of captured outer variable + } +} diff --git a/src/test/compile-fail/once-cant-move-out-of-non-once-on-stack.rs b/src/test/compile-fail/once-cant-move-out-of-non-once-on-stack.rs new file mode 100644 index 00000000000..42c8b9a9998 --- /dev/null +++ b/src/test/compile-fail/once-cant-move-out-of-non-once-on-stack.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. +// This program would segfault if it were legal. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: &fn()) { + blk(); + blk(); +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); //~ ERROR cannot move out of captured outer variable + } +} diff --git a/src/test/run-pass/once-move-out-on-heap.rs b/src/test/run-pass/once-move-out-on-heap.rs new file mode 100644 index 00000000000..8846abb47fc --- /dev/null +++ b/src/test/run-pass/once-move-out-on-heap.rs @@ -0,0 +1,27 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: ~once fn()) { + blk(); +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); + } +} diff --git a/src/test/run-pass/once-move-out-on-stack.rs b/src/test/run-pass/once-move-out-on-stack.rs new file mode 100644 index 00000000000..21e4a3fa426 --- /dev/null +++ b/src/test/run-pass/once-move-out-on-stack.rs @@ -0,0 +1,27 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Testing guarantees provided by once functions. + +extern mod extra; +use extra::arc; +use std::util; + +fn foo(blk: &once fn()) { + blk(); +} + +fn main() { + let x = arc::ARC(true); + do foo { + assert!(*x.get()); + util::ignore(x); + } +} From 643be38cfe269effdc02111677559f74385b1056 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Wed, 19 Jun 2013 15:53:05 -0400 Subject: [PATCH 4/6] Hide the once-move-out privilege for stack fns behind '-Z once-fns' --- src/librustc/driver/session.rs | 55 ++++++++++-------- src/librustc/middle/mem_categorization.rs | 57 +++++++++++-------- .../once-cant-call-twice-on-stack.rs | 1 + src/test/run-pass/once-move-out-on-stack.rs | 1 + 4 files changed, 66 insertions(+), 48 deletions(-) diff --git a/src/librustc/driver/session.rs b/src/librustc/driver/session.rs index 73e8cfea8ca..e4cb614c948 100644 --- a/src/librustc/driver/session.rs +++ b/src/librustc/driver/session.rs @@ -45,32 +45,33 @@ pub struct config { float_type: float_ty } -pub static verbose: uint = 1 << 0; -pub static time_passes: uint = 1 << 1; -pub static count_llvm_insns: uint = 1 << 2; -pub static time_llvm_passes: uint = 1 << 3; -pub static trans_stats: uint = 1 << 4; -pub static asm_comments: uint = 1 << 5; -pub static no_verify: uint = 1 << 6; -pub static trace: uint = 1 << 7; -pub static coherence: uint = 1 << 8; -pub static borrowck_stats: uint = 1 << 9; -pub static borrowck_note_pure: uint = 1 << 10; -pub static borrowck_note_loan: uint = 1 << 11; -pub static no_landing_pads: uint = 1 << 12; -pub static debug_llvm: uint = 1 << 13; -pub static count_type_sizes: uint = 1 << 14; -pub static meta_stats: uint = 1 << 15; -pub static no_opt: uint = 1 << 16; +pub static verbose: uint = 1 << 0; +pub static time_passes: uint = 1 << 1; +pub static count_llvm_insns: uint = 1 << 2; +pub static time_llvm_passes: uint = 1 << 3; +pub static trans_stats: uint = 1 << 4; +pub static asm_comments: uint = 1 << 5; +pub static no_verify: uint = 1 << 6; +pub static trace: uint = 1 << 7; +pub static coherence: uint = 1 << 8; +pub static borrowck_stats: uint = 1 << 9; +pub static borrowck_note_pure: uint = 1 << 10; +pub static borrowck_note_loan: uint = 1 << 11; +pub static no_landing_pads: uint = 1 << 12; +pub static debug_llvm: uint = 1 << 13; +pub static count_type_sizes: uint = 1 << 14; +pub static meta_stats: uint = 1 << 15; +pub static no_opt: uint = 1 << 16; pub static no_monomorphic_collapse: uint = 1 << 17; -pub static gc: uint = 1 << 18; -pub static jit: uint = 1 << 19; -pub static debug_info: uint = 1 << 20; -pub static extra_debug_info: uint = 1 << 21; -pub static statik: uint = 1 << 22; -pub static print_link_args: uint = 1 << 23; -pub static no_debug_borrows: uint = 1 << 24; -pub static lint_llvm : uint = 1 << 25; +pub static gc: uint = 1 << 18; +pub static jit: uint = 1 << 19; +pub static debug_info: uint = 1 << 20; +pub static extra_debug_info: uint = 1 << 21; +pub static statik: uint = 1 << 22; +pub static print_link_args: uint = 1 << 23; +pub static no_debug_borrows: uint = 1 << 24; +pub static lint_llvm: uint = 1 << 25; +pub static once_fns: uint = 1 << 26; pub fn debugging_opts_map() -> ~[(~str, ~str, uint)] { ~[(~"verbose", ~"in general, enable more debug printouts", verbose), @@ -112,6 +113,9 @@ pub fn debugging_opts_map() -> ~[(~str, ~str, uint)] { (~"lint-llvm", ~"Run the LLVM lint pass on the pre-optimization IR", lint_llvm), + (~"once-fns", + ~"Allow 'once fn' closures to deinitialize captured variables", + once_fns), ] } @@ -293,6 +297,7 @@ impl Session_ { pub fn debug_borrows(@self) -> bool { self.opts.optimize == No && !self.debugging_opt(no_debug_borrows) } + pub fn once_fns(@self) -> bool { self.debugging_opt(once_fns) } // pointless function, now... pub fn str_of(@self, id: ast::ident) -> @str { diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index d020cf651a4..2eedd35c314 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -497,30 +497,41 @@ impl mem_categorization_ctxt { let ty = ty::node_id_to_type(self.tcx, fn_node_id); match ty::get(ty).sty { ty::ty_closure(ref closure_ty) => { - match (closure_ty.sigil, closure_ty.onceness) { - (ast::BorrowedSigil, ast::Many) => { - let upvar_cmt = - self.cat_def(id, span, expr_ty, *inner); - @cmt_ { - id:id, - span:span, - cat:cat_stack_upvar(upvar_cmt), - mutbl:upvar_cmt.mutbl.inherit(), - ty:upvar_cmt.ty - } + // Decide whether to use implicit reference or by copy/move + // capture for the upvar. This, combined with the onceness, + // determines whether the closure can move out of it. + let var_is_refd = match (closure_ty.sigil, closure_ty.onceness) { + // Many-shot stack closures can never move out. + (ast::BorrowedSigil, ast::Many) => true, + // 1-shot stack closures can move out with "-Z once-fns". + (ast::BorrowedSigil, ast::Once) + if self.tcx.sess.once_fns() => false, + (ast::BorrowedSigil, ast::Once) => true, + // Heap closures always capture by copy/move, and can + // move out iff they are once. + (ast::OwnedSigil, _) | (ast::ManagedSigil, _) => false, + + }; + if var_is_refd { + let upvar_cmt = + self.cat_def(id, span, expr_ty, *inner); + @cmt_ { + id:id, + span:span, + cat:cat_stack_upvar(upvar_cmt), + mutbl:upvar_cmt.mutbl.inherit(), + ty:upvar_cmt.ty } - (ast::BorrowedSigil, ast::Once) | - (ast::OwnedSigil, _) | (ast::ManagedSigil, _) => { - // FIXME #2152 allow mutation of moved upvars - @cmt_ { - id:id, - span:span, - cat:cat_copied_upvar(CopiedUpvar { - upvar_id: upvar_id, - onceness: closure_ty.onceness}), - mutbl:McImmutable, - ty:expr_ty - } + } else { + // FIXME #2152 allow mutation of moved upvars + @cmt_ { + id:id, + span:span, + cat:cat_copied_upvar(CopiedUpvar { + upvar_id: upvar_id, + onceness: closure_ty.onceness}), + mutbl:McImmutable, + ty:expr_ty } } } diff --git a/src/test/compile-fail/once-cant-call-twice-on-stack.rs b/src/test/compile-fail/once-cant-call-twice-on-stack.rs index 7048f1a7a76..10877be549e 100644 --- a/src/test/compile-fail/once-cant-call-twice-on-stack.rs +++ b/src/test/compile-fail/once-cant-call-twice-on-stack.rs @@ -11,6 +11,7 @@ // Testing guarantees provided by once functions. // This program would segfault if it were legal. +// compile-flags:-Z once-fns extern mod extra; use extra::arc; use std::util; diff --git a/src/test/run-pass/once-move-out-on-stack.rs b/src/test/run-pass/once-move-out-on-stack.rs index 21e4a3fa426..fb8a4517294 100644 --- a/src/test/run-pass/once-move-out-on-stack.rs +++ b/src/test/run-pass/once-move-out-on-stack.rs @@ -10,6 +10,7 @@ // Testing guarantees provided by once functions. +// compile-flags:-Z once-fns extern mod extra; use extra::arc; use std::util; From 3ba8404c88972f2324ce97fa7e5a433399d946ed Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Wed, 19 Jun 2013 16:03:31 -0400 Subject: [PATCH 5/6] Oops, hide 'unless once fn' error message hint behind -Z once-fns too. --- .../middle/borrowck/gather_loans/gather_moves.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index d982be684a2..b28b056e22b 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -115,11 +115,14 @@ fn check_is_legal_to_move_from(bccx: @BorrowckCtxt, // These are separate from the above cases for a better error message. mc::cat_stack_upvar(*) | mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, _ }) => { + let once_hint = if bccx.tcx.sess.once_fns() { + " (unless the destination closure type is `once fn')" + } else { + "" + }; bccx.span_err( cmt0.span, - fmt!("cannot move out of %s \ - (unless the destination closure type is `once fn')", - bccx.cmt_to_str(cmt))); + fmt!("cannot move out of %s%s", bccx.cmt_to_str(cmt), once_hint)); false } From e0788c7f523dc13609179b8f40c09af5c8668df6 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Sat, 29 Jun 2013 03:54:09 -0400 Subject: [PATCH 6/6] xfail-fast once fn run-pass tests --- src/test/run-pass/once-move-out-on-heap.rs | 2 ++ src/test/run-pass/once-move-out-on-stack.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/test/run-pass/once-move-out-on-heap.rs b/src/test/run-pass/once-move-out-on-heap.rs index 8846abb47fc..38b23fd128d 100644 --- a/src/test/run-pass/once-move-out-on-heap.rs +++ b/src/test/run-pass/once-move-out-on-heap.rs @@ -10,6 +10,8 @@ // Testing guarantees provided by once functions. +// xfail-fast + extern mod extra; use extra::arc; use std::util; diff --git a/src/test/run-pass/once-move-out-on-stack.rs b/src/test/run-pass/once-move-out-on-stack.rs index fb8a4517294..e881f576673 100644 --- a/src/test/run-pass/once-move-out-on-stack.rs +++ b/src/test/run-pass/once-move-out-on-stack.rs @@ -10,6 +10,8 @@ // Testing guarantees provided by once functions. +// xfail-fast + // compile-flags:-Z once-fns extern mod extra; use extra::arc;