add tydescs into shape, rewrite walk_fn_contents()

src/comp/middle/trans.rs

@@ -959,9 +959,9 @@ fn trans_stack_local_derived_tydesc(cx: @block_ctxt, llsz: ValueRef,
     ret llmyroottydesc;
 }
 
-// Objects and closures store their type parameters differently (in the object
-// or closure itself rather than in the type descriptor).
-tag ty_param_storage { tps_normal; tps_obj(uint); tps_fn(uint); }
+// Objects store their type parameters differently (in the object itself
+// rather than in the type descriptor).
+tag ty_param_storage { tps_normal; tps_obj(uint); }
 
 fn get_derived_tydesc(cx: @block_ctxt, t: ty::t, escapes: bool,
                       storage: ty_param_storage,
@@ -980,7 +980,7 @@ fn get_derived_tydesc(cx: @block_ctxt, t: ty::t, escapes: bool,
     let is_obj_body;
     alt storage {
         tps_normal. { is_obj_body = false; }
-        tps_obj(_) | tps_fn(_) { is_obj_body = true; }
+        tps_obj(_) { is_obj_body = true; }
     }
 
     bcx_ccx(cx).stats.n_derived_tydescs += 1u;
@@ -1028,7 +1028,6 @@ fn get_derived_tydesc(cx: @block_ctxt, t: ty::t, escapes: bool,
     alt storage {
       tps_normal. { obj_params = 0u; }
       tps_obj(np) { obj_params = np; }
-      tps_fn(np) { obj_params = 0x80000000u | np; }
     }
 
     let v;

src/comp/middle/trans_closure.rs

@@ -314,16 +314,8 @@ fn maybe_clone_tydesc(bcx: @block_ctxt,
         let bound_tydesc = GEPi(bcx, llbox, [0, abi::cbox_elt_tydesc]);
         let ti = none;
 
-        // NDM I believe this is the correct value,
-        // but using it exposes bugs and limitations
-        // in the shape code.  Therefore, I am using
-        // tps_normal, which is what we used before.
-        //
-        // let tps = tps_fn(vec::len(lltyparams));
-
-        let tps = tps_normal;
         let {result:closure_td, _} =
-            trans::get_tydesc(bcx, cbox_ty, true, tps, ti);
+            trans::get_tydesc(bcx, cbox_ty, true, tps_normal, ti);
         trans::lazily_emit_tydesc_glue(bcx, abi::tydesc_field_take_glue, ti);
         trans::lazily_emit_tydesc_glue(bcx, abi::tydesc_field_drop_glue, ti);
         trans::lazily_emit_tydesc_glue(bcx, abi::tydesc_field_free_glue, ti);

src/rt/rust_cc.cpp

@@ -357,7 +357,7 @@ class mark : public shape::data<mark,shape::ptr> {
           case shape::SHAPE_BOX_FN: {
               // Record an irc for the environment box, but don't descend
               // into it since it will be walked via the box's allocation
-              shape::data<mark,shape::ptr>::walk_fn_contents1(dp);
+              shape::data<mark,shape::ptr>::walk_fn_contents1(dp, false);
               break;
           }
           case shape::SHAPE_BARE_FN:        // Does not close over data.
@@ -552,15 +552,14 @@ class sweep : public shape::data<sweep,shape::ptr> {
               fn_env_pair pair = *(fn_env_pair*)dp;
 
               // free closed over data:
-              // 
-              // FIXME--this is a bit sketchy, since there is an
-              // embedded tydesc that we will be using to walk the
-              // data, but it will be freed as we walk.  In the
-              // generated code we pull this desc out and free it
-              // later.  We may well want to do the same.  However,
-              // since all we use from the descr. is the "shape", I
-              // think we're ok.
-              shape::data<sweep,shape::ptr>::walk_fn_contents1(dp);
+              shape::data<sweep,shape::ptr>::walk_fn_contents1(dp, true);
+
+              // now free the embedded type descr:
+              //
+              // see comment in walk_fn_contents1() concerning null_td
+              // to understand why this does not occur during the normal
+              // walk.
+              upcall_s_free_shared_type_desc((type_desc*)pair.env->td);
 
               // now free the ptr:
               task->kernel->free(pair.env);

src/rt/rust_shape.cpp

@@ -44,15 +44,7 @@ type_param::make(const type_desc **tydescs, unsigned n_tydescs,
     return ptrs;
 }
 
-// Constructs type parameters from a function shape. This is a bit messy,
-// because it requires that the function shape have a specific format.
-type_param *
-type_param::from_fn_shape(rust_opaque_closure *env, arena &arena) {
-    unsigned n_tydescs = env->td->n_obj_params & 0x7fffffff;
-    return make(env->captured_tds, n_tydescs, arena);
-}
-
-// Constructs type parameters from an object shape. This is also a bit messy,
+// Constructs type parameters from an object shape. This is a bit messy,
 // because it requires that the object shape have a specific format.
 type_param *
 type_param::from_obj_shape(const uint8_t *sp, ptr dp, arena &arena) {
@@ -354,6 +346,10 @@ public:
         data<cmp,ptr_pair>::walk_box_contents1();
     }
 
+    void walk_tydesc2(char) {
+        cmp_pointer();
+    }
+
     void walk_fn2(char) { return cmp_two_pointers(); }
     void walk_obj2()    { return cmp_two_pointers(); }
 

src/rt/rust_shape.h

@@ -306,8 +306,6 @@ public:
     const rust_shape_tables *tables;
     const type_param *params;   // subparameters
 
-    // Constructs type parameters from a function shape.
-    static type_param *from_fn_shape(rust_opaque_closure *env, arena &arena);
     // Creates type parameters from an object shape description.
     static type_param *from_obj_shape(const uint8_t *sp, ptr dp,
                                       arena &arena);
@@ -326,8 +324,8 @@ public:
         // have to actually have the data pointer, since we don't statically
         // know from the type of an object or function which type parameters
         // it closes over.
-        assert(!tydesc->n_obj_params && "Type-parametric objects and "
-               "functions must go through from_tydesc_and_data() instead!");
+        assert(!tydesc->n_obj_params && "Type-parametric objects "
+               "must go through from_tydesc_and_data() instead!");
 
         return make(tydesc->first_param, tydesc->n_params, arena);
     }
@@ -337,20 +335,11 @@ public:
         if (tydesc->n_obj_params) {
             uintptr_t n_obj_params = tydesc->n_obj_params;
             const type_desc **first_param;
-            if (n_obj_params & 0x80000000) {
-                // Function closure.
-                DPRINT("n_obj_params FN %lu, tydesc %p, starting at %p\n",
-                       (unsigned long)n_obj_params, tydesc,
-                       dp + sizeof(uintptr_t) + tydesc->size);
-                n_obj_params &= 0x7fffffff;
-                first_param = (const type_desc **)(dp + sizeof(uintptr_t));
-            } else {
-                // Object closure.
-                DPRINT("n_obj_params OBJ %lu, tydesc %p, starting at %p\n",
-                       (unsigned long)n_obj_params, tydesc,
-                       dp + sizeof(uintptr_t) * 2);
-                first_param = (const type_desc **)(dp + sizeof(uintptr_t) * 2);
-            }
+            // Object closure.
+            DPRINT("n_obj_params OBJ %lu, tydesc %p, starting at %p\n",
+                   (unsigned long)n_obj_params, tydesc,
+                   dp + sizeof(uintptr_t) * 2);
+            first_param = (const type_desc **)(dp + sizeof(uintptr_t) * 2);
             return make(first_param, n_obj_params, arena);
         }
 
@@ -852,7 +841,7 @@ protected:
 
     void walk_box_contents1();
     void walk_uniq_contents1();
-    void walk_fn_contents1(ptr &dp);
+    void walk_fn_contents1(ptr &dp, bool null_td);
     void walk_obj_contents1(ptr &dp);
     void walk_iface_contents1(ptr &dp);
     void walk_variant1(tag_info &tinfo, tag_variant_t variant);
@@ -1008,20 +997,40 @@ data<T,U>::walk_tag1(tag_info &tinfo) {
 
 template<typename T,typename U>
 void
-data<T,U>::walk_fn_contents1(ptr &dp) {
+data<T,U>::walk_fn_contents1(ptr &dp, bool null_td) {
     fn_env_pair pair = bump_dp<fn_env_pair>(dp);
     if (!pair.env)
         return;
 
     arena arena;
-    type_param *params =
-      type_param::from_fn_shape(pair.env, arena);
     const type_desc *closure_td = pair.env->td;
+    type_param *params =
+      type_param::from_tydesc(closure_td, arena);
     ptr closure_dp((uintptr_t)pair.env);
     T sub(*static_cast<T *>(this), closure_td->shape, params,
           closure_td->shape_tables, closure_dp);
     sub.align = true;
+
+    if (null_td) {
+        // if null_td flag is true, null out the type descr from
+        // the data structure while we walk.  This is used in cycle
+        // collector when we are sweeping up data.  The idea is that
+        // we are using the information in the embedded type desc to
+        // walk the contents, so we do not want to free it during that
+        // walk.  This is not *strictly* necessary today because
+        // type_param::from_tydesc() actually pulls out the "shape"
+        // string and other information and copies it into a new
+        // location that is unaffected by the free.  But it seems
+        // safer, particularly as this pulling out of information will
+        // not cope with nested, derived type descriptors.
+        pair.env->td = NULL;
+    }
+
     sub.walk();
+
+    if (null_td) {
+        pair.env->td = closure_td;
+    }
 }
 
 template<typename T,typename U>
@@ -1138,7 +1147,7 @@ private:
     void walk_fn2(char kind) {
         out << prefix << "fn";
         prefix = "";
-        data<log,ptr>::walk_fn_contents1(dp);
+        data<log,ptr>::walk_fn_contents1(dp, false);
     }
 
     void walk_obj2() {

src/rt/rust_upcall.cpp

@@ -283,12 +283,14 @@ void upcall_s_free_shared_type_desc(type_desc *td)
     rust_task *task = rust_scheduler::get_task();
     LOG_UPCALL_ENTRY(task);
 
-    // Recursively free any referenced descriptors:
-    for (unsigned i = 0; i < td->n_params; i++) {
-        upcall_s_free_shared_type_desc((type_desc*) td->first_param[i]);
-    }
+    if (td) {
+        // Recursively free any referenced descriptors:
+        for (unsigned i = 0; i < td->n_params; i++) {
+            upcall_s_free_shared_type_desc((type_desc*) td->first_param[i]);
+        }
 
-    task->kernel->free(td);
+        task->kernel->free(td);
+    }
 }
 
 extern "C" CDECL void

src/test/run-pass/uniq-cc-generic.rs

+tag maybe_pointy {
+    none;
+    p(@pointy);
+}
+
+type pointy = {
+    mutable a : maybe_pointy,
+    d : sendfn() -> uint,
+};
+
+fn make_uniq_closure<A:send>(a: A) -> sendfn() -> uint {
+    sendfn() -> uint { ptr::addr_of(a) as uint }
+}
+
+fn empty_pointy() -> @pointy {
+    ret @{
+        mutable a : none,
+        d : make_uniq_closure("hi")
+    }
+}
+
+fn main()
+{
+    let v = empty_pointy();
+    v.a = p(v);
+}

src/test/run-pass/uniq-cc.rs

+tag maybe_pointy {
+    none;
+    p(@pointy);
+}
+
+type pointy = {
+    mutable a : maybe_pointy,
+    c : ~int,
+    d : sendfn()->(),
+};
+
+fn empty_pointy() -> @pointy {
+    ret @{
+        mutable a : none,
+        c : ~22,
+        d : sendfn()->(){},
+    }
+}
+
+fn main()
+{
+    let v = empty_pointy();
+    v.a = p(v);
+}

src/test/run-pass/uniq-fn-leak.rs

-// xfail-test
-    tag maybe_pointy {
-        none;
-        p(@pointy);
-    }
-
-    type pointy = {
-        mutable a : maybe_pointy,
-        c : ~int,
-        d : sendfn()->(),
-    };
-
-    fn empty_pointy() -> @pointy {
-        ret @{
-            mutable a : none,
-            c : ~22,
-            d : sendfn()->(){},
-        }
-    }
-
-    fn main()
-    {
-        let v = empty_pointy();
-        v.a = p(v);
-    }