c++: revamp the support for variants

The current approach was too adhoc: the symbols were not sufficiently
self-contained, in particular wrt memory management.  The "new"
guideline is the one that should have been followed from the start:
let the symbols handle themslves, instead of leaving their users to
it.  It was justified by the will to avoid gratuitious moves and
copies, but the current approach does not seem to be slower, yet it
will probably be simpler to adjust to support move semantics from
C++11.

The documentation says that the %parse-param are available from the
%destructor.  In retrospect, that was a silly design decision, which
we can break for variants, as its a new feature.  It should be phased
out for non-variants too.

* data/variant.hh: A variant never knows if it stores something or
not, it is up to its users to store this information.
Yet, in parse.assert mode, make sure the empty/filled variants
are properly used.
(b4_symbol_constructor_define_): Don't call directly the symbol
constructor, to save a useless temporary.
* data/stack.hh (push): Steal the pushed value instead of duplicating
it.
This will simplify the callers of push, who handled this "move"
approach themselves.
* data/c++.m4 (basic_symbol): Let -1, as kind, denote the fact that
a symbol is empty.
This is needed for instance when shifting the lookahead: yyla
is given as argument to "push", and its value is then moved on
the stack.  But then yyla must be declared "empty" so that its
destructor won't be called.
(basic_symbol::move): New.
Move the responsibility of calling the destructor from yy_destroy
to ~basic_symbol in the case of variants.
* data/lalr1.cc (stack_symbol_type): Now a derived class from its
previous value, so that we can add a constructor from a symbol_type.
(by_state): State -1 means empty.
(yypush_): Factor, by calling one overload from the other one, and
using the new semantics of stack::push.
No longer reclaim by hand the memory from rhs symbols, since now
that we store objects with proper destructors, they will be reclaimed
automatically.
Conversely, be sure to delete yylhs.
* tests/c++.at (C++ Variant-based Symbols): New "unit" test for
symbols.

