Add "%define assert" to variants.

This is used to help the user catch cases where some value gets
ovewritten by a new one.  This should not happen, as this will
probably leak.

Unfortunately this uncovered a bug in the C++ parser itself: the
lookahead value was not destroyed between two calls to yylex.  For
instance if the previous lookahead was a std::string, and then an int,
then the value of the std::string was correctly taken (i.e., the
lookahead was now an empty string), but std::string structure itself
was not reclaimed.

This is now done in variant::build(other&) (which is used to take the
value of the lookahead): other is not only stolen from its value, it
is also destroyed.  This incurs a new performance penalty of a few
percent, and union becomes faster again.

	* data/lalr1-fusion.cc (variant::build(other&)): Destroy other.
	(b4_variant_if): New.
	(variant::built): New.
	Use it whereever the status of the variant changes.
	* etc/bench.pl.in: Check the penalty of %define assert.

diff --git a/ChangeLog b/ChangeLog
index 8054b254801d436d08ebd5456c641b5720f1679a..b7cac25103876499d1063ef5cef836d9a393cf5f 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2008-11-07  Akim Demaille  <demaille@gostai.com>
+
+       Add "%define assert" to variants.
+       This is used to help the user catch cases where some value gets
+       ovewritten by a new one.  This should not happen, as this will
+       probably leak.
+       
+       Unfortunately this uncovered a bug in the C++ parser itself: the
+       lookahead value was not destroyed between two calls to yylex.  For
+       instance if the previous lookahead was a std::string, and then an int,
+       then the value of the std::string was correctly taken (i.e., the
+       lookahead was now an empty string), but std::string structure itself
+       was not reclaimed.
+       
+       This is now done in variant::build(other&) (which is used to take the
+       value of the lookahead): other is not only stolen from its value, it
+       is also destroyed.  This incurs a new performance penalty of a few
+       percent, and union becomes faster again.
+       
+       * data/lalr1-fusion.cc (variant::build(other&)): Destroy other.
+       (b4_variant_if): New.
+       (variant::built): New.
+       Use it whereever the status of the variant changes.
+       * etc/bench.pl.in: Check the penalty of %define assert.
+

 2008-11-07  Akim Demaille  <demaille@gostai.com>
 
        Use "%define variant" in bench.pl.
 2008-11-07  Akim Demaille  <demaille@gostai.com>
 
        Use "%define variant" in bench.pl.

diff --git a/data/lalr1-fusion.cc b/data/lalr1-fusion.cc
index 63bc1d5ab57e794cda41527259526e520da8ddab..1f1129ae8a4ea4200127924adbec16a2ef539325 100644 (file)
--- a/data/lalr1-fusion.cc
+++ b/data/lalr1-fusion.cc
@@ -40,6 +40,12 @@ b4_variant_if([
 ]) # b4_variant_if
 
 
 ]) # b4_variant_if
 
 

+# b4_assert_if([IF-ASSERTIONS-ARE-USED], [IF-NOT])
+# ----------------------------------------------------
+m4_define([b4_assert_if],
+[b4_percent_define_ifdef([[assert]], [$1], [$2])])
+
+

 # b4_rhs_value(RULE-LENGTH, NUM, [TYPE])
 # --------------------------------------
 # Expansion of $<TYPE>NUM, where the current rule has RULE-LENGTH
 # b4_rhs_value(RULE-LENGTH, NUM, [TYPE])
 # --------------------------------------
 # Expansion of $<TYPE>NUM, where the current rule has RULE-LENGTH


@@ -161,6 +167,7 @@ dnl FIXME: This is wrong, we want computed header guards.
 
 ]b4_percent_code_get([[requires]])[
 
 
 ]b4_percent_code_get([[requires]])[
 

+]b4_assert_if([#include <cassert>])[

 #include <string>
 #include <iostream>
 #include "stack.hh"
 #include <string>
 #include <iostream>
 #include "stack.hh"


@@ -177,12 +184,22 @@ dnl FIXME: This is wrong, we want computed header guards.
   /// via the current state.
   template <size_t S>
   struct variant
   /// via the current state.
   template <size_t S>
   struct variant

-  {
+  {]b4_assert_if([
+    /// Whether something is contained.
+    bool built;
+
+    /// Initially uninitialized.
+    variant ()
+      : built(false)
+    {}])[
+

     /// Instantiate a \a T in here.
     template <typename T>
     inline T&
     build()
     /// Instantiate a \a T in here.
     template <typename T>
     inline T&
     build()

-    {
+    {]b4_assert_if([
+      assert(!built);
+      built = true;])[

       return *new (buffer) T;
     }
 
       return *new (buffer) T;
     }
 


@@ -190,7 +207,8 @@ dnl FIXME: This is wrong, we want computed header guards.
     template <typename T>
     inline T&
     as()
     template <typename T>
     inline T&
     as()

-    {
+    {]b4_assert_if([
+      assert(built);])[

       return reinterpret_cast<T&>(buffer);
     }
 
       return reinterpret_cast<T&>(buffer);
     }
 


@@ -198,7 +216,8 @@ dnl FIXME: This is wrong, we want computed header guards.
     template <typename T>
     inline const T&
     as() const
     template <typename T>
     inline const T&
     as() const

-    {
+    {]b4_assert_if([
+      assert(built);])[

       return reinterpret_cast<const T&>(buffer);
     }
 
       return reinterpret_cast<const T&>(buffer);
     }
 


@@ -218,6 +237,7 @@ dnl FIXME: This is wrong, we want computed header guards.
     {
       build<T>();
       swap<T>(other);
     {
       build<T>();
       swap<T>(other);

+      other.destroy<T>();

     }
 
     /// Destroy the stored \a T.
     }
 
     /// Destroy the stored \a T.


@@ -225,7 +245,8 @@ dnl FIXME: This is wrong, we want computed header guards.
     inline void
     destroy()
     {
     inline void
     destroy()
     {

-      as<T>().~T();
+      as<T>().~T();]b4_assert_if([
+      built = false;])[

     }
 
     /// A buffer large enough to store any of the semantic values.
     }
 
     /// A buffer large enough to store any of the semantic values.

diff --git a/etc/bench.pl.in b/etc/bench.pl.in
index f4f5a7e56d9847d5163f3324fc1a4ee1dac44736..97facb5195a23e9a23fa8221f4a72024921e86cb 100755 (executable)
--- a/etc/bench.pl.in
+++ b/etc/bench.pl.in
@@ -816,6 +816,7 @@ sub bench_variant_parser ()
       "f-var-deb" => ['%skeleton "lalr1-fusion.cc"', '%debug', '%define variant'],
       "f-var-dtr"       => ['%skeleton "lalr1-fusion.cc"', '%define variant', "%code {\n#define VARIANT_DESTROY\n}"],
       "f-var-deb-dtr" => ['%skeleton "lalr1-fusion.cc"', '%debug', '%define variant', "%code {\n#define VARIANT_DESTROY\n}"],
       "f-var-deb" => ['%skeleton "lalr1-fusion.cc"', '%debug', '%define variant'],
       "f-var-dtr"       => ['%skeleton "lalr1-fusion.cc"', '%define variant', "%code {\n#define VARIANT_DESTROY\n}"],
       "f-var-deb-dtr" => ['%skeleton "lalr1-fusion.cc"', '%debug', '%define variant', "%code {\n#define VARIANT_DESTROY\n}"],

+      "f-var-deb-dtr-ass" => ['%skeleton "lalr1-fusion.cc"', '%debug', '%define variant', "%code {\n#define VARIANT_DESTROY\n}", "%define assert"],

      )
     );
 }
      )
     );
 }