Permalink
Browse files

prevent regions from escaping in ifaces; remove &r.T syntax

  • Loading branch information...
1 parent eb0a34c commit e0ea67a2a60c694a6c3424d99e4692c2725b8506 @nikomatsakis nikomatsakis committed Jul 18, 2012
Showing with 411 additions and 130 deletions.
  1. +42 −19 src/libcore/task.rs
  2. +3 −1 src/libstd/deque.rs
  3. +1 −2 src/libsyntax/parse/parser.rs
  4. +1 −1 src/libsyntax/print/pprust.rs
  5. +1 −1 src/rustc/metadata/tyencode.rs
  6. +88 −5 src/rustc/middle/kind.rs
  7. +1 −1 src/rustc/middle/trans/base.rs
  8. +1 −1 src/rustc/middle/trans/reflect.rs
  9. +2 −2 src/rustc/middle/trans/type_use.rs
  10. +34 −13 src/rustc/middle/ty.rs
  11. +8 −8 src/rustc/middle/typeck/astconv.rs
  12. +1 −1 src/rustc/middle/typeck/check.rs
  13. +2 −2 src/rustc/middle/typeck/check/method.rs
  14. +99 −44 src/rustc/middle/typeck/check/regionck.rs
  15. +1 −1 src/rustc/middle/typeck/check/vtable.rs
  16. +3 −1 src/rustc/middle/typeck/collect.rs
  17. +2 −2 src/rustc/middle/typeck/infer.rs
  18. +3 −2 src/rustc/middle/typeck/rscope.rs
  19. +1 −1 src/rustc/util/ppaux.rs
  20. +1 −1 src/test/bench/shootout-binarytrees.rs
  21. +1 −1 src/test/compile-fail/block-arg-used-as-lambda-with-illegal-cap.rs
  22. +20 −0 src/test/compile-fail/kindck-owned-trait-contains.rs
  23. +35 −0 src/test/compile-fail/kindck-owned-trait-scoped.rs
  24. +11 −0 src/test/compile-fail/kindck-owned-trait.rs
  25. +4 −4 src/test/compile-fail/kindck-owned.rs
  26. +10 −0 src/test/run-pass/borrowck-borrow-from-at-vec.rs
  27. +1 −1 src/test/run-pass/issue-2734.rs
  28. +1 −1 src/test/run-pass/issue-2735.rs
  29. +16 −0 src/test/run-pass/kindck-owned-trait-contains-1.rs
  30. +6 −6 src/test/run-pass/reflect-visit-data.rs
  31. +1 −1 src/test/run-pass/regions-addr-of-ret.rs
  32. +1 −1 src/test/run-pass/regions-bot.rs
  33. +2 −2 src/test/run-pass/regions-fn-subtyping.rs
  34. +4 −1 src/test/run-pass/regions-iface.rs
  35. +2 −2 src/test/run-pass/regions-mock-trans.rs
  36. +1 −1 src/test/run-pass/regions-params.rs
