Arrange multops so that Boolean arguments come first.

This helps recursive implication checks.  Also order
atomic propositions with strverscmp().

* src/ltlast/formula.hh (formula_ptr_less_than_multop,
is_literal, atomic_prop_cmp): New.
* src/ltlast/formula.cc (is_literal, atomic_prop_cmp): Implement them.
* src/ltlast/multop.cc: Use formula_ptr_less_than_multop.
* src/ltltest/isop.test, src/ltltest/ltlfilt.test,
src/tgbatest/det.test, src/tgbatest/dstar.test,
src/tgbatest/explicit.test, src/tgbatest/explpro2.test,
src/tgbatest/explpro3.test, src/tgbatest/explprod.test,
src/tgbatest/nondet.test, src/tgbatest/tripprod.test: Adjust tests.
* NEWS: Mention the new order.

src/ltlast/formula.cc

@@ -20,9 +20,14 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+#include "config.h"
 #include "formula.hh"
 #include "misc/hash.hh"
+#include "misc/casts.hh"
 #include <iostream>
+#include "unop.hh"
+#include "atomic_prop.hh"
+#include <string.h>
 
 namespace spot
 {
@@ -114,5 +119,22 @@ namespace spot
 
       return out;
     }
+
+
+    const formula* is_literal(const formula* f)
+    {
+      const unop* g = is_Not(f);
+      if (g)
+	f = g->child();
+      return is_atomic_prop(f);
+    }
+
+    int atomic_prop_cmp(const formula* f, const formula* g)
+    {
+      const atomic_prop* a = down_cast<const atomic_prop*>(f);
+      const atomic_prop* b = down_cast<const atomic_prop*>(g);
+      return strverscmp(a->name().c_str(), b->name().c_str());
+    }
+
   }
 }

src/ltlast/formula.hh

@@ -354,6 +354,20 @@ namespace spot
       opkind kind_;
     };
 
+
+    /// \brief Change \a f into <code>a</code> if it is equal to
+    /// <code>!a</code> or <code>a</code>.
+    ///
+    /// Return 0 otherwise.
+    SPOT_API
+    const formula* is_literal(const formula* f);
+
+
+    /// Compare two atomic propositions.
+    SPOT_API
+    int atomic_prop_cmp(const formula* f, const formula* g);
+
+
     /// \ingroup ltl_essentials
     /// \brief Strict Weak Ordering for <code>const formula*</code>.
     ///
@@ -377,6 +391,67 @@ namespace spot
 	assert(right);
 	if (left == right)
 	  return false;
+
+	size_t l = left->hash();
+	size_t r = right->hash();
+	if (l != r)
+	  return l < r;
+	// Because the hash code assigned to each formula is the
+	// number of formulae constructed so far, it is very unlikely
+	// that we will ever reach a case were two different formulae
+	// have the same hash.  This will happen only ever with have
+	// produced 256**sizeof(size_t) formulae (i.e. max_count has
+	// looped back to 0 and started over).  In that case we can
+	// order two formulae by looking at their text representation.
+	// We could be more efficient and look at their AST, but it's
+	// not worth the burden.  (Also ordering pointers is ruled out
+	// because it breaks the determinism of the implementation.)
+	return left->dump() < right->dump();
+      }
+    };
+
+    /// \brief Strict Weak Ordering for <code>const formula*</code>
+    ///        inside ltl::multop.
+    /// \ingroup ltl_essentials
+    ///
+    /// This is the comparison functor used by to order the
+    /// ltl::multop operands.  It keeps Boolean formulae first in
+    /// order to speed up implication checks.
+    ///
+    /// Also keep literal alphabetically ordered.
+    struct formula_ptr_less_than_multop:
+      public std::binary_function<const formula*, const formula*, bool>
+    {
+      bool
+      operator()(const formula* left, const formula* right) const
+      {
+	assert(left);
+	assert(right);
+	if (left == right)
+	  return false;
+
+	// We want Boolean formulae first.
+	bool lib = left->is_boolean();
+	if (lib != right->is_boolean())
+	  return lib;
+
+	// We have two Boolean formulae
+	if (lib)
+	  {
+	    // Literals should come first
+	    const formula* litl = is_literal(left);
+	    const formula* litr = is_literal(right);
+	    if (!litl != !litr)
+	      return litl;
+	    if (litl)
+	      {
+		// And they should be sorted alphabetically
+		int cmp = atomic_prop_cmp(litl, litr);
+		if (cmp)
+		  return cmp < 0;
+	      }
+	  }
+
 	size_t l = left->hash();
 	size_t r = right->hash();
 	if (l != r)

src/ltlast/multop.cc

@@ -199,13 +199,13 @@ namespace spot
 	      }
 	  }
 	// - AndNLM(Exps1...,Bool1,Exps2...,Bool2,Exps3...) =
-	//    AndNLM(Exps1...,Exps2...,Exps3...,And(Bool1,Bool2))
+	//    AndNLM(And(Bool1,Bool2),Exps1...,Exps2...,Exps3...)
 	// - AndRat(Exps1...,Bool1,Exps2...,Bool2,Exps3...) =
-	//    AndRat(Exps1...,Exps2...,Exps3...,And(Bool1,Bool2))
+	//    AndRat(And(Bool1,Bool2),Exps1...,Exps2...,Exps3...)
 	// - OrRat(Exps1...,Bool1,Exps2...,Bool2,Exps3...) =
-	//    AndRat(Exps1...,Exps2...,Exps3...,Or(Bool1,Bool2))
+	//    AndRat(Or(Bool1,Bool2),Exps1...,Exps2...,Exps3...)
 	if (!b->empty())
-	  v->push_back(multop::instance(op, b));
+	  v->insert(v->begin(), multop::instance(op, b));
 	else
 	  delete b;
       }
@@ -240,18 +240,15 @@ namespace spot
 		i = v->erase(i);
 		continue;
 	      }
-	    if (const multop* p = is_multop(*i))
+	    if (const multop* p = is_multop(*i, op))
 	      {
-		if (p->op() == op)
-		  {
-		    unsigned ps = p->size();
-		    for (unsigned n = 0; n < ps; ++n)
-		      inlined.push_back(p->nth(n)->clone());
-		    (*i)->destroy();
-		    // FIXME: Do not use erase.  See previous FIXME.
-		    i = v->erase(i);
-		    continue;
-		  }
+		unsigned ps = p->size();
+		for (unsigned n = 0; n < ps; ++n)
+		  inlined.push_back(p->nth(n)->clone());
+		(*i)->destroy();
+		// FIXME: Do not use erase.  See previous FIXME.
+		i = v->erase(i);
+		continue;
 	      }
 	    // All operator except "Concat" and "Fusion" are
 	    // commutative, so we just keep a list of the inlined
@@ -263,13 +260,13 @@ namespace spot
 	    ++i;
 	  }
 	if (op == Concat || op == Fusion)
-	  *v = inlined;
+	  v->swap(inlined);
 	else
 	  v->insert(v->end(), inlined.begin(), inlined.end());
       }
 
       if (op != Concat && op != Fusion)
-	std::sort(v->begin(), v->end(), formula_ptr_less_than());
+	std::sort(v->begin(), v->end(), formula_ptr_less_than_multop());
 
       unsigned orig_size = v->size();