Factor OnePassCompiler::forwards into local var.

Iteration of a `Forwards` object is destructive,
which previously meant that we had to clone it in order
to iterate over it. Once the compiler iterates over the
forwarding jobs, it never touches them again, so this
was an extra copy. This patch plays a little type tetris
to get rid of that extra copy.

Author: Ethan Pailes

src/onepass.rs

@@ -478,11 +478,6 @@ pub struct OnePassCompiler {
     /// A mapping from instruction indices to flags indicating
     /// if they should have the STATE_MATCH flag set.
     accepting_states: Vec<bool>,
-
-    /// A DAG of forwarding relationships indicating when
-    /// a state needs to be forwarded to an Action state
-    /// once that Action state has been fully constructed.
-    forwards: Forwards,
 }
 
 #[derive(Debug)]
@@ -568,27 +563,33 @@ impl OnePassCompiler {
                 x
             },
             accepting_states: vec![false; prog.len()],
-            forwards: Forwards::new(),
             prog: prog,
         })
     }
 
     /// Attempt to compile the regex to a OnePass DFA
     pub fn compile(mut self) -> Result<OnePass, OnePassError> {
+        // A DAG of forwarding relationships indicating when
+        // a state needs to be forwarded to an Action state
+        // once that Action state has been fully constructed.
+        let mut forwards = Forwards::new();
+
         // Compute the prioritized transition tables for all of the
         // instructions which get states.
         let mut state_edge = vec![0];
         while let Some(i) = state_edge.pop() {
-            state_edge.extend(try!(self.inst_trans(i)));
+            state_edge.extend(try!(self.inst_trans(i, &mut forwards)));
         }
 
         // Solve the dependency relationships between all the
         // forwarding directives that were emitted by inst_trans.
-        try!(self.solve_forwards());
+        for fwd in forwards.into_iter_topo() {
+            self.perform_forward(try!(fwd));
+        }
 
         // Now emit the transitions in a form that we can actually
         // execute.
-        self.emit_transitions();
+        self.bake_transitions();
 
         Ok(OnePass {
             table: self.table,
@@ -611,7 +612,8 @@ impl OnePassCompiler {
     /// via `inst_trans`.
     fn inst_trans(
         &mut self,
-        inst_idx: usize
+        inst_idx: usize,
+        forwards: &mut Forwards,
     ) -> Result<Vec<usize>, OnePassError> {
         trace!("::inst_trans inst_idx={}", inst_idx);
 
@@ -642,7 +644,7 @@ impl OnePassCompiler {
         while let Some(child_idx) = resume.pop() {
             match &self.prog[child_idx] {
                 &Inst::EmptyLook(_) | &Inst::Save(_) => {
-                    self.forwards.forward(inst_idx, child_idx, priority);
+                    forwards.forward(inst_idx, child_idx, priority);
                     children.push(child_idx);
                 }
                 &Inst::Bytes(ref inst) => {
@@ -685,10 +687,7 @@ impl OnePassCompiler {
         Ok(children)
     }
 
-    /// Topologically sort the forwarding jobs so that we
-    /// start with jobs that have no dependencies and then
-    /// shuffle the transitions over. Mutate `self.transitions`
-    /// in place.
+    /// Execute a forwarding job.
     ///
     /// To make that a little more concrete, consider the program snippet:
     ///
@@ -724,71 +723,65 @@ impl OnePassCompiler {
     /// The arrow flows in a funny direction because we want the jobs
     /// with no dependencies to live at the roots of the DAG so that
     /// we can process them first.
-    fn solve_forwards(&mut self) -> Result<(), OnePassError> {
-        // TODO(ethan):yakshaving drop the clone
-        for fwd in self.forwards.clone().into_iter_topo() {
-            let fwd = try!(fwd);
-            debug_assert!(fwd.from != fwd.to);
+    fn perform_forward(&mut self, fwd: Forward) {
+        debug_assert!(fwd.from != fwd.to);
 
-            let tgt = match &self.prog[fwd.to] {
-                &Inst::EmptyLook(_) | &Inst::Save(_) =>
-                    TransitionTarget::ActionInst(fwd.to),
-                _ =>
-                    TransitionTarget::BytesInst(fwd.to),
-            };
-
-            let (from_ts, to_ts) = if fwd.from < fwd.to {
-                let (stub, tail) = self.transitions.split_at_mut(fwd.to);
-                (&mut stub[fwd.from], &mut tail[0])
-            } else {
-                let (stub, tail) = self.transitions.split_at_mut(fwd.from);
-                (&mut tail[0], &mut stub[fwd.to])
-            };
+        let tgt = match &self.prog[fwd.to] {
+            &Inst::EmptyLook(_) | &Inst::Save(_) =>
+                TransitionTarget::ActionInst(fwd.to),
+            _ => TransitionTarget::BytesInst(fwd.to),
+        };
 
-            let (from_ts, to_ts) = match (from_ts, to_ts) {
-                (&mut Some(ref mut from_ts), &mut Some(ref to_ts)) => {
-                    (from_ts, to_ts)
-                }
-                _ => unreachable!("forwards must be between real nodes."),
-            };
+        // Get a pair of mutable references to the two different
+        // transition tables in borrow checker approved fashion.
+        let (from_ts, to_ts) = if fwd.from < fwd.to {
+            let (stub, tail) = self.transitions.split_at_mut(fwd.to);
+            (&mut stub[fwd.from], &mut tail[0])
+        } else {
+            let (stub, tail) = self.transitions.split_at_mut(fwd.from);
+            (&mut tail[0], &mut stub[fwd.to])
+        };
+        let (from_ts, to_ts) = match (from_ts, to_ts) {
+            (&mut Some(ref mut from_ts), &mut Some(ref to_ts)) => {
+                (from_ts, to_ts)
+            }
+            _ => unreachable!("forwards must be between real nodes."),
+        };
 
-            // now shuffle the transitions from `to` to `from`.
-            for (to_t, from_t) in to_ts.0.iter().zip(from_ts.0.iter_mut()) {
-                if to_t.tgt == TransitionTarget::Die {
-                    continue;
-                }
-                if from_t.priority > fwd.priority {
-                    continue;
-                }
+        // now shuffle the transitions from `to` to `from`.
+        for (to_t, from_t) in to_ts.0.iter().zip(from_ts.0.iter_mut()) {
+            if to_t.tgt == TransitionTarget::Die {
+                continue;
+            }
+            if from_t.priority > fwd.priority {
+                continue;
+            }
 
-                // we should never encounter equal priorities
-                debug_assert!(from_t.priority != fwd.priority);
+            // we should never encounter equal priorities
+            debug_assert!(from_t.priority != fwd.priority);
 
-                *from_t = Transition {
-                    tgt: tgt.clone(),
-                    priority: fwd.priority,
-                };
-            }
+            *from_t = Transition {
+                tgt: tgt.clone(),
+                priority: fwd.priority,
+            };
+        }
 
-            // Finally, if a match instruction is reachable through
-            // a save fwd (which can never fail), the from state is accepting.
-            match &self.prog[fwd.to] {
-                &Inst::Save(_) => {
-                    self.accepting_states[fwd.from] =
-                        self.accepting_states[fwd.to];
-                }
-                _ => {}
+        // Finally, if a match instruction is reachable through
+        // a save fwd (which can never fail), the from state is accepting.
+        match &self.prog[fwd.to] {
+            &Inst::Save(_) => {
+                self.accepting_states[fwd.from] =
+                    self.accepting_states[fwd.to];
             }
+            _ => {}
         }
-
-        Ok(())
     }
 
     /// Once all the per-instruction transition tables have been worked
     /// out, we can bake them into the single flat transition table we
     /// are going to use for the actual DFA. This function creates the
     /// baked form, storing it in `self.table`.
-    fn emit_transitions(&mut self) {
+    fn bake_transitions(&mut self) {
         // pre-compute the state indices
         let mut state_starts = Vec::with_capacity(self.prog.len());
         let mut off = 0;
@@ -897,9 +890,6 @@ enum TransitionTarget {
 
 
 /// A (hopefully) DAG of forwarding jobs.
-///
-/// TODO(ethan):yakshaving there are more clones than I am comfortable
-///                        with here.
 #[derive(Debug, Clone)]
 struct Forwards {
     jobs: Vec<Forward>,