Skip to content

Commit 132cfcd

Browse files
borsbors
committed
auto merge of #7363 : bblum/rust/soundness, r=nikomatsakis
The commit f9a5453 is meant to be a temporary hold-over. Whether or not there is added a way for the compiler to "implicitly borrow" stack closures in this way, there should be a codegen optimization that prevents having to traverse possibly-very-many function pointers to find the function you ultimately wanted to call. I tried to separate out the changes so this particular commit could be straight-up reverted if auto-borrowing happens in the future. r? @nikomatsakis
2 parents c80e3ba + d7544fe commit 132cfcd

40 files changed

+366
-640
lines changed

‎src/libextra/fun_treemap.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ pub fn traverse<K, V: Copy>(m: Treemap<K, V>, f: &fn(&K, &V)) {
6666
// matches to me, so I changed it. but that may be a
6767
// de-optimization -- tjc
6868
Node(@ref k, @ref v, left, right) => {
69-
traverse(left, f);
69+
traverse(left, |k,v| f(k,v));
7070
f(k, v);
71-
traverse(right, f);
71+
traverse(right, |k,v| f(k,v));
7272
}
7373
}
7474
}

‎src/libextra/rope.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,7 @@ pub mod node {
10781078

10791079
pub fn loop_chars(node: @Node, it: &fn(c: char) -> bool) -> bool {
10801080
return loop_leaves(node,|leaf| {
1081-
leaf.content.slice(leaf.byte_offset, leaf.byte_len).iter().all(it)
1081+
leaf.content.slice(leaf.byte_offset, leaf.byte_len).iter().all(|c| it(c))
10821082
});
10831083
}
10841084

@@ -1101,7 +1101,7 @@ pub mod node {
11011101
loop {
11021102
match (*current) {
11031103
Leaf(x) => return it(x),
1104-
Concat(ref x) => if loop_leaves(x.left, it) { //non tail call
1104+
Concat(ref x) => if loop_leaves(x.left, |l| it(l)) { //non tail call
11051105
current = x.right; //tail call
11061106
} else {
11071107
return false;

‎src/libextra/sort.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ pub fn merge_sort<T:Copy>(v: &[T], le: Le<T>) -> ~[T] {
4242
let mid = v_len / 2 + begin;
4343
let a = (begin, mid);
4444
let b = (mid, end);
45-
return merge(le, merge_sort_(v, a, le), merge_sort_(v, b, le));
45+
return merge(|x,y| le(x,y), merge_sort_(v, a, |x,y| le(x,y)),
46+
merge_sort_(v, b, |x,y| le(x,y)));
4647
}
4748

4849
fn merge<T:Copy>(le: Le<T>, a: &[T], b: &[T]) -> ~[T] {
@@ -83,10 +84,10 @@ fn qsort<T>(arr: &mut [T], left: uint,
8384
right: uint, compare_func: Le<T>) {
8485
if right > left {
8586
let pivot = (left + right) / 2u;
86-
let new_pivot = part::<T>(arr, left, right, pivot, compare_func);
87+
let new_pivot = part::<T>(arr, left, right, pivot, |x,y| compare_func(x,y));
8788
if new_pivot != 0u {
8889
// Need to do this check before recursing due to overflow
89-
qsort::<T>(arr, left, new_pivot - 1u, compare_func);
90+
qsort::<T>(arr, left, new_pivot - 1u, |x,y| compare_func(x,y));
9091
}
9192
qsort::<T>(arr, new_pivot + 1u, right, compare_func);
9293
}
@@ -1202,7 +1203,7 @@ mod big_tests {
12021203

12031204
struct LVal<'self> {
12041205
val: uint,
1205-
key: &'self fn(@uint),
1206+
key: &'self fn:Copy(@uint),
12061207
}
12071208

12081209
#[unsafe_destructor]

‎src/libextra/sync.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,9 @@ impl RWlock {
563563
(&self.order_lock).acquire();
564564
do (&self.access_lock).access_waitqueue {
565565
(&self.order_lock).release();
566-
task::rekillable(blk)
566+
do task::rekillable {
567+
blk()
568+
}
567569
}
568570
}
569571
}
@@ -1182,12 +1184,12 @@ mod tests {
11821184
Write => x.write(blk),
11831185
Downgrade =>
11841186
do x.write_downgrade |mode| {
1185-
(&mode).write(blk);
1187+
do mode.write { blk() };
11861188
},
11871189
DowngradeRead =>
11881190
do x.write_downgrade |mode| {
11891191
let mode = x.downgrade(mode);
1190-
(&mode).read(blk);
1192+
do mode.read { blk() };
11911193
},
11921194
}
11931195
}
@@ -1340,10 +1342,10 @@ mod tests {
13401342
fn lock_cond(x: &RWlock, downgrade: bool, blk: &fn(c: &Condvar)) {
13411343
if downgrade {
13421344
do x.write_downgrade |mode| {
1343-
(&mode).write_cond(blk)
1345+
do mode.write_cond |c| { blk(c) }
13441346
}
13451347
} else {
1346-
x.write_cond(blk)
1348+
do x.write_cond |c| { blk(c) }
13471349
}
13481350
}
13491351
let x = ~RWlock();

‎src/libextra/test.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ impl BenchHarness {
678678
679679
// Initial bench run to get ballpark figure.
680680
let mut n = 1_u64;
681-
self.bench_n(n, f);
681+
self.bench_n(n, |x| f(x));
682682
683683
while n < 1_000_000_000 &&
684684
self.ns_elapsed() < 1_000_000_000 {
@@ -694,7 +694,7 @@ impl BenchHarness {
694694
695695
n = u64::max(u64::min(n+n/2, 100*last), last+1);
696696
n = round_up(n);
697-
self.bench_n(n, f);
697+
self.bench_n(n, |x| f(x));
698698
}
699699
}
700700
@@ -714,7 +714,7 @@ impl BenchHarness {
714714
magnitude * 2);
715715
716716
let samples = do vec::from_fn(n_samples) |_| {
717-
self.bench_n(n_iter as u64, f);
717+
self.bench_n(n_iter as u64, |x| f(x));
718718
self.ns_per_iter() as f64
719719
};
720720

‎src/libextra/treemap.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -511,14 +511,14 @@ impl<K: TotalOrd, V> TreeNode<K, V> {
511511

512512
fn each<'r, K: TotalOrd, V>(node: &'r Option<~TreeNode<K, V>>,
513513
f: &fn(&'r K, &'r V) -> bool) -> bool {
514-
node.iter().advance(|x| each(&x.left, f) && f(&x.key, &x.value) &&
515-
each(&x.right, f))
514+
node.iter().advance(|x| each(&x.left, |k,v| f(k,v)) && f(&x.key, &x.value) &&
515+
each(&x.right, |k,v| f(k,v)))
516516
}
517517

518518
fn each_reverse<'r, K: TotalOrd, V>(node: &'r Option<~TreeNode<K, V>>,
519519
f: &fn(&'r K, &'r V) -> bool) -> bool {
520-
node.iter().advance(|x| each_reverse(&x.right, f) && f(&x.key, &x.value) &&
521-
each_reverse(&x.left, f))
520+
node.iter().advance(|x| each_reverse(&x.right, |k,v| f(k,v)) && f(&x.key, &x.value) &&
521+
each_reverse(&x.left, |k,v| f(k,v)))
522522
}
523523

524524
fn mutate_values<'r, K: TotalOrd, V>(node: &'r mut Option<~TreeNode<K, V>>,
@@ -527,9 +527,9 @@ fn mutate_values<'r, K: TotalOrd, V>(node: &'r mut Option<~TreeNode<K, V>>,
527527
match *node {
528528
Some(~TreeNode{key: ref key, value: ref mut value, left: ref mut left,
529529
right: ref mut right, _}) => {
530-
if !mutate_values(left, f) { return false }
530+
if !mutate_values(left, |k,v| f(k,v)) { return false }
531531
if !f(key, value) { return false }
532-
if !mutate_values(right, f) { return false }
532+
if !mutate_values(right, |k,v| f(k,v)) { return false }
533533
}
534534
None => return false
535535
}

‎src/libextra/workcache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ struct WorkKey {
107107
impl to_bytes::IterBytes for WorkKey {
108108
#[inline]
109109
fn iter_bytes(&self, lsb0: bool, f: to_bytes::Cb) -> bool {
110-
self.kind.iter_bytes(lsb0, f) && self.name.iter_bytes(lsb0, f)
110+
self.kind.iter_bytes(lsb0, |b| f(b)) && self.name.iter_bytes(lsb0, |b| f(b))
111111
}
112112
}
113113

‎src/librust/rust.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ impl ValidUsage {
5757
}
5858

5959
enum Action<'self> {
60-
Call(&'self fn(args: &[~str]) -> ValidUsage),
61-
CallMain(&'static str, &'self fn()),
60+
Call(&'self fn:Copy(args: &[~str]) -> ValidUsage),
61+
CallMain(&'static str, &'self fn:Copy()),
6262
}
6363

6464
enum UsageSource<'self> {
6565
UsgStr(&'self str),
66-
UsgCall(&'self fn()),
66+
UsgCall(&'self fn:Copy()),
6767
}
6868

6969
struct Command<'self> {

‎src/librustc/metadata/filesearch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ pub fn mk_filesearch(maybe_sysroot: &Option<@Path>,
4848
debug!("filesearch: searching additional lib search paths [%?]",
4949
self.addl_lib_search_paths.len());
5050
// a little weird
51-
self.addl_lib_search_paths.iter().advance(f);
51+
self.addl_lib_search_paths.iter().advance(|path| f(path));
5252

5353
debug!("filesearch: searching target lib path");
5454
if !f(&make_target_lib_path(self.sysroot,

‎src/librustc/metadata/tydecode.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,11 @@ fn parse_trait_store(st: &mut PState) -> ty::TraitStore {
189189
fn parse_substs(st: &mut PState, conv: conv_did) -> ty::substs {
190190
let self_r = parse_opt(st, |st| parse_region(st) );
191191

192-
let self_ty = parse_opt(st, |st| parse_ty(st, conv) );
192+
let self_ty = parse_opt(st, |st| parse_ty(st, |x,y| conv(x,y)) );
193193

194194
assert_eq!(next(st), '[');
195195
let mut params: ~[ty::t] = ~[];
196-
while peek(st) != ']' { params.push(parse_ty(st, conv)); }
196+
while peek(st) != ']' { params.push(parse_ty(st, |x,y| conv(x,y))); }
197197
st.pos = st.pos + 1u;
198198

199199
return ty::substs {
@@ -270,8 +270,8 @@ fn parse_str(st: &mut PState, term: char) -> ~str {
270270
}
271271

272272
fn parse_trait_ref(st: &mut PState, conv: conv_did) -> ty::TraitRef {
273-
let def = parse_def(st, NominalType, conv);
274-
let substs = parse_substs(st, conv);
273+
let def = parse_def(st, NominalType, |x,y| conv(x,y));
274+
let substs = parse_substs(st, |x,y| conv(x,y));
275275
ty::TraitRef {def_id: def, substs: substs}
276276
}
277277

@@ -301,18 +301,18 @@ fn parse_ty(st: &mut PState, conv: conv_did) -> ty::t {
301301
'c' => return ty::mk_char(),
302302
't' => {
303303
assert_eq!(next(st), '[');
304-
let def = parse_def(st, NominalType, conv);
305-
let substs = parse_substs(st, conv);
304+
let def = parse_def(st, NominalType, |x,y| conv(x,y));
305+
let substs = parse_substs(st, |x,y| conv(x,y));
306306
assert_eq!(next(st), ']');
307307
return ty::mk_enum(st.tcx, def, substs);
308308
}
309309
'x' => {
310310
assert_eq!(next(st), '[');
311-
let def = parse_def(st, NominalType, conv);
312-
let substs = parse_substs(st, conv);
311+
let def = parse_def(st, NominalType, |x,y| conv(x,y));
312+
let substs = parse_substs(st, |x,y| conv(x,y));
313313
let store = parse_trait_store(st);
314314
let mt = parse_mutability(st);
315-
let bounds = parse_bounds(st, conv);
315+
let bounds = parse_bounds(st, |x,y| conv(x,y));
316316
assert_eq!(next(st), ']');
317317
return ty::mk_trait(st.tcx, def, substs, store, mt, bounds.builtin_bounds);
318318
}
@@ -346,7 +346,7 @@ fn parse_ty(st: &mut PState, conv: conv_did) -> ty::t {
346346
'T' => {
347347
assert_eq!(next(st), '[');
348348
let mut params = ~[];
349-
while peek(st) != ']' { params.push(parse_ty(st, conv)); }
349+
while peek(st) != ']' { params.push(parse_ty(st, |x,y| conv(x,y))); }
350350
st.pos = st.pos + 1u;
351351
return ty::mk_tup(st.tcx, params);
352352
}
@@ -380,15 +380,15 @@ fn parse_ty(st: &mut PState, conv: conv_did) -> ty::t {
380380
}
381381
}
382382
'"' => {
383-
let _ = parse_def(st, TypeWithId, conv);
384-
let inner = parse_ty(st, conv);
383+
let _ = parse_def(st, TypeWithId, |x,y| conv(x,y));
384+
let inner = parse_ty(st, |x,y| conv(x,y));
385385
inner
386386
}
387387
'B' => ty::mk_opaque_box(st.tcx),
388388
'a' => {
389389
assert_eq!(next(st), '[');
390-
let did = parse_def(st, NominalType, conv);
391-
let substs = parse_substs(st, conv);
390+
let did = parse_def(st, NominalType, |x,y| conv(x,y));
391+
let substs = parse_substs(st, |x,y| conv(x,y));
392392
assert_eq!(next(st), ']');
393393
return ty::mk_struct(st.tcx, did, substs);
394394
}
@@ -473,8 +473,8 @@ fn parse_closure_ty(st: &mut PState, conv: conv_did) -> ty::ClosureTy {
473473
let purity = parse_purity(next(st));
474474
let onceness = parse_onceness(next(st));
475475
let region = parse_region(st);
476-
let bounds = parse_bounds(st, conv);
477-
let sig = parse_sig(st, conv);
476+
let bounds = parse_bounds(st, |x,y| conv(x,y));
477+
let sig = parse_sig(st, |x,y| conv(x,y));
478478
ty::ClosureTy {
479479
purity: purity,
480480
sigil: sigil,
@@ -500,7 +500,7 @@ fn parse_sig(st: &mut PState, conv: conv_did) -> ty::FnSig {
500500
assert_eq!(next(st), '[');
501501
let mut inputs = ~[];
502502
while peek(st) != ']' {
503-
inputs.push(parse_ty(st, conv));
503+
inputs.push(parse_ty(st, |x,y| conv(x,y)));
504504
}
505505
st.pos += 1u; // eat the ']'
506506
let ret_ty = parse_ty(st, conv);
@@ -544,8 +544,8 @@ pub fn parse_type_param_def_data(data: &[u8], start: uint,
544544
}
545545

546546
fn parse_type_param_def(st: &mut PState, conv: conv_did) -> ty::TypeParameterDef {
547-
ty::TypeParameterDef {def_id: parse_def(st, NominalType, conv),
548-
bounds: @parse_bounds(st, conv)}
547+
ty::TypeParameterDef {def_id: parse_def(st, NominalType, |x,y| conv(x,y)),
548+
bounds: @parse_bounds(st, |x,y| conv(x,y))}
549549
}
550550

551551
fn parse_bounds(st: &mut PState, conv: conv_did) -> ty::ParamBounds {
@@ -571,7 +571,7 @@ fn parse_bounds(st: &mut PState, conv: conv_did) -> ty::ParamBounds {
571571
param_bounds.builtin_bounds.add(ty::BoundSized);
572572
}
573573
'I' => {
574-
param_bounds.trait_bounds.push(@parse_trait_ref(st, conv));
574+
param_bounds.trait_bounds.push(@parse_trait_ref(st, |x,y| conv(x,y)));
575575
}
576576
'.' => {
577577
return param_bounds;

‎src/librustc/middle/borrowck/move_data.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ impl MoveData {
411411

412412
let mut p = self.path(index).first_child;
413413
while p != InvalidMovePathIndex {
414-
if !self.each_extending_path(p, f) {
414+
if !self.each_extending_path(p, |x| f(x)) {
415415
return false;
416416
}
417417
p = self.path(p).next_sibling;

‎src/librustc/middle/kind.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ fn with_appropriate_checker(cx: Context, id: node_id,
171171
// check that only immutable variables are implicitly copied in
172172
check_imm_free_var(cx, fv.def, fv.span);
173173

174-
check_freevar_bounds(cx, fv.span, var_t, bounds);
174+
check_freevar_bounds(cx, fv.span, var_t, bounds, None);
175175
}
176176

177177
fn check_for_box(cx: Context, fv: &freevar_entry, bounds: ty::BuiltinBounds) {
@@ -182,13 +182,18 @@ fn with_appropriate_checker(cx: Context, id: node_id,
182182
// check that only immutable variables are implicitly copied in
183183
check_imm_free_var(cx, fv.def, fv.span);
184184

185-
check_freevar_bounds(cx, fv.span, var_t, bounds);
185+
check_freevar_bounds(cx, fv.span, var_t, bounds, None);
186186
}
187187

188-
fn check_for_block(cx: Context, fv: &freevar_entry, bounds: ty::BuiltinBounds) {
188+
fn check_for_block(cx: Context, fv: &freevar_entry,
189+
bounds: ty::BuiltinBounds, region: ty::Region) {
189190
let id = ast_util::def_id_of_def(fv.def).node;
190191
let var_t = ty::node_id_to_type(cx.tcx, id);
191-
check_freevar_bounds(cx, fv.span, var_t, bounds);
192+
// FIXME(#3569): Figure out whether the implicit borrow is actually
193+
// mutable. Currently we assume all upvars are referenced mutably.
194+
let implicit_borrowed_type = ty::mk_mut_rptr(cx.tcx, region, var_t);
195+
check_freevar_bounds(cx, fv.span, implicit_borrowed_type,
196+
bounds, Some(var_t));
192197
}
193198

194199
fn check_for_bare(cx: Context, fv: @freevar_entry) {
@@ -205,8 +210,9 @@ fn with_appropriate_checker(cx: Context, id: node_id,
205210
ty::ty_closure(ty::ClosureTy {sigil: ManagedSigil, bounds: bounds, _}) => {
206211
b(|cx, fv| check_for_box(cx, fv, bounds))
207212
}
208-
ty::ty_closure(ty::ClosureTy {sigil: BorrowedSigil, bounds: bounds, _}) => {
209-
b(|cx, fv| check_for_block(cx, fv, bounds))
213+
ty::ty_closure(ty::ClosureTy {sigil: BorrowedSigil, bounds: bounds,
214+
region: region, _}) => {
215+
b(|cx, fv| check_for_block(cx, fv, bounds, region))
210216
}
211217
ty::ty_bare_fn(_) => {
212218
b(check_for_bare)
@@ -366,14 +372,21 @@ pub fn check_typaram_bounds(cx: Context,
366372
}
367373

368374
pub fn check_freevar_bounds(cx: Context, sp: span, ty: ty::t,
369-
bounds: ty::BuiltinBounds)
375+
bounds: ty::BuiltinBounds, referenced_ty: Option<ty::t>)
370376
{
371377
do check_builtin_bounds(cx, ty, bounds) |missing| {
372-
cx.tcx.sess.span_err(
373-
sp,
374-
fmt!("cannot capture variable of type `%s`, which does not fulfill \
375-
`%s`, in a bounded closure",
376-
ty_to_str(cx.tcx, ty), missing.user_string(cx.tcx)));
378+
// Will be Some if the freevar is implicitly borrowed (stack closure).
379+
// Emit a less mysterious error message in this case.
380+
match referenced_ty {
381+
Some(rty) => cx.tcx.sess.span_err(sp,
382+
fmt!("cannot implicitly borrow variable of type `%s` in a bounded \
383+
stack closure (implicit reference does not fulfill `%s`)",
384+
ty_to_str(cx.tcx, rty), missing.user_string(cx.tcx))),
385+
None => cx.tcx.sess.span_err(sp,
386+
fmt!("cannot capture variable of type `%s`, which does \
387+
not fulfill `%s`, in a bounded closure",
388+
ty_to_str(cx.tcx, ty), missing.user_string(cx.tcx))),
389+
}
377390
cx.tcx.sess.span_note(
378391
sp,
379392
fmt!("this closure's environment must satisfy `%s`",

0 commit comments

Comments
 (0)