* iface/gspn/gspn.cc, src/ltlvisit/basicreduce.cc,

src/ltlvisit/destroy.cc, src/ltlvisit/dotty.cc,
src/ltlvisit/dump.cc, src/ltlvisit/length.cc,
src/ltlvisit/nenoform.cc, src/ltlvisit/reduce.cc,
src/ltlvisit/syntimpl.cc, src/ltlvisit/tostring.cc,
src/tgba/formula2bdd.cc, src/tgba/tgbabddconcreteproduct.cc,
src/tgba/tgbatba.cc, src/tgbaalgos/dotty.cc,
src/tgbaalgos/dupexp.cc, src/tgbaalgos/lbtt.cc,
src/tgbaalgos/ltl2tgba_lacim.cc, src/tgbaalgos/neverclaim.cc,
src/tgbaalgos/save.cc, src/tgbaalgos/stats.cc,
src/tgbaalgos/gtec/nsheap.cc, src/tgbaalgos/gtec/nsheap.hh:
Declare private classes and helper function in anonymous namespaces.
* HACKING, src/sanity/style.test: Document and check this.
Also check for trailing { after namespace or class.
* src/ltlast/predecl.hh, src/ltlast/visitor.hh,
src/tgba/tgbareduc.hh: Fix trailing {.

ChangeLog

@@ -1,3 +1,22 @@
+2004-10-18  Alexandre Duret-Lutz  <adl@src.lip6.fr>
+
+	* iface/gspn/gspn.cc, src/ltlvisit/basicreduce.cc,
+	src/ltlvisit/destroy.cc, src/ltlvisit/dotty.cc,
+	src/ltlvisit/dump.cc, src/ltlvisit/length.cc,
+	src/ltlvisit/nenoform.cc, src/ltlvisit/reduce.cc,
+	src/ltlvisit/syntimpl.cc, src/ltlvisit/tostring.cc,
+	src/tgba/formula2bdd.cc, src/tgba/tgbabddconcreteproduct.cc,
+	src/tgba/tgbatba.cc, src/tgbaalgos/dotty.cc,
+	src/tgbaalgos/dupexp.cc, src/tgbaalgos/lbtt.cc,
+	src/tgbaalgos/ltl2tgba_lacim.cc, src/tgbaalgos/neverclaim.cc,
+	src/tgbaalgos/save.cc, src/tgbaalgos/stats.cc,
+	src/tgbaalgos/gtec/nsheap.cc, src/tgbaalgos/gtec/nsheap.hh:
+	Declare private classes and helper function in anonymous namespaces.
+	* HACKING, src/sanity/style.test: Document and check this.
+	Also check for trailing { after namespace or class.
+	* src/ltlast/predecl.hh, src/ltlast/visitor.hh,
+	src/tgba/tgbareduc.hh: Fix trailing {.
+
 2004-10-15  Alexandre Duret-Lutz  <adl@src.lip6.fr>
 
 	* src/tgbaalgos/gtec/ce.cc: Reinstall change from 2004-09-21.

HACKING

@@ -249,3 +249,9 @@ Other style recommandations
     Prefer <iosfwd> when predeclarations are sufficient, and then
     use for instance use just <ostream> in the corresponding .cc file.
     (A plain <iostream> is needed when using std::cout, std::cerr, etc.)
+
+  * Always declare helper functions and other local class definitions
+    (used in a single .cc files) in anonymous namespaces.  (The risk
+    otherwise is to declare two classes with the same name: the linker
+    will ignore one of the two silently.  The resulting bugs are often
+    difficult to understand.)

iface/gspn/gspn.cc

@@ -27,6 +27,8 @@
 namespace spot
 {
 
+  namespace
+  {
     // state_gspn
     //////////////////////////////////////////////////////////////////////
 
@@ -461,6 +463,8 @@ namespace spot
       return bddtrue;
     }
 
+  } // anonymous
+
   // gspn_interface
   //////////////////////////////////////////////////////////////////////
 

src/ltlast/predecl.hh

@@ -29,8 +29,10 @@
 #ifndef SPOT_LTLAST_PREDECL_HH
 # define SPOT_LTLAST_PREDECL_HH
 
-namespace spot {
+namespace spot
-  namespace ltl {
+{
+  namespace ltl
+  {
     struct visitor;
     struct const_visitor;
 

src/ltlast/visitor.hh

@@ -26,9 +26,10 @@
 
 #include "predecl.hh"
 
-namespace spot {
+namespace spot
-  namespace ltl {
+{
-
+  namespace ltl
+  {
     /// \brief Formula visitor that can modify the formula.
     ///
     /// Writing visitors is the prefered way

src/ltlvisit/basicreduce.cc

@@ -31,7 +31,6 @@ namespace spot
 {
   namespace ltl
   {
-
     bool
     is_GF(const formula* f)
     {
@@ -58,8 +57,8 @@ namespace spot
       return false;
     }
 
-    formula* basic_reduce(const formula* f);
+    namespace
-
+    {
       class basic_reduce_visitor: public visitor
       {
       public:
@@ -333,7 +332,8 @@ namespace spot
 			  // (a U b) & (c U b) = (a & c) U b
 			  formula* ftmp = dynamic_cast<binop*>(*i)->second();
 			  multop::vec* tmpUright = new multop::vec;
-			for (multop::vec::iterator j = i; j != res->end(); j++)
+			  for (multop::vec::iterator j = i; j != res->end();
+			       j++)
 			    {
 			      if (!*j)
 				continue;
@@ -363,7 +363,8 @@ namespace spot
 			  // (a R b) & (a R c) = a R (b & c)
 			  formula* ftmp = dynamic_cast<binop*>(*i)->first();
 			  multop::vec* tmpRright = new multop::vec;
-			for (multop::vec::iterator j = i; j != res->end(); j++)
+			  for (multop::vec::iterator j = i; j != res->end();
+			       j++)
 			    {
 			      if (!*j)
 				continue;
@@ -442,7 +443,8 @@ namespace spot
 			  // (a U b) | (a U c) = a U (b | c)
 			  formula* ftmp = bo->first();
 			  multop::vec* tmpUright = new multop::vec;
-			for (multop::vec::iterator j = i; j != res->end(); j++)
+			  for (multop::vec::iterator j = i; j != res->end();
+			       j++)
 			    {
 			      if (!*j)
 				continue;
@@ -470,7 +472,8 @@ namespace spot
 			  // (a R b) | (c R b) = (a | c) R b
 			  formula* ftmp = dynamic_cast<binop*>(*i)->second();
 			  multop::vec* tmpRright = new multop::vec;
-			for (multop::vec::iterator j = i; j != res->end(); j++)
+			  for (multop::vec::iterator j = i; j != res->end();
+			       j++)
 			    {
 			      if (!*j)
 				continue;
@@ -579,6 +582,7 @@ namespace spot
       protected:
 	formula* result_;
       };
+    }
 
     formula*
     basic_reduce(const formula* f)

src/ltlvisit/destroy.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.
 //
@@ -25,7 +25,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class destroy_visitor: public postfix_visitor
       {
       public:
@@ -35,6 +36,7 @@ namespace spot
 	  formula::unref(c);
 	}
       };
+    }
 
     void
     destroy(const formula* f)

src/ltlvisit/dotty.cc

@@ -29,7 +29,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class dotty_visitor: public const_visitor
       {
       public:
@@ -122,6 +123,7 @@ namespace spot
 	  return true;
 	}
       };
+    }
 
     std::ostream&
     dotty(std::ostream& os, const formula* f)

src/ltlvisit/dump.cc

@@ -28,7 +28,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class dump_visitor: public const_visitor
       {
       public:
@@ -88,6 +89,7 @@ namespace spot
       private:
 	std::ostream& os_;
       };
+    }
 
     std::ostream&
     dump(std::ostream& os, const formula* f)

src/ltlvisit/length.cc

@@ -27,7 +27,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class length_visitor: public const_visitor
       {
       public:
@@ -83,6 +84,7 @@ namespace spot
       protected:
 	int result_; // size of the formula
       };
+    }
 
     int
     length(const formula* f)

src/ltlvisit/nenoform.cc

@@ -27,7 +27,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class negative_normal_form_visitor: public visitor
       {
       public:
@@ -93,11 +94,13 @@ namespace spot
 	      return;
 	    case unop::F:
 	      /* !Fa == G!a */
-	    result_ = unop::instance(negated_ ? unop::G : unop::F, recurse(f));
+	      result_ = unop::instance(negated_ ? unop::G : unop::F,
+				       recurse(f));
 	      return;
 	    case unop::G:
 	      /* !Ga == F!a */
-	    result_ = unop::instance(negated_ ? unop::F : unop::G, recurse(f));
+	      result_ = unop::instance(negated_ ? unop::F : unop::G,
+				       recurse(f));
 	      return;
 	    }
 	  /* Unreachable code.  */
@@ -187,6 +190,7 @@ namespace spot
 	formula* result_;
 	bool negated_;
       };
+    }
 
     formula*
     negative_normal_form(const formula* f, bool negated)

src/ltlvisit/reduce.cc

@@ -34,7 +34,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       class reduce_visitor: public visitor
       {
       public:
@@ -287,6 +288,8 @@ namespace spot
 	int opt_;
       };
 
+    } // anonymous
+
     formula*
     reduce(const formula* f, int opt)
     {

src/ltlvisit/syntimpl.cc

@@ -31,6 +31,8 @@
 namespace spot
 {
   namespace ltl
+  {
+    namespace
     {
 
       class eventual_universal_visitor: public const_visitor
@@ -162,22 +164,6 @@ namespace spot
       };
 
 
-    bool
-    is_eventual(const formula* f)
-    {
-      eventual_universal_visitor v;
-      const_cast<formula*>(f)->accept(v);
-      return v.is_eventual();
-    }
-
-    bool
-    is_universal(const formula* f)
-    {
-      eventual_universal_visitor v;
-      const_cast<formula*>(f)->accept(v);
-      return v.is_universal();
-    }
-
       /////////////////////////////////////////////////////////////////////////
 
 
@@ -528,6 +514,24 @@ namespace spot
 	const formula* f;
       };
 
+    } // anonymous
+
+    bool
+    is_eventual(const formula* f)
+    {
+      eventual_universal_visitor v;
+      const_cast<formula*>(f)->accept(v);
+      return v.is_eventual();
+    }
+
+    bool
+    is_universal(const formula* f)
+    {
+      eventual_universal_visitor v;
+      const_cast<formula*>(f)->accept(v);
+      return v.is_universal();
+    }
+
     // This is called by syntactic_implication() after the
     // formulae have been normalized.
     bool

src/ltlvisit/tostring.cc

@@ -32,7 +32,8 @@ namespace spot
 {
   namespace ltl
   {
-
+    namespace
+    {
       static bool
       is_bare_word(const char* str)
       {
@@ -184,6 +185,7 @@ namespace spot
 	std::ostream& os_;
 	bool top_level_;
       };
+    }
 
     std::ostream&
     to_string(const formula* f, std::ostream& os)

src/sanity/style.test

@@ -72,6 +72,12 @@ for dir in "${INCDIR-..}" "${INCDIR-..}"/../iface; do
     grep '[	 ]switch (.*).*{' $tmp &&
       diag 'Opening { should be on its own line.'
 
+    grep 'namespace .*{' $tmp &&
+      diag 'Opening { should be on its own line.'
+
+    grep 'class .*{' $tmp &&
+      diag 'Opening { should be on its own line.'
+
     grep '( ' $tmp &&
       diag 'No space after opening (.'
 
@@ -137,6 +143,15 @@ for dir in "${INCDIR-..}" "${INCDIR-..}"/../iface; do
             diag 'Avoid <iostream> in headers, better use <iosfwd>.'
         fi
         ;;
+      *.cc)
+        if grep 'namespace$' $tmp >/dev/null; then
+	  :
+	else
+	  # We only check classes, but the rule should apply to functions too
+	  grep '^[	 ]*class[	 ]' $tmp &&
+	    diag 'Private definitions must be in anonymous namespace.'
+	fi
+	;;
     esac
 
 

src/tgba/formula2bdd.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.
 //
@@ -30,6 +30,8 @@ namespace spot
 {
   using namespace ltl;
 
+  namespace
+  {
     class formula_to_bdd_visitor: public ltl::const_visitor
     {
     public:
@@ -150,14 +152,6 @@ namespace spot
       bdd res_;
     };
 
-  bdd
-  formula_to_bdd(const formula* f, bdd_dict* d, void* for_me)
-  {
-    formula_to_bdd_visitor v(d, for_me);
-    f->accept(v);
-    return v.result();
-  }
-
     // Convert a BDD which is known to be a conjonction into a formula.
     static ltl::formula*
     conj_to_formula(bdd b, const bdd_dict* d)
@@ -190,6 +184,16 @@ namespace spot
       return multop::instance(multop::And, v);
     }
 
+  } // anonymous
+
+  bdd
+  formula_to_bdd(const formula* f, bdd_dict* d, void* for_me)
+  {
+    formula_to_bdd_visitor v(d, for_me);
+    f->accept(v);
+    return v.result();
+  }
+
   const formula*
   bdd_to_formula(bdd f, const bdd_dict* d)
   {

src/tgba/tgbabddconcreteproduct.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.
 //
@@ -23,6 +23,8 @@
 #include "tgbabddconcreteproduct.hh"
 
 namespace spot
+{
+  namespace
   {
     /// \brief Helper class for product().
     ///
@@ -73,6 +75,7 @@ namespace spot
       tgba_bdd_core_data data_;
       bdd init_;
     };
+  }
 
   tgba_bdd_concrete*
   product(const tgba_bdd_concrete* left, const tgba_bdd_concrete* right)

src/tgba/tgbareduc.hh

@@ -39,8 +39,12 @@ namespace spot
     typedef Sgi::vector<state_couple*> delayed_simulation_relation;
   */
 
-  class direct_simulation_relation : public simulation_relation{};
+  class direct_simulation_relation: public simulation_relation
-  class delayed_simulation_relation : public simulation_relation{};
+  {
+  };
+  class delayed_simulation_relation: public simulation_relation
+  {
+  };
 
 
   class tgba_reduc: public tgba_explicit,

src/tgba/tgbatba.cc

@@ -26,7 +26,8 @@
 
 namespace spot
 {
-
+  namespace
+  {
     /// \brief A state for spot::tgba_tba_proxy.
     ///
     /// This state is in fact a pair of state: the state from the tgba
@@ -104,7 +105,8 @@ namespace spot
     };
 
 
-  /// \brief Iterate over the successors of tgba_tba_proxy computed on the fly.
+    /// \brief Iterate over the successors of tgba_tba_proxy computed
+    /// on the fly.
     class tgba_tba_proxy_succ_iterator: public tgba_succ_iterator
     {
       typedef tgba_tba_proxy::cycle_list list;
@@ -191,6 +193,7 @@ namespace spot
       const bdd the_acceptance_cond_;
     };
 
+  } // anonymous
 
   tgba_tba_proxy::tgba_tba_proxy(const tgba* a)
     : a_(a)

src/tgbaalgos/dotty.cc

@@ -28,7 +28,8 @@
 
 namespace spot
 {
-
+  namespace
+  {
     class dotty_bfs : public tgba_reachable_iterator_breadth_first
     {
     public:
@@ -73,6 +74,7 @@ namespace spot
     private:
       std::ostream& os_;
     };
+  }
 
   std::ostream&
   dotty_reachable(std::ostream& os, const tgba* g)

src/tgbaalgos/dupexp.cc

@@ -25,8 +25,10 @@
 #include <map>
 #include "reachiter.hh"
 
-namespace spot {
+namespace spot
-
+{
+  namespace
+  {
     template <class T>
     class dupexp_iter: public T
     {
@@ -93,6 +95,8 @@ namespace spot {
       std::map<int, std::string> name_;
     };
 
+  } // anonymous
+
   tgba_explicit*
   tgba_dupexp_bfs(const tgba* aut)
   {

src/tgbaalgos/gtec/nsheap.cc

@@ -22,6 +22,8 @@
 #include "nsheap.hh"
 
 namespace spot
+{
+  namespace
   {
     class numbered_state_heap_hash_map_const_iterator:
       public numbered_state_heap_const_iterator
@@ -71,6 +73,7 @@ namespace spot
       numbered_state_heap_hash_map::hash_type::const_iterator i;
       const numbered_state_heap_hash_map::hash_type& h;
     };
+  } // anonymous
 
   numbered_state_heap_hash_map::~numbered_state_heap_hash_map()
   {

src/tgbaalgos/gtec/nsheap.hh

@@ -119,12 +119,11 @@ namespace spot
     virtual int size() const;
 
     virtual numbered_state_heap_const_iterator* iterator() const;
-  protected:
+
     typedef Sgi::hash_map<const state*, int,
 			  state_ptr_hash, state_ptr_equal> hash_type;
+  protected:
     hash_type h;		///< Map of visited states.
-
-    friend class numbered_state_heap_hash_map_const_iterator;
   };
 
   /// \brief Factory for numbered_state_heap_hash_map.

src/tgbaalgos/lbtt.cc

@@ -29,6 +29,8 @@
 #include "misc/bddlt.hh"
 
 namespace spot
+{
+  namespace
   {
     // At some point we'll need to print an acceptance set into LBTT's
     // format.  LBTT expects numbered acceptance sets, so first we'll
@@ -153,6 +155,7 @@ namespace spot
       acceptance_cond_splitter acs_;
     };
 
+  } // anonymous
 
   std::ostream&
   lbtt_reachable(std::ostream& os, const tgba* g)
@@ -161,6 +164,4 @@ namespace spot
     b.run();
     return os;
   }
-
-
 }

src/tgbaalgos/ltl2tgba_lacim.cc

@@ -30,6 +30,8 @@
 #include "ltl2tgba_lacim.hh"
 
 namespace spot
+{
+  namespace
   {
     using namespace ltl;
 
@@ -250,6 +252,7 @@ namespace spot
       tgba_bdd_concrete_factory& fact_;
       bool root_;
     };
+  } // anonymous
 
   tgba_bdd_concrete*
   ltl_to_tgba_lacim(const ltl::formula* f, bdd_dict* dict)

src/tgbaalgos/neverclaim.cc

@@ -32,7 +32,8 @@
 
 namespace spot
 {
-
+  namespace
+  {
     class never_claim_bfs : public tgba_reachable_iterator_breadth_first
     {
     public:
@@ -179,6 +180,7 @@ namespace spot
       bool fi_needed_;
       state* init_;
     };
+  } // anonymous
 
   std::ostream&
   never_claim_reachable(std::ostream& os, const tgba_tba_proxy* g,

src/tgbaalgos/save.cc

@@ -28,7 +28,8 @@
 
 namespace spot
 {
-
+  namespace
+  {
     class save_bfs: public tgba_reachable_iterator_breadth_first
     {
     public:
@@ -93,7 +94,7 @@ namespace spot
 	return os_;
       }
     };
-
+  }
 
   std::ostream&
   tgba_save_reachable(std::ostream& os, const tgba* g)

src/tgbaalgos/stats.cc

@@ -25,7 +25,8 @@
 
 namespace spot
 {
-
+  namespace
+  {
     class stats_bfs: public tgba_reachable_iterator_breadth_first
     {
     public:
@@ -49,6 +50,7 @@ namespace spot
     private:
       tgba_statistics& s_;
     };
+  } // anonymous
 
   tgba_statistics
   stats_reachable(const tgba* g)