diff --git a/data/c++.m4 b/data/c++.m4
index f9fcb9d2dd01ff26f614791559a66e78e10c9876..591e3e8fc025994beed4647a1e3d8de4eafead6d 100644 (file)
--- a/data/c++.m4
+++ b/data/c++.m4
@@ -166,6 +166,9 @@ m4_define([b4_public_types_declare],
     template <typename Base>
     struct basic_symbol : Base
     {
     template <typename Base>
     struct basic_symbol : Base
     {

+      /// Alias to Base.
+      typedef Base super_type;
+

       /// Default constructor.
       inline basic_symbol ();
 ]b4_locations_if([
       /// Default constructor.
       inline basic_symbol ();
 ]b4_locations_if([


@@ -184,6 +187,7 @@ m4_define([b4_public_types_declare],
                            const semantic_type& v]b4_locations_if([,
                            const location_type& l])[);
 
                            const semantic_type& v]b4_locations_if([,
                            const location_type& l])[);
 

+      ~basic_symbol ();

       /// Assignment operator.
       inline basic_symbol& operator= (const basic_symbol& other);
 
       /// Assignment operator.
       inline basic_symbol& operator= (const basic_symbol& other);
 


@@ -209,10 +213,17 @@ m4_define([b4_public_types_declare],
       /// Constructor.
       inline by_type (token_type t);
 
       /// Constructor.
       inline by_type (token_type t);
 

+      /// Steal the type of \a that.
+      void move (by_type& that);
+

       /// The symbol type.
       /// The symbol type.

+      ///
+      /// -1 when this symbol is empty.

       int type;
 
       /// The type (corresponding to \a type).
       int type;
 
       /// The type (corresponding to \a type).

+      ///
+      /// -1 when this symbol is empty.

       inline int type_get () const;
 
       /// The token.
       inline int type_get () const;
 
       /// The token.


@@ -275,39 +286,52 @@ m4_define([b4_public_types_define],
           [const semantic_type& v],
           b4_locations_if([const location_type& l]))[)
     : Base (t)
           [const semantic_type& v],
           b4_locations_if([const location_type& l]))[)
     : Base (t)

-    , value ()]b4_locations_if([
+    , value (]b4_variant_if([], [v])[)]b4_locations_if([

     , location (l)])[
     , location (l)])[

-  {
-    // FIXME: The YYUSE macro is only available in the .cc skeleton files.  It
-    // is not available in .hh files, where this code is when using %defines.
+  {]b4_variant_if([[

     (void) v;
     (void) v;

-    ]b4_variant_if([b4_symbol_variant([this->type_get ()], [value], [copy],
-                                      [v])],
-                   [value = v;])[
-  }
+    ]b4_symbol_variant([this->type_get ()], [value], [copy], [v])])[}

 
   template <typename Base>
   ]b4_parser_class_name[::basic_symbol<Base>::basic_symbol (]b4_join(
           [typename Base::value_type t],
           b4_locations_if([const location_type& l]))[)
 
   template <typename Base>
   ]b4_parser_class_name[::basic_symbol<Base>::basic_symbol (]b4_join(
           [typename Base::value_type t],
           b4_locations_if([const location_type& l]))[)

-    : Base (t)]b4_locations_if([
+    : Base (t)
+    , value ()]b4_locations_if([

     , location (l)])[
   {}
 
     , location (l)])[
   {}
 

+  template <typename Base>
+  inline
+  ]b4_parser_class_name[::basic_symbol<Base>::~basic_symbol ()
+  {]b4_variant_if([[
+    // User destructor.
+    int yytype = this->type_get ();
+    switch (yytype)
+    {
+]b4_symbol_foreach([b4_symbol_destructor])dnl
+[   default:
+      break;
+    }
+
+    // Type destructor.
+  ]b4_symbol_variant([[yytype]], [[value]], [[template destroy]])])[
+  }
+

   template <typename Base>
   void
   ]b4_parser_class_name[::basic_symbol<Base>::move (basic_symbol& s)
   {
   template <typename Base>
   void
   ]b4_parser_class_name[::basic_symbol<Base>::move (basic_symbol& s)
   {

-    this->type = s.type_get ();]b4_locations_if([
-    location = s.location;])[
-    ]b4_variant_if([b4_symbol_variant([s.type_get ()], [value], [move],
+    super_type::move(s);
+    ]b4_variant_if([b4_symbol_variant([this->type_get ()], [value], [move],

                                       [s.value])],
                                       [s.value])],

-                   [value = s.value;])[
+                   [value = s.value;])[]b4_locations_if([
+    location = s.location;])[

   }
 
   // by_type.
   ]b4_parser_class_name[::by_type::by_type ()
   }
 
   // by_type.
   ]b4_parser_class_name[::by_type::by_type ()

-     : type ()
+     : type (-1)

   {}
 
   ]b4_parser_class_name[::by_type::by_type (const by_type& other)
   {}
 
   ]b4_parser_class_name[::by_type::by_type (const by_type& other)


@@ -318,6 +342,14 @@ m4_define([b4_public_types_define],
     : type (yytranslate_ (t))
   {}
 
     : type (yytranslate_ (t))
   {}
 

+  inline
+  void
+  ]b4_parser_class_name[::by_type::move (by_type& that)
+  {
+    type = that.type;
+    that.type = -1;
+  }
+

   int
   ]b4_parser_class_name[::by_type::type_get () const
   {
   int
   ]b4_parser_class_name[::by_type::type_get () const
   {

diff --git a/data/lalr1.cc b/data/lalr1.cc
index a195602c5e0e8e4ddf0c21f9bcf9992102560f6e..5a05db768aa2ef89df32f4e1c1470a9d2630259c 100644 (file)
--- a/data/lalr1.cc
+++ b/data/lalr1.cc
@@ -281,10 +281,18 @@ b4_location_define])])[
       /// Copy constructor.
       inline by_state (const by_state& other);
 
       /// Copy constructor.
       inline by_state (const by_state& other);
 

+      void move (by_state& that)
+      {
+        state = that.state;
+        that.state = -1;
+      }
+

       /// The state.
       state_type state;
 
       /// The type (corresponding to \a state).
       /// The state.
       state_type state;
 
       /// The type (corresponding to \a state).

+      ///
+      /// -1 when empty.

       inline int type_get () const;
 
       /// The type used to store the symbol type.
       inline int type_get () const;
 
       /// The type used to store the symbol type.


@@ -292,7 +300,16 @@ b4_location_define])])[
     };
 
     /// "Internal" symbol: element of the stack.
     };
 
     /// "Internal" symbol: element of the stack.

