* src/sanity/style.test: Warn about places where size() is used

instead of empty().
* src/misc/bddalloc.cc (bdd_allocator::extend): Use empty() rather
than size() when checking emptiness of lists.
* src/tgbaalgos/reductgba_sim_del.cc, src/tgbaalgos/minimalce.cc,
src/ltlvisit/basicreduce.cc, src/ltlvisit/reduce.cc,
src/tgbaalgos/gtec/ce.cc, src/tgbaalgos/ltl2tgba_fm.cc,
src/misc/minato.cc: Likewise.
* src/ltlast/multop.cc (multop::instance): Call ->size() only once.

ChangeLog

@@ -1,3 +1,15 @@
+2004-09-21  Alexandre Duret-Lutz  <adl@src.lip6.fr>
+
+	* src/sanity/style.test: Warn about places where size() is used
+	instead of empty().
+	* src/misc/bddalloc.cc (bdd_allocator::extend): Use empty() rather
+	than size() when checking emptiness of lists.
+	* src/tgbaalgos/reductgba_sim_del.cc, src/tgbaalgos/minimalce.cc,
+	src/ltlvisit/basicreduce.cc, src/ltlvisit/reduce.cc,
+	src/tgbaalgos/gtec/ce.cc, src/tgbaalgos/ltl2tgba_fm.cc,
+	src/misc/minato.cc: Likewise.
+	* src/ltlast/multop.cc (multop::instance): Call ->size() only once.
+
 2004-09-20  Alexandre Duret-Lutz  <adl@src.lip6.fr>
 
 	Update to SWIG 1.3.22.

src/ltlast/multop.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2003  Laboratoire d'Informatique de Paris 6 (LIP6),
+// Copyright (C) 2003, 2004  Laboratoire d'Informatique de Paris 6 (LIP6),
 // département Systèmes Répartis Coopératifs (SRC), Université Pierre
 // et Marie Curie.
 //
@@ -152,7 +152,8 @@ namespace spot
 	  }
 	}
 
-      if (v->size() == 0)
+      vec::size_type s = v->size();
+      if (s == 0)
 	{
 	  delete v;
 	  switch (op)
@@ -165,7 +166,7 @@ namespace spot
 	  /* Unreachable code.  */
 	  assert(0);
 	}
-      if (v->size() == 1)
+      else if (s == 1)
 	{
 	  formula* res = (*v)[0];
 	  delete v;

src/ltlvisit/basicreduce.cc

@@ -101,12 +101,12 @@ namespace spot
 	    res1->push_back(clone(mo->nth(i)));
 	destroy(mo);
 	multop::vec* res3 = new multop::vec;
-	if (res1->size())
+	if (!res1->empty())
 	  res3->push_back(unop::instance(op,
 					 multop::instance(op_child, res1)));
 	else
 	  delete res1;
-	if (resGF->size())
+	if (!resGF->empty())
 	  res3->push_back(multop::instance(op_child, resGF));
 	else
 	  delete resGF;
@@ -518,27 +518,27 @@ namespace spot
 	delete res;
 
 
-	if (tmpX && tmpX->size())
+	if (tmpX && !tmpX->empty())
 	  tmpOther->push_back(unop::instance(unop::X,
 					     multop::instance(mo->op(),
 							      tmpX)));
-	else if (tmpX && !tmpX->size())
+	else if (tmpX)
 	  delete tmpX;
 
 
-	if (tmpU && tmpU->size())
+	if (tmpU && !tmpU->empty())
 	  tmpOther->push_back(multop::instance(mo->op(), tmpU));
-	else if (tmpU && !tmpU->size())
+	else if (tmpU)
 	  delete tmpU;
 
 
-	if (tmpR && tmpR->size())
+	if (tmpR && !tmpR->empty())
 	  tmpOther->push_back(multop::instance(mo->op(), tmpR));
-	else if (tmpR && !tmpR->size())
+	else if (tmpR)
 	  delete tmpR;
 
 
-	if (tmpGF && tmpGF->size())
+	if (tmpGF && !tmpGF->empty())
 	  {
 	    formula* ftmp
 	      = unop::instance(unop::G,
@@ -547,11 +547,11 @@ namespace spot
 							       tmpGF)));
 	    tmpOther->push_back(ftmp);
 	  }
