From e8ddef93da5f112795eff66ff7dc7ccccc1baa86 Mon Sep 17 00:00:00 2001
From: Brian Anderson <banderson@mozilla.com>
Date: Tue, 12 Mar 2013 00:48:41 -0700
Subject: [PATCH] core: Cleanup rt::context

---
 src/libcore/rt/context.rs | 41 ++++++++++++++++++++++++++++++---------
 src/libcore/rt/sched.rs   | 10 ++--------
 src/libcore/rt/stack.rs   |  1 +
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/src/libcore/rt/context.rs b/src/libcore/rt/context.rs
index dfb7bdf04c3b..11512c3fe7ac 100644
--- a/src/libcore/rt/context.rs
+++ b/src/libcore/rt/context.rs
@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use option::*;
 use super::stack::StackSegment;
 use libc::c_void;
 use cast::{transmute, transmute_mut_unsafe,
@@ -16,17 +17,30 @@ use cast::{transmute, transmute_mut_unsafe,
 // XXX: Registers is boxed so that it is 16-byte aligned, for storing
 // SSE regs.  It would be marginally better not to do this. In C++ we
 // use an attribute on a struct.
-pub struct Context(~Registers);
+// XXX: It would be nice to define regs as `~Option<Registers>` since
+// the registers are sometimes empty, but the discriminant would
+// then misalign the regs again.
+pub struct Context {
+    /// The context entry point, saved here for later destruction
+    start: Option<~~fn()>,
+    /// Hold the registers while the task or scheduler is suspended
+    regs: ~Registers
+}
 
 pub impl Context {
     static fn empty() -> Context {
-        Context(new_regs())
+        Context {
+            start: None,
+            regs: new_regs()
+        }
     }
 
     /// Create a new context that will resume execution by running ~fn()
-    /// # Safety Note
-    /// The `start` closure must remain valid for the life of the Task
-    static fn new(start: &~fn(), stack: &mut StackSegment) -> Context {
+    static fn new(start: ~fn(), stack: &mut StackSegment) -> Context {
+        // XXX: Putting main into a ~ so it's a thin pointer and can
+        // be passed to the spawn function.  Another unfortunate
+        // allocation
+        let start = ~start;
 
         // The C-ABI function that is the task entry point
         extern fn task_start_wrapper(f: &~fn()) { (*f)() }
@@ -46,15 +60,24 @@ pub impl Context {
 
         initialize_call_frame(&mut *regs, fp, argp, sp);
 
-        return Context(regs);
+        return Context {
+            start: Some(start),
+            regs: regs
+        }
     }
 
+    /* Switch contexts
+
+    Suspend the current execution context and resume another by
+    saving the registers values of the executing thread to a Context
+    then loading the registers from a previously saved Context.
+    */
     static fn swap(out_context: &mut Context, in_context: &Context) {
         let out_regs: &mut Registers = match out_context {
-            &Context(~ref mut r) => r
+            &Context { regs: ~ref mut r, _ } => r
         };
         let in_regs: &Registers = match in_context {
-            &Context(~ref r) => r
+            &Context { regs: ~ref r, _ } => r
         };
 
         unsafe { swap_registers(out_regs, in_regs) };
@@ -88,7 +111,7 @@ fn initialize_call_frame(regs: &mut Registers,
                          fptr: *c_void, arg: *c_void, sp: *mut uint) {
 
     let sp = align_down(sp);
-    let sp = mut_offset(sp, -4); // XXX: -4 words? Needs this be done at all?
+    let sp = mut_offset(sp, -4);
 
     unsafe { *sp = arg as uint; }
     let sp = mut_offset(sp, -1);
diff --git a/src/libcore/rt/sched.rs b/src/libcore/rt/sched.rs
index 8f315452e5e5..ff0b3d28e8d7 100644
--- a/src/libcore/rt/sched.rs
+++ b/src/libcore/rt/sched.rs
@@ -285,8 +285,6 @@ pub impl Scheduler {
 const TASK_MIN_STACK_SIZE: uint = 10000000; // XXX: Too much stack
 
 pub struct Task {
-    /// The task entry point, saved here for later destruction
-    priv start: ~~fn(),
     /// The segment of stack on which the task is currently running or,
     /// if the task is blocked, on which the task will resume execution
     priv current_stack_segment: StackSegment,
@@ -297,15 +295,11 @@ pub struct Task {
 
 impl Task {
     static fn new(stack_pool: &mut StackPool, start: ~fn()) -> Task {
-        // XXX: Putting main into a ~ so it's a thin pointer and can
-        // be passed to the spawn function.  Another unfortunate
-        // allocation
-        let start = ~Task::build_start_wrapper(start);
+        let start = Task::build_start_wrapper(start);
         let mut stack = stack_pool.take_segment(TASK_MIN_STACK_SIZE);
         // NB: Context holds a pointer to that ~fn
-        let initial_context = Context::new(&*start, &mut stack);
+        let initial_context = Context::new(start, &mut stack);
         return Task {
-            start: start,
             current_stack_segment: stack,
             saved_context: initial_context,
         };
diff --git a/src/libcore/rt/stack.rs b/src/libcore/rt/stack.rs
index 02c47218ed83..ef48025ffe67 100644
--- a/src/libcore/rt/stack.rs
+++ b/src/libcore/rt/stack.rs
@@ -27,6 +27,7 @@ pub impl StackSegment {
         }
     }
 
+    /// Point one word beyond the high end of the allocated stack
     fn end(&self) -> *uint {
         unsafe {
             vec::raw::to_ptr(self.buf).offset(self.buf.len()) as *uint