-    typedef basic_symbol<by_state> stack_symbol_type;
+    struct stack_symbol_type : basic_symbol<by_state>
+    {
+      /// Superclass.
+      typedef basic_symbol<by_state> super_type;
+      /// Construct an empty symbol.
+      stack_symbol_type ();
+      /// Steal the contents from \a sym to build this.
+      stack_symbol_type (state_type s, symbol_type& sym);
+      stack_symbol_type& operator= (const stack_symbol_type& that);
+    };

 
     /// Stack type.
     typedef stack<stack_symbol_type> stack_type;
 
     /// Stack type.
     typedef stack<stack_symbol_type> stack_type;


@@ -512,7 +529,7 @@ m4_if(b4_prefix, [yy], [],
 
   // by_state.
   ]b4_parser_class_name[::by_state::by_state ()
 
   // by_state.
   ]b4_parser_class_name[::by_state::by_state ()

-    : state ()
+    : state (-1)

   {}
 
   ]b4_parser_class_name[::by_state::by_state (const by_state& other)
   {}
 
   ]b4_parser_class_name[::by_state::by_state (const by_state& other)


@@ -526,7 +543,33 @@ m4_if(b4_prefix, [yy], [],
   int
   ]b4_parser_class_name[::by_state::type_get () const
   {
   int
   ]b4_parser_class_name[::by_state::type_get () const
   {

-    return yystos_[state];
+    return state == -1 ? -1 : yystos_[state];
+  }
+
+  inline
+  ]b4_parser_class_name[::stack_symbol_type::stack_symbol_type ()
+  {}
+
+
+  inline
+  ]b4_parser_class_name[::stack_symbol_type::stack_symbol_type (state_type s, symbol_type& sym)
+    : super_type (s]b4_locations_if([, sym.location])[)
+  {]b4_variant_if([[
+    ]b4_symbol_variant([sym.type_get ()], [value], [move], [sym.value])],
+    [value = sym.value;])[
+    // sym is emptied.
+    sym.type = -1;
+  }
+
+  inline
+  ]b4_parser_class_name[::stack_symbol_type&
+  ]b4_parser_class_name[::stack_symbol_type::operator= (const stack_symbol_type& that)
+  {
+    state = that.state;]b4_variant_if([[
+    ]b4_symbol_variant([that.type_get ()], [value], [copy], [that.value])],
+    [value = that.value;])[]b4_locations_if([
+    location = that.location;])[
+    return *this;

   }
 
 
   }
 
 


@@ -535,7 +578,7 @@ m4_if(b4_prefix, [yy], [],
   ]b4_parser_class_name[::yy_destroy_ (const char* yymsg, basic_symbol<Base>& yysym) const
   {
     if (yymsg)
   ]b4_parser_class_name[::yy_destroy_ (const char* yymsg, basic_symbol<Base>& yysym) const
   {
     if (yymsg)

-      YY_SYMBOL_PRINT (yymsg, yysym);
+      YY_SYMBOL_PRINT (yymsg, yysym);]b4_variant_if([], [

 
     // User destructor.
     int yytype = yysym.type_get ();
 
     // User destructor.
     int yytype = yysym.type_get ();


@@ -544,10 +587,7 @@ m4_if(b4_prefix, [yy], [],
 ]b4_symbol_foreach([b4_symbol_destructor])dnl
 [       default:
           break;
 ]b4_symbol_foreach([b4_symbol_destructor])dnl
 [       default:
           break;

-      }]b4_variant_if([
-
-    // Type destructor.
-  b4_symbol_variant([[yytype]], [[yysym.value]], [[template destroy]])])[
+      }])[

   }
 
 #if ]b4_api_PREFIX[DEBUG
   }
 
 #if ]b4_api_PREFIX[DEBUG


@@ -575,17 +615,8 @@ m4_if(b4_prefix, [yy], [],
   void
   ]b4_parser_class_name[::yypush_ (const char* m, state_type s, symbol_type& sym)
   {
   void
   ]b4_parser_class_name[::yypush_ (const char* m, state_type s, symbol_type& sym)
   {

-    if (m)
-      YY_SYMBOL_PRINT (m, sym);
-]b4_variant_if(
-[[
-  stack_symbol_type ss (]b4_join([s],
-      [sym.value], b4_locations_if([sym.location]))[);
-  ]b4_symbol_variant([sym.type_get ()], [sym.value], [destroy], [])[;
-  yystack_.push (ss);
-]],
-[[    yystack_.push (stack_symbol_type (]b4_join([s],
-                         [sym.value], b4_locations_if([sym.location]))[));]])[
+    stack_symbol_type t (s, sym);
+    yypush_ (m, t);

   }
 
   void
   }
 
   void


@@ -593,14 +624,7 @@ m4_if(b4_prefix, [yy], [],
   {
     if (m)
       YY_SYMBOL_PRINT (m, s);
   {
     if (m)
       YY_SYMBOL_PRINT (m, s);

-]b4_variant_if(
-[[
-  stack_symbol_type ss (]b4_join([s.state],
-      [s.value], b4_locations_if([s.location]))[);
-  ]b4_symbol_variant([s.type_get ()], [s.value], [destroy], [])[;
-  yystack_.push (ss);
-]],
-[    yystack_.push (s);])[
+    yystack_.push (s);

   }
 
   void
   }
 
   void


@@ -821,21 +845,6 @@ b4_dollar_popdef])[]dnl
         YYERROR;
       }
     YY_SYMBOL_PRINT ("-> $$ =", yylhs);
         YYERROR;
       }
     YY_SYMBOL_PRINT ("-> $$ =", yylhs);

