Fixed some Star scenarios and crashers all around

istatik/ParserWindow.cpp

@@ -41,9 +41,9 @@ WindowResponse ParserWindow::Input(const Batch& ibatch) {
   // Find first item in incParser's output list, and draw everything
   response.actions.push_back(WindowAction(WindowAction::MOVE, 0, 0, 0));
   Batch batch;
+  g_log.debug() << "Extracting changes from IncParser";
   m_incParser.ExtractChanges(batch);
-  // WARNING: can't show the batch, because its deleted items may be gone already :O
-  g_log.info() << "Printing WindowResponse list for batch of size: " << batch.Size();
+  g_log.info() << "Printing WindowResponse list for batch of size " << batch.Size() << ": " << batch;
   if (m_nodes) {
     g_log.debug() << "m_nodes is: " << *m_nodes;
   }
@@ -63,6 +63,22 @@ WindowResponse ParserWindow::Input(const Batch& ibatch) {
             node->right = pos->right;
             node->left = pos;
             pos->right = node;
+            if (node->right) {
+              node->right->left = node;
+            }
+            g_log.debug() << "Inserted " << *node;
+            if (node->left) {
+              g_log.debug() << " - with left: " << *node->left;
+              if (node->left->right) {
+                g_log.debug() << " - - which has right: " << *node->left->right;
+              }
+            }
+            if (node->right) {
+              g_log.debug() << " - with right: " << *node->right;
+              if (node->right->left) {
+                g_log.debug() << " - - which has left: " << *node->right->left;
+              }
+            }
           } else {
             g_log.debug() << "Setting this node as m_nodes";
             node->right = m_nodes;
@@ -85,22 +101,36 @@ WindowResponse ParserWindow::Input(const Batch& ibatch) {
           }
           List* node = node_i->second;
           g_log.debug() << "Delete node: " << node->name << ":" << node->value;
-          if (node->left) {
-            node->left->right = node->right;
-            if (node->right) {
-              node->right->left = node->left;
-            }
+          List* left = node->left;
+          List* right = node->right;
+          if (left && left->right == node) {
+            left->right = right;
+          }
+          if (right && right->left == node) {
+            right->left = left;
           }
           if (m_nodes == node) {
             g_log.debug() << "Deleted node is m_nodes";
-            if (node->left) {
+            if (left) {
               g_log.debug() << "going left";
-              m_nodes = node->left;
-            } else {
+              g_log.debug() << "going left to " << *left << " from " << *node;
+              m_nodes = left;
+            } else if (right) {
               g_log.debug() << "going right";
-              m_nodes = node->right;
+              g_log.debug() << "going right to " << *right << " from " << *node;
+              m_nodes = right;
+            } else {
+              g_log.debug() << "nowhere to go; m_nodes is clear";
+              m_nodes = NULL;
             }
           }
+          g_log.debug() << "Deleted " << *node;
+          if (node->left) {
+            g_log.debug() << " - with left: " << *node->left;
+          }
+          if (node->right) {
+            g_log.debug() << " - with right: " << *node->right;
+          }
           delete node;
           m_nodeMap.erase(node_i);
           g_log.debug() << "Done deleting node";

statik/Batch.cpp

@@ -6,6 +6,9 @@
 #include "List.h"
 #include "SLog.h"
 
+#include <boost/lexical_cast.hpp>
+using boost::lexical_cast;
+
 #include <ostream>
 #include <string>
 using std::ostream;
@@ -19,7 +22,12 @@ std::string Batch::BatchItem::Print() const {
   if (!node) {
     throw SError("Cannot print defective BatchItem");
   }
-  string s(UnMapBatchOp(op) + " " + node->Print());
+  string s(UnMapBatchOp(op) + " ");
+  if (OP_DELETE == op) {
+    s += lexical_cast<string>(node);
+  } else {
+    s += lexical_cast<string>(node) + " " + node->Print();
+  }
   if (pos) {
     s += " @ " + pos->Print();
   }

statik/Grapher.cpp

@@ -119,11 +119,19 @@ void Grapher::AddOBatch(const string& context, const Batch& batch, const string&
     default:
       throw SError("Cannot add batch to Grapher: unknown operation");
     }
-    const List& node = *i->node;
-    m_graph += dotVar(&node, context) + " [label=\"" + Util::safeLabelStr(node.name + (node.value.empty() ? "" : (node.name.empty() ? "" : ":") + node.value)) + "\", style=\"filled\", fillcolor=\"#dddddd\", fontsize=12.0, color=\"" + color + "\", penwidth=4.0];\n";
-    if (i+1 != batch.end()) {
-      const List& next = *(i+1)->node;
-      m_graph += dotVar(&node, context) + " -> " + dotVar(&next, context) + ";\n";
+    if (Batch::OP_DELETE == i->op) {
+      m_graph += dotVar(i->node, context) + " [label=\"(deleted)\", style=\"filled\", fillcolor=\"#dddddd\", fontsize=12.0, color=\"" + color + "\", penwidth=4.0];\n";
+      if (i+1 != batch.end()) {
+        const List& next = *(i+1)->node;
+        m_graph += dotVar(i->node, context) + " -> " + dotVar(&next, context) + ";\n";
+      }
+    } else {
+      const List& node = *i->node;
+      m_graph += dotVar(&node, context) + " [label=\"" + Util::safeLabelStr(node.name + (node.value.empty() ? "" : (node.name.empty() ? "" : ":") + node.value)) + "\", style=\"filled\", fillcolor=\"#dddddd\", fontsize=12.0, color=\"" + color + "\", penwidth=4.0];\n";
+      if (i+1 != batch.end()) {
+        const List& next = *(i+1)->node;
+        m_graph += dotVar(&node, context) + " -> " + dotVar(&next, context) + ";\n";
+      }
     }
   }
   m_graph += "}\n";

statik/IncParser.cpp

@@ -138,6 +138,10 @@ void IncParser::ExtractChanges(Batch& out_batch) {
   }
   g_log.debug() << "EXTRACT DONE: " << m_name;
   m_touchedNodes.clear();
+  for (std::vector<STree*>::iterator i = m_forcedChanges.begin(); i != m_forcedChanges.end(); ++i) {
+    (*i)->Unforce();
+  }
+  m_forcedChanges.clear();
   Cleanup();
   if (!out_batch.IsEmpty()) {
     DrawGraph(m_root, /*inode*/ NULL, &out_batch);
@@ -202,6 +206,10 @@ void IncParser::TouchNode(const STree& node) {
   m_touchedNodes.insert(&node);
 }
 
+void IncParser::ForceChange(STree& node) {
+  m_forcedChanges.push_back(&node);
+}
+
 int IncParser::INodeCompare(const List& a, const List& b) const {
   return m_orderList.Compare(a, b);
 }
@@ -214,7 +222,7 @@ void IncParser::DrawGraph(const STree& onode, const List* inode, const Batch* ba
   if (!m_grapher.get()) {
     return;
   }
-  SanityCheck();
+  SanityCheck("DrawGraph");
   const List* istart = GetFirstINode();
   if (istart) {
     m_grapher->AddIList(m_name, *istart, m_name + " input");
@@ -324,7 +332,7 @@ void IncParser::DeleteNode(List& inode) {
 
   ProcessActions();
   m_listeners.RemoveAllListeners(&inode);
-  m_orderList.Delete(inode);
+  m_deleteFromOrderListSoon.push_back(&inode);
   g_log.debug() << "IncParser: Delete done";
 }
 
@@ -533,11 +541,17 @@ void IncParser::ComputeOutput_Delete(const STree& node, Batch& out_batch, State:
 
 void IncParser::Cleanup() {
   g_log.debug() << "IncParser " << m_name << " - cleaning up";
+  g_san.debug() << "Cleaning up order list";
+  for (std::vector<const List*>::const_iterator i = m_deleteFromOrderListSoon.begin();
+       i != m_deleteFromOrderListSoon.end(); ++i) {
+    m_orderList.Delete(**i);
+  }
+  m_deleteFromOrderListSoon.clear();
   g_san.debug() << "Cleaning up node pool: " << m_nodePool;
   m_nodePool.Cleanup();
   g_san.debug() << "Cleaning up IList pool: " << m_ilistPool;
   m_ilistPool.Cleanup();
-  SanityCheck();
+  SanityCheck("Cleanup");
 }
 
 void IncParser::InsertOutput(const OutputItem& item, Batch& out_batch, const List*& behind_node, State::Station worst_station) {
@@ -591,12 +605,13 @@ const List* IncParser::GetOEnd(const OutputItem& item) const {
   return item.child->GetOutputFunc().OEnd();
 }
 
-void IncParser::SanityCheck() {
-  g_log.debug() << "Sanity check " << m_sancount;
+void IncParser::SanityCheck(const std::string& why) {
+  g_log.debug() << "Sanity check " << m_sancount << " for " << why;
   g_log.debug();
   g_san.info();
   g_san.info() << "Sanity check " << m_sancount;
   SanityCheck(&m_root);
+  g_san.debug() << "Sanity " << m_sancount << " is done.";
   ++m_sancount;
 }
 

statik/IncParser.h

@@ -72,6 +72,10 @@ class IncParser {
   // Indicate that a node has been computed.  Called by STree::ParseNode()
   void TouchNode(const STree& node);
 
+  // Indicate that a node must be force-changed (i.e. its parent MUST believe
+  // that the node has changed)
+  void ForceChange(STree& node);
+
   int INodeCompare(const List& a, const List& b) const;
 
   listener_set GetListeners(const List& x) const;
@@ -111,7 +115,7 @@ class IncParser {
   void UpdateOutput(OutputItem& item, Batch& out_batch, const List*& behind_node, State::Station worst_station);
   const List* GetOEnd(const OutputItem& item) const;
 
-  void SanityCheck();
+  void SanityCheck(const std::string& why);
   void SanityCheck(const STree* s) const;
 
   Rule m_rootRule; // Rule for the Root node
@@ -121,9 +125,12 @@ class IncParser {
   ObjectPool<List> m_ilistPool;
   input_map m_inputMap;
   ObjectPool<STree> m_nodePool;
+  // because deleted nodes still need to be compared during ExtractChanges()
+  std::vector<const List*> m_deleteFromOrderListSoon;
   OrderList m_orderList;
   action_map m_actions_by_depth;
   ListenerTable<const List*, STree*> m_listeners;
+  std::vector<STree*> m_forcedChanges;
 
   output_map m_outputPerNode;
   std::set<const STree*> m_touchedNodes;

statik/OutputFunc.cpp

@@ -202,12 +202,14 @@ void OutputFunc_Sequence::operator() () {
   g_log.debug() << "OutputFunc_Sequence() " << *m_node;
   m_output.clear();
 
+/*
   if (m_node->GetState().IsBad()) {
     for (STree::child_iter child = m_node->children.begin(); child != m_node->children.end(); ++child) {
       m_output.push_back(OutputItem(**child, m_node->GetState().GetStation()));
     }
     return;
   }
+*/
 
   for (STree::child_iter child = m_node->children.begin(); child != m_node->children.end(); ++child) {
     g_log.debug() << " - Considering child " << **child;

statik/STree.cpp

@@ -30,7 +30,8 @@ STree::STree(IncParser& incParser,
     m_depth(m_parent ? m_parent->GetDepth() + 1 : 0),
     m_isClear(true),
     m_parseFunc(rule.CloneParseFunc()),
-    m_outputFunc(rule.CloneOutputFunc()) {
+    m_outputFunc(rule.CloneOutputFunc()),
+    m_forceChange(false) {
   m_parseFunc->Init(*this);
   m_outputFunc->Init(*this);
 }
@@ -73,14 +74,17 @@ void STree::ParseNode(ParseAction::Action action, const List& inode, const STree
   bool hasChanged0 = old_istart != new_istart;
   bool hasChanged1 = old_iend != new_iend;
   bool hasChanged2 = old_state != m_state;
-  bool hasChanged = hasChanged0 || hasChanged1 || hasChanged2;
-  g_log.info() << " - - - - " << *this << " has " << (hasChanged ? "" : "NOT ") << "changed  " << hasChanged1 << ":" << hasChanged2;
+  bool hasChanged3 = m_forceChange;
+  bool hasChanged = hasChanged0 || hasChanged1 || hasChanged2 || hasChanged3;
+  g_log.info() << " - - - - " << *this << " has " << (hasChanged ? "" : "NOT ") << "changed  " << hasChanged1 << ":" << hasChanged2 << ":" << hasChanged3;
   m_incParser.TouchNode(*this);
   if (hasChanged && !m_isClear && m_parent && !m_state.IsPending()) {
+    g_log.debug() << " - - - - " << " enqueueing child update due to change";
     ParseAction::Action a = ParseAction::ChildUpdate;
     m_incParser.Enqueue(ParseAction(a, *m_parent, inode, this));
   }
   m_incParser.DrawGraph(*this, &inode, NULL, initiator);
+  m_incParser.ForceChange(*this);
 }
 
 void STree::ClearNode(const List& inode) {

statik/STree.h

@@ -41,6 +41,8 @@ class STree {
   void ParseNode(ParseAction::Action action, const List& inode, const STree* initiator);
   void ClearNode(const List& inode);
 
+  void Unforce() { m_forceChange = false; }
+
   bool IsClear() const { return m_isClear; }
   const State& GetState() const { return m_state; }
   State& GetState() { return m_state; }
@@ -67,6 +69,7 @@ class STree {
   IConnection m_iconnection;
   std::auto_ptr<ParseFunc> m_parseFunc;
   std::auto_ptr<OutputFunc> m_outputFunc;
+  bool m_forceChange;
 
   void ClearSubNode();
 };

statik/Star.cpp

@@ -109,10 +109,11 @@ void ParseFunc_Star::operator() (ParseAction::Action action, const List& inode,
         m_node->children.erase(child);
         if (inode.right && inode.right->left != &inode) {
           g_log.info() << "Restarting self at right inode " << *inode.right;
-          g_log.warning() << "Probably shouldn't be doing this here (is top-node behaviour different than subsidiaries?)";
-          m_node->GetIncParser().Enqueue(ParseAction(ParseAction::Restart, *m_node, *inode.right, m_node));
-          state.GoPending();
-          return;
+          g_log.warning() << "Probably shouldn't be doing this here, i.e.  maybe the parent should decide that we should restart at our right inode now that our IStart is gone.  But is top-node behaviour different than subsidiaries?";
+          //m_node->GetIncParser().Enqueue(ParseAction(ParseAction::Restart, *m_node, *inode.right, m_node));
+          g_log.info() << "Setting our IStart to the child's IStart, and continuing to determine state.";
+          m_node->GetIConnection().Restart(m_node->children.at(0)->IStart());
+          // keep going to determine our state
         } else {
           g_log.warning() << "ParseFunc_Star at " << *m_node << ": First child was cleared, but it's not as if our first INode was deleted but we can just move forward an INode.  Clearing self.";
           m_node->ClearNode(inode);
@@ -266,10 +267,10 @@ void ParseFunc_Star::operator() (ParseAction::Action action, const List& inode,
   const STree* lockedChild = NULL;
   for (STree::child_iter i = m_node->children.begin(); i != m_node->children.end(); ++i) {
     const State& istate = (*i)->GetState();
-    if (!istate.IsComplete()) {
+    if (!breachChild && !istate.IsComplete()) {
       breachChild = *i;
     }
-    if (istate.IsLocked()) {
+    if (!lockedChild && istate.IsLocked()) {
       lockedChild = *i;
     }
     if (breachChild && lockedChild) {