add tydescs into shape, rewrite walk_fn_contents()

4 files changed, 53 insertions, 47 deletions

diff --git a/src/rt/rust_cc.cpp b/src/rt/rust_cc.cpp
index 1f99fe98f71..78138434d73 100644
--- a/src/rt/rust_cc.cpp
+++ b/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);
diff --git a/src/rt/rust_shape.cpp b/src/rt/rust_shape.cpp
index b7ab591ce04..961cfe7bc46 100644
--- a/src/rt/rust_shape.cpp
+++ b/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(); }
 
diff --git a/src/rt/rust_shape.h b/src/rt/rust_shape.h
index b4efae9e704..41cc9c11fb3 100644
--- a/src/rt/rust_shape.h
+++ b/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() {
diff --git a/src/rt/rust_upcall.cpp b/src/rt/rust_upcall.cpp
index 12ef28065bd..65299a7d244 100644
--- a/src/rt/rust_upcall.cpp
+++ b/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