-]b4_variant_if([[
-    // Destroy the rhs symbols.
-    for (int i = 0; i < yylen; ++i)
-      // Destroy a variant whose value may have been swapped with
-      // yylhs.value (for instance if the action was "std::swap($$,
-      // $1)").  The value of yylhs.value (hence possibly one of these
-      // rhs symbols) depends on the default construction for this
-      // type.  In the case of pointers for instance, no
-      // initialization is done, so the value is junk.  Therefore do
-      // not try to report the value of symbols about to be destroyed
-      // in the debug trace, it's possibly junk.  Hence yymsg = 0.
-      // Besides, that keeps exactly the same traces as with the other
-      // Bison skeletons.
-      yy_destroy_ (YY_NULL, yystack_[i]);]])[
-

     yypop_ (yylen);
     yylen = 0;
     YY_STACK_PRINT ();
     yypop_ (yylen);
     yylen = 0;
     YY_STACK_PRINT ();


@@ -890,7 +899,8 @@ b4_dollar_popdef])[]dnl
       goto yyerrorlab;]b4_locations_if([[
     yyerror_range[1].location = yystack_[yylen - 1].location;]])b4_variant_if([[
     /* $$ was initialized before running the user action.  */
       goto yyerrorlab;]b4_locations_if([[
     yyerror_range[1].location = yystack_[yylen - 1].location;]])b4_variant_if([[
     /* $$ was initialized before running the user action.  */

-    yy_destroy_ ("Error: discarding", yylhs);]])[
+    YY_SYMBOL_PRINT ("Error: discarding", yylhs);
+    yylhs.~stack_symbol_type();]])[

     /* Do not reclaim the symbols of the rule whose action triggered
        this YYERROR.  */
     yypop_ (yylen);
     /* Do not reclaim the symbols of the rule whose action triggered
        this YYERROR.  */
     yypop_ (yylen);

diff --git a/data/stack.hh b/data/stack.hh
index 7ed55606bb0c4219de50180193187bdce7f552c2..037c212fc6a549462f8a480d5982495ce1eed8bd 100644 (file)
--- a/data/stack.hh
+++ b/data/stack.hh
@@ -53,11 +53,15 @@ m4_define([b4_stack_define],
       return seq_[seq_.size () - 1 - i];
     }
 
       return seq_[seq_.size () - 1 - i];
     }
 