-	else if (tmpGF && !tmpGF->size())
+	else if (tmpGF)
 	  delete tmpGF;
 
 
-	if (tmpFG && tmpFG->size())
+	if (tmpFG && !tmpFG->empty())
 	  {
 	    formula* ftmp = 0;
 	    if (mo->op() == multop::And)
@@ -566,7 +566,7 @@ namespace spot
 				 multop::instance(mo->op(), tmpFG));
 	    tmpOther->push_back(ftmp);
 	  }
-	else if (tmpFG && !tmpFG->size())
+	else if (tmpFG)
 	  delete tmpFG;
 
 

src/ltlvisit/reduce.cc

@@ -268,7 +268,7 @@ namespace spot
 
 	  }
 
-	if (res->size())
+	if (!res->empty())
 	  {
 	    result_ = multop::instance(mo->op(), res);
 	    return;

src/misc/bddalloc.cc

@@ -85,7 +85,7 @@ namespace spot
   {
     // If we already have some free variable at the end
     // of the variable space, allocate just the difference.
-    if (fl.size() > 0 && fl.back().first + fl.back().second == lvarnum)
+    if (!fl.empty() && fl.back().first + fl.back().second == lvarnum)
       {
 	int res = fl.back().first;
 	int endvar = fl.back().second;

src/misc/minato.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2003  Laboratoire d'Informatique de Paris 6 (LIP6),
+// Copyright (C) 2003, 2004  Laboratoire d'Informatique de Paris 6 (LIP6),
 // département Systèmes Répartis Coopératifs (SRC), Université Pierre
 // et Marie Curie.
 //
@@ -44,7 +44,7 @@ namespace spot
   bdd
   minato_isop::next()
   {
-    while (todo_.size())
+    while (!todo_.empty())
       {
 	local_vars& l = todo_.top();
 	switch (l.step)

src/sanity/style.test

@@ -108,18 +108,23 @@ for dir in "${INCDIR-..}" "${INCDIR-..}"/../iface; do
     grep '{.*{' $tmp &&
       diag 'No two { on the same line.'
 
-    grep -e 'delete[	 ]*[(][^(]*[)];' $tmp &&
+    grep 'delete[	 ]*[(][^(]*[)];' $tmp &&
       diag 'No useless parentheses after delete.'
 
-    grep -e 'return[	 ]*[(][^(]*[)];' $tmp &&
+    grep 'return[	 ]*[(][^(]*[)];' $tmp &&
       diag 'No useless parentheses after return.'
 
     grep 'NULL' $tmp &&
       diag 'Use 0 instead of NULL.  NULL is not portable.'
 
+    # std::list::size() can be O(n).  Better use empty() whenever
+    # possible, even for other containers.
+    grep -E '(->|[.])size\(\) [=!]= 0|![a-zA-Z0-9_]*(->|[.])size\(\)|(if |while |assert)\([a-zA-Z0-9_]*(->|[.])size\(\)\)' $tmp &&
+      diag 'Prefer empty() to check emptiness.'
+
     case $file in
       *.hh | *.hxx)
-        if grep -e '(cout|cerr|clog)' $tmp >/dev/null; then
+        if grep -E '(cout|cerr|clog)' $tmp >/dev/null; then
           :
 	else
           grep '#.*include.*<iostream>' $tmp &&

src/tgbaalgos/gtec/ce.cc

@@ -338,9 +338,9 @@ namespace spot
 		todo.pop();
 		delete iter;
 		seen.erase(s);
-		if (todo.size())
+		if (!todo.empty())
 		  {
-		    assert(path.size());
+		    assert(!path.empty());
 		    path.pop_back();
 		  }
 		continue;
@@ -377,7 +377,7 @@ namespace spot
 
 	    // If we already have a best path, let see if the current
 	    // one is better.
-	    if (best_path.size())
+	    if (!best_path.empty())
 	      {
 		// When comparing the merits of two paths, only the
 		// acceptance conditions we are trying the traverse
@@ -491,9 +491,9 @@ namespace spot
 		todo.pop();
 		delete iter;
 		seen.erase(s);
-		if (todo.size())
+		if (!todo.empty())
 		  {
-		    assert(path.size());
+		    assert(!path.empty());
 		    path.pop_back();
 		  }
 		continue;
@@ -530,7 +530,7 @@ namespace spot
 
 	    // If we already have a best path, let see if the current
 	    // one is better.
-	    if (best_path.size())
+	    if (!best_path.empty())
 	      {
 		// When comparing the merits of two paths, only the
 		// acceptance conditions we are trying the traverse

src/tgbaalgos/ltl2tgba_fm.cc

@@ -552,7 +552,7 @@ namespace spot
 
       ~formula_canonizer()
       {
-	while (f2b_.size())
+	while (!f2b_.empty())
 	  {
 	    formula_to_bdd_map::iterator i = f2b_.begin();
 	    const formula* f = i->first;

src/tgbaalgos/minimalce.cc

@@ -174,7 +174,7 @@ namespace spot
 	  ++id;
 	}
 
-      assert(cycle.size() != 0);
+      assert(!cycle.empty());
       is = cycle.end();
       is--;
       id = cycle.begin();
@@ -593,7 +593,7 @@ namespace spot
     recurse_find(s, os);
     //std::cout << "nb_found : " << nb_found << std::endl;
 
-    if (min_ce->size() == 0)
+    if (min_ce->empty())
       {
 	delete min_ce;
 	min_ce = 0;
@@ -629,7 +629,7 @@ namespace spot
     hash_type::iterator i;
     tgba_succ_iterator* iter = 0;
 
-    if (!h_lenght.size())
+    if (h_lenght.empty())
       {
 	// it's a new search
 	//std::cout << "it's a new search" << std::endl;
@@ -670,7 +670,7 @@ namespace spot
 
 	    const state* succ = iter->current_state();
 
-	    if ((min_ce->size() == 0) ||
+	    if (min_ce->empty() ||
 		((int)stack.size() + 1 <= min_ce->size()))
 	      {
 		int depth = in_stack(succ, os);
@@ -750,7 +750,7 @@ namespace spot
 		  }
 
 		else if ((h_lenght[succ] > (int)stack.size() + 1) &&
-			 (min_ce->size() != 0))
+			 !min_ce->empty())
 		  {
 		    s = succ;
 		    iter->next();
@@ -832,7 +832,7 @@ namespace spot
 
 	const state* succ = iter->current_state();
 
-	if ((min_ce->size() == 0) ||
+	if (min_ce->empty() ||
 	    ((int)stack.size() + 1 <= min_ce->size()))
 	  {
 	    int depth = in_stack(succ, os);
@@ -872,7 +872,7 @@ namespace spot
 		recurse_find(succ, os, mode);
 	      }
 	    else if ((h_lenght[succ] > (int)stack.size() + 1) &&
-		     (min_ce->size() != 0))
+		     !min_ce->empty())
 	      {
 		//std::cout << "recurse 3 : " << stack.size() << " ";
 		mode = careful;
@@ -946,7 +946,7 @@ namespace spot
     //std::cout << os.str() << "save counter" << std::endl;
 
     nb_found++;
-    if (!min_ce->size())
+    if (min_ce->empty())
       delete min_ce;
 
     min_ce = new ce::counter_example(a);

src/tgbaalgos/reductgba_sim_del.cc

@@ -72,7 +72,7 @@ namespace spot
     // We take the max of the progress measure of the successor node
     // because we are on a spoiler.
 
-    if (lnode_succ->size() == 0)
+    if (lnode_succ->empty())
       progress_measure_ = nb_spoiler_loose_ + 1;
 
     if (progress_measure_ >= nb_spoiler_loose_ + 1)
@@ -229,7 +229,7 @@ namespace spot
     // We take the min of the progress measure of the successor node
     // because we are on a duplicator.
 
-    if (lnode_succ->size() == 0)
+    if (lnode_succ->empty())
       progress_measure_ = nb_spoiler_loose_ + 1;
 
     if (progress_measure_ >= nb_spoiler_loose_ + 1)