View
@@ -832,10 +832,10 @@ fn spawn_raw(opts: task_opts, +f: fn~()) {
* types; arbitrary type coercion is possible this way. The interface is safe
* as long as all key functions are monomorphic.
*/
-type local_data_key<T> = fn@(+@T);
+type local_data_key<T: owned> = fn@(+@T);
iface local_data { }
-impl<T> of local_data for @T { }
+impl<T: owned> of local_data for @T { }
// We use dvec because it's the best data structure in core. If TLS is used
// heavily in future, this could be made more efficient with a proper map.
@@ -852,6 +852,7 @@ extern fn cleanup_task_local_map(map_ptr: *libc::c_void) unsafe {
// Gets the map from the runtime. Lazily initialises if not done so already.
unsafe fn get_task_local_map(task: *rust_task) -> task_local_map {
+
// Relies on the runtime initialising the pointer to null.
// NOTE: The map's box lives in TLS invisibly referenced once. Each time
// we retrieve it for get/set, we make another reference, which get/set
@@ -872,16 +873,20 @@ unsafe fn get_task_local_map(task: *rust_task) -> task_local_map {
}
}
-unsafe fn key_to_key_value<T>(key: local_data_key<T>) -> *libc::c_void {
+unsafe fn key_to_key_value<T: owned>(
+ key: local_data_key<T>) -> *libc::c_void {
+
// Keys are closures, which are (fnptr,envptr) pairs. Use fnptr.
// Use reintepret_cast -- transmute would leak (forget) the closure.
let pair: (*libc::c_void, *libc::c_void) = unsafe::reinterpret_cast(key);
pair.first()
}
// If returning some(..), returns with @T with the map's reference. Careful!
-unsafe fn local_data_lookup<T>(map: task_local_map, key: local_data_key<T>)
- -> option<(uint, *libc::c_void)> {
+unsafe fn local_data_lookup<T: owned>(
+ map: task_local_map, key: local_data_key<T>)
+ -> option<(uint, *libc::c_void)> {
+
let key_value = key_to_key_value(key);
let map_pos = (*map).position(|entry|
alt entry { some((k,_,_)) { k == key_value } none { false } }
@@ -893,8 +898,10 @@ unsafe fn local_data_lookup<T>(map: task_local_map, key: local_data_key<T>)
}
}
-unsafe fn local_get_helper<T>(task: *rust_task, key: local_data_key<T>,
- do_pop: bool) -> option<@T> {
+unsafe fn local_get_helper<T: owned>(
+ task: *rust_task, key: local_data_key<T>,
+ do_pop: bool) -> option<@T> {
+
let map = get_task_local_map(task);
// Interpret our findings from the map
do local_data_lookup(map, key).map |result| {
@@ -912,17 +919,23 @@ unsafe fn local_get_helper<T>(task: *rust_task, key: local_data_key<T>,
}
}
-unsafe fn local_pop<T>(task: *rust_task,
- key: local_data_key<T>) -> option<@T> {
+unsafe fn local_pop<T: owned>(
+ task: *rust_task,
+ key: local_data_key<T>) -> option<@T> {
+
local_get_helper(task, key, true)
}
-unsafe fn local_get<T>(task: *rust_task,
- key: local_data_key<T>) -> option<@T> {
+unsafe fn local_get<T: owned>(
+ task: *rust_task,
+ key: local_data_key<T>) -> option<@T> {
+
local_get_helper(task, key, false)
}
-unsafe fn local_set<T>(task: *rust_task, key: local_data_key<T>, +data: @T) {
+unsafe fn local_set<T: owned>(
+ task: *rust_task, key: local_data_key<T>, +data: @T) {
+
let map = get_task_local_map(task);
// Store key+data as *voids. Data is invisibly referenced once; key isn't.
let keyval = key_to_key_value(key);
@@ -956,8 +969,10 @@ unsafe fn local_set<T>(task: *rust_task, key: local_data_key<T>, +data: @T) {
}
}
-unsafe fn local_modify<T>(task: *rust_task, key: local_data_key<T>,
- modify_fn: fn(option<@T>) -> option<@T>) {
+unsafe fn local_modify<T: owned>(
+ task: *rust_task, key: local_data_key<T>,
+ modify_fn: fn(option<@T>) -> option<@T>) {
+
// Could be more efficient by doing the lookup work, but this is easy.
let newdata = modify_fn(local_pop(task, key));
if newdata.is_some() {
@@ -970,29 +985,37 @@ unsafe fn local_modify<T>(task: *rust_task, key: local_data_key<T>,
* Remove a task-local data value from the table, returning the
* reference that was originally created to insert it.
*/
-unsafe fn local_data_pop<T>(key: local_data_key<T>) -> option<@T> {
+unsafe fn local_data_pop<T: owned>(
+ key: local_data_key<T>) -> option<@T> {
+
local_pop(rustrt::rust_get_task(), key)
}
/**
* Retrieve a task-local data value. It will also be kept alive in the
* table until explicitly removed.
*/
-unsafe fn local_data_get<T>(key: local_data_key<T>) -> option<@T> {
+unsafe fn local_data_get<T: owned>(
+ key: local_data_key<T>) -> option<@T> {
+
local_get(rustrt::rust_get_task(), key)
}
/**
* Store a value in task-local data. If this key already has a value,
* that value is overwritten (and its destructor is run).
*/
-unsafe fn local_data_set<T>(key: local_data_key<T>, +data: @T) {
+unsafe fn local_data_set<T: owned>(
+ key: local_data_key<T>, +data: @T) {
+
local_set(rustrt::rust_get_task(), key, data)
}
/**
* Modify a task-local data value. If the function returns 'none', the
* data is removed (and its reference dropped).
*/
-unsafe fn local_data_modify<T>(key: local_data_key<T>,
- modify_fn: fn(option<@T>) -> option<@T>) {
+unsafe fn local_data_modify<T: owned>(
+ key: local_data_key<T>,
+ modify_fn: fn(option<@T>) -> option<@T>) {
+
local_modify(rustrt::rust_get_task(), key, modify_fn)
}
View
@@ -193,7 +193,9 @@ mod tests {
type eqfn<T> = fn@(T, T) -> bool;
- fn test_parameterized<T: copy>(e: eqfn<T>, a: T, b: T, c: T, d: T) {
+ fn test_parameterized<T: copy owned>(
+ e: eqfn<T>, a: T, b: T, c: T, d: T) {
+
let deq: deque::t<T> = deque::create::<T>();
assert (deq.size() == 0u);
deq.add_front(a);
@@ -383,8 +383,7 @@ class parser {
let name =
alt copy self.token {
token::IDENT(sid, _) => {
- if self.look_ahead(1u) == token::DOT || // backwards compat
- self.look_ahead(1u) == token::BINOP(token::SLASH) {
+ if self.look_ahead(1u) == token::BINOP(token::SLASH) {
self.bump(); self.bump();
some(self.get_str(sid))
} else {
@@ -345,7 +345,7 @@ fn print_type_ex(s: ps, &&ty: @ast::ty, print_colons: bool) {
ast::ty_rptr(region, mt) {
alt region.node {
ast::re_anon { word(s.s, ~"&"); }
- _ { print_region(s, region); word(s.s, ~"."); }
+ _ { print_region(s, region); word(s.s, ~"/"); }
}
print_mt(s, mt);
}
@@ -272,7 +272,7 @@ fn enc_sty(w: io::writer, cx: @ctxt, st: ty::sty) {
w.write_char('I');
w.write_uint(id.to_uint());
}
- ty::ty_param(id, did) {
+ ty::ty_param({idx: id, def_id: did}) {
w.write_char('p');
w.write_str(cx.ds(did));
w.write_char('|');
View
@@ -115,7 +115,7 @@ fn with_appropriate_checker(cx: ctx, id: node_id, b: fn(check_fn)) {
fn check_for_box(cx: ctx, id: node_id, fv: option<@freevar_entry>,
is_move: bool, var_t: ty::t, sp: span) {
// all captured data must be owned
- if !check_owned(cx, var_t, sp) { ret; }
+ if !check_owned(cx.tcx, var_t, sp) { ret; }
// copied in data must be copyable, but moved in data can be anything
let is_implicit = fv.is_some();
@@ -217,7 +217,13 @@ fn check_expr(e: @expr, cx: ctx, v: visit::vt<ctx>) {
alt e.node {
expr_assign(_, ex) |
expr_unary(box(_), ex) | expr_unary(uniq(_), ex) |
- expr_ret(some(ex)) | expr_cast(ex, _) { maybe_copy(cx, ex); }
+ expr_ret(some(ex)) {
+ maybe_copy(cx, ex);
+ }
+ expr_cast(source, _) {
+ maybe_copy(cx, source);
+ check_cast_for_escaping_regions(cx, source, e);
+ }
expr_copy(expr) { check_copy_ex(cx, expr, false); }
// Vector add copies, but not "implicitly"
expr_assign_op(_, _, ex) { check_copy_ex(cx, ex, false) }
@@ -440,15 +446,92 @@ fn check_send(cx: ctx, ty: ty::t, sp: span) -> bool {
}
}
-fn check_owned(cx: ctx, ty: ty::t, sp: span) -> bool {
- if !ty::kind_is_owned(ty::type_kind(cx.tcx, ty)) {
- cx.tcx.sess.span_err(sp, ~"not an owned value");
+// note: also used from middle::typeck::regionck!
+fn check_owned(tcx: ty::ctxt, ty: ty::t, sp: span) -> bool {
+ if !ty::kind_is_owned(ty::type_kind(tcx, ty)) {
+ alt ty::get(ty).struct {
+ ty::ty_param(*) {
+ tcx.sess.span_err(sp, ~"value may contain borrowed \
+ pointers; use `owned` bound");
+ }
+ _ {
+ tcx.sess.span_err(sp, ~"value may contain borrowed \
+ pointers");
+ }
+ }
false
} else {
true
}
}
+/// This is rather subtle. When we are casting a value to a
+/// instantiated iface like `a as iface/&r`, regionck already ensures
+/// that any borrowed pointers that appear in the type of `a` are
+/// bounded by `&r`. However, it is possible that there are *type
+/// parameters* in the type of `a`, and those *type parameters* may
+/// have borrowed pointers within them. We have to guarantee that the
+/// regions which appear in those type parameters are not obscured.
+///
+/// Therefore, we ensure that one of three conditions holds:
+///
+/// (1) The iface instance cannot escape the current fn. This is
+/// guaranteed if the region bound `&r` is some scope within the fn
+/// itself. This case is safe because whatever borrowed pointers are
+/// found within the type parameter, they must enclose the fn body
+/// itself.
+///
+/// (2) The type parameter appears in the type of the iface. For
+/// example, if the type parameter is `T` and the iface type is
+/// `deque<T>`, then whatever borrowed ptrs may appear in `T` also
+/// appear in `deque<T>`.
+///
+/// (3) The type parameter is owned (and therefore does not contain
+/// borrowed ptrs).
+fn check_cast_for_escaping_regions(
+ cx: ctx,
+ source: @expr,
+ target: @expr) {
+
+ // Determine what type we are casting to; if it is not an iface, then no
+ // worries.
+ let target_ty = ty::expr_ty(cx.tcx, target);
+ let target_substs = alt ty::get(target_ty).struct {
+ ty::ty_trait(_, substs) => {substs}
+ _ => { ret; /* not a cast to a trait */ }
+ };
+
+ // Check, based on the region associated with the iface, whether it can
+ // possibly escape the enclosing fn item (note that all type parameters
+ // must have been declared on the enclosing fn item):
+ alt target_substs.self_r {
+ some(ty::re_scope(*)) => { ret; /* case (1) */ }
+ none | some(ty::re_static) | some(ty::re_free(*)) => {}
+ some(ty::re_bound(*)) | some(ty::re_var(*)) => {
+ cx.tcx.sess.span_bug(
+ source.span,
+ #fmt["bad region found in kind: %?", target_substs.self_r]);
+ }
+ }
+
+ // Assuming the iface instance can escape, then ensure that each parameter
+ // either appears in the iface type or is owned:
+ let target_params = ty::param_tys_in_type(target_ty);
+ let source_ty = ty::expr_ty(cx.tcx, source);
+ do ty::walk_ty(source_ty) |ty| {
+ alt ty::get(ty).struct {
+ ty::ty_param(source_param) => {
+ if target_params.contains(source_param) {
+ /* case (2) */
+ } else {
+ check_owned(cx.tcx, ty, source.span); /* case (3) */
+ }
+ }
+ _ => {}
+ }
+ }
+}
+
//
// Local Variables:
// mode: rust
@@ -1317,7 +1317,7 @@ enum copy_action { INIT, DROP_EXISTING, }
fn type_is_structural_or_param(t: ty::t) -> bool {
if ty::type_is_structural(t) { ret true; }
alt ty::get(t).struct {
- ty::ty_param(_, _) { ret true; }
+ ty::ty_param(*) { ret true; }
_ { ret false; }
}
}
@@ -272,7 +272,7 @@ impl methods for reflector {
ty::ty_trait(_, _) { self.leaf(~"trait") }
ty::ty_var(_) { self.leaf(~"var") }
ty::ty_var_integral(_) { self.leaf(~"var_integral") }
- ty::ty_param(n, _) { self.visit(~"param", ~[self.c_uint(n)]) }
+ ty::ty_param(p) { self.visit(~"param", ~[self.c_uint(p.idx)]) }
ty::ty_self { self.leaf(~"self") }
ty::ty_type { self.leaf(~"type") }
ty::ty_opaque_box { self.leaf(~"opaque_box") }
@@ -137,8 +137,8 @@ fn type_needs_inner(cx: ctx, use: uint, ty: ty::t,
}
false
}
- ty::ty_param(n, _) {
- cx.uses[n] |= use;
+ ty::ty_param(p) {
+ cx.uses[p.idx] |= use;
false
}
_ { true }
Oops, something went wrong.

0 comments on commit e0ea67a

Please sign in to comment.