+    /// Steal the contents of \a t.
+    ///
+    /// Close to move-semantics.

     inline
     void
     inline
     void

-    push (const T& t)
+    push (T& t)

     {
     {

-      seq_.push_back (t);
+      seq_.push_back (T());
+      operator[](0).move (t);

     }
 
     inline
     }
 
     inline

diff --git a/data/variant.hh b/data/variant.hh
index c96cfeb6b5797a6117176da27154524dd70b5ff4..bbae8cb9bebe2f0e2b9e3f3cba4e65b7ff42eeee 100644 (file)
--- a/data/variant.hh
+++ b/data/variant.hh
@@ -100,13 +100,29 @@ m4_define([b4_variant_define],
       , tname (YY_NULL)])[
     {}
 
       , tname (YY_NULL)])[
     {}
 

-    /// Instantiate a \a T in here.
+    /// Construct and fill.
+    template <typename T>
+    variant (const T& t)]b4_parse_assert_if([
+      : built (true)
+      , tname (typeid (T).name ())])[
+    {
+      YYASSERT (sizeof (T) <= S);
+      new (buffer.raw) T (t);
+    }
+
+    /// Destruction, allowed only if empty.
+    ~variant ()
+    {]b4_parse_assert_if([
+      YYASSERT (!built);
+    ])[}
+
+    /// Instantiate an empty \a T in here.

     template <typename T>
     T&
     build ()
     {]b4_parse_assert_if([
     template <typename T>
     T&
     build ()
     {]b4_parse_assert_if([

-      //YYASSERT (!built);
-      //YYASSERT (!tname);
+      YYASSERT (!built);
+      YYASSERT (!tname);

       YYASSERT (sizeof (T) <= S);
       built = true;
       tname = typeid (T).name ();])[
       YYASSERT (sizeof (T) <= S);
       built = true;
       tname = typeid (T).name ();])[


@@ -118,24 +134,14 @@ m4_define([b4_variant_define],
     T&
     build (const T& t)
     {]b4_parse_assert_if([
     T&
     build (const T& t)
     {]b4_parse_assert_if([

-      //YYASSERT (!built);
-      //YYASSERT (!tname);
+      YYASSERT (!built);
+      YYASSERT (!tname);

       YYASSERT (sizeof (T) <= S);
       built = true;
       tname = typeid (T).name ();])[
       return *new (buffer.raw) T (t);
     }
 
       YYASSERT (sizeof (T) <= S);
       built = true;
       tname = typeid (T).name ();])[
       return *new (buffer.raw) T (t);
     }
 

-    /// Construct and fill.
-    template <typename T>
-    variant (const T& t)]b4_parse_assert_if([
-      : built (true)
-      , tname (typeid (T).name ())])[
-    {
-      YYASSERT (sizeof (T) <= S);
-      new (buffer.raw) T (t);
-    }
-

     /// Accessor to a built \a T.
     template <typename T>
     T&
     /// Accessor to a built \a T.
     template <typename T>
     T&


@@ -314,14 +320,15 @@ m4_define([b4_symbol_constructor_define_],
   b4_parser_class_name::make_[]b4_symbol_([$1], [id]) (dnl
 b4_join(b4_symbol_if([$1], [has_type],
                      [const b4_symbol([$1], [type])& v]),
   b4_parser_class_name::make_[]b4_symbol_([$1], [id]) (dnl
 b4_join(b4_symbol_if([$1], [has_type],
                      [const b4_symbol([$1], [type])& v]),

-        b4_locations_if([const location_type& l])))
+        b4_locations_if([const location_type& l])))[

   {
   {

-    return symbol_type (b4_join([token::b4_symbol([$1], [id])],
-                                b4_symbol_if([$1], [has_type], [v]),
-                                b4_locations_if([l])));
+    symbol_type res (token::]b4_symbol([$1], [id])[]b4_locations_if([, l])[);
+    ]b4_symbol_if([$1], [has_type], [res.value.build (v);])[
+    //    ]b4_locations_if([res.location = l;])[
+    return res;

   }
 
   }
 

-])])])
+]])])])

 
 
 # b4_symbol_constructor_define
 
 
 # b4_symbol_constructor_define

diff --git a/tests/c++.at b/tests/c++.at
index e5cd069caf08826e92ae65fb499c3c0b771cfef6..9b52900f0e4fce86dffd4bcc28c10eae165d4d7f 100644 (file)
--- a/tests/c++.at
+++ b/tests/c++.at
@@ -18,6 +18,91 @@
 AT_BANNER([[C++ Features.]])
 
 
 AT_BANNER([[C++ Features.]])
 
 

+## --------------------------- ##
+## C++ Variant-based Symbols.  ##
+## --------------------------- ##
+
+AT_SETUP([C++ Variant-based Symbols])
+
+AT_KEYWORDS([variant])
+
+AT_BISON_OPTION_PUSHDEFS([%skeleton "lalr1.cc" %debug $1])
+# Store strings and integers in a list of strings.
+AT_DATA_GRAMMAR([list.yy],
+[[%skeleton "lalr1.cc"
+%define api.value.type variant
+%define parse.assert
+%debug
+
+%code top
+{
+  // Get access to stack_symbol_type for the tests.
+# define private public
+}
+%code provides
+{
+  ]AT_YYLEX_DECLARE[
+}
+
+%token <int> INT "int"
+%type < std::list<int> > exp
+
+%printer { yyo << $$; } <int>
+%printer
+  {
+    for (std::list<int>::const_iterator i = $$.begin (); i != $$.end (); ++i)
+      {
+        if (i != $$.begin ())
+          yyo << ", ";
+        yyo << *i;
+      }
+  } < std::list<int> >
+
+%code requires { #include <list> }
+%code { int yylex (yy::parser::semantic_type* yylval); }
+
+%%
+exp: "int" { $$.push_back ($1); }
+%%
+]AT_YYERROR_DEFINE[
+]AT_YYLEX_DEFINE[
+
+int main()
+{
+  {
+    yy::parser::symbol_type s = yy::parser::make_INT(12);
+    std::cerr << s.value.as<int>() << std::endl;
+  }
+
+  {
+    yy::parser::symbol_type s = yy::parser::make_INT(123);
+    yy::parser::stack_symbol_type ss(1, s);
+    std::cerr << ss.value.as<int>() << std::endl;
+  }
+
+  {
+    yy::parser::stack_type st;
+    for (int i = 0; i < 100; ++i)
+      {
+        yy::parser::symbol_type s(yy::parser::make_INT(i));
+        yy::parser::stack_symbol_type ss(1, s);
+        st.push(ss);
+      }
+  }
+}
+]])
+
+AT_BISON_CHECK([-o list.cc list.yy])
+AT_COMPILE_CXX([list], [$NO_STRICT_ALIAS_CXXFLAGS list.cc])
+AT_PARSER_CHECK([./list], 0, [],
+[12
+123
+])
+
+AT_BISON_OPTION_POPDEFS
+AT_CLEANUP
+
+

 ## ---------- ##
 ## Variants.  ##
 ## ---------- ##
 ## ---------- ##
 ## Variants.  ##
 ## ---------- ##