From: Akim Demaille Date: Mon, 28 Jan 2013 13:29:43 +0000 (+0100) Subject: c++: revamp the support for variants X-Git-Tag: v2.7.90~152 X-Git-Url: https://git.saurik.com/bison.git/commitdiff_plain/97ae878ec376149ba49bd08c1bd6590dd5224671?hp=5f87211cb40944a1f81631542404db93f869aa8f 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 f9fcb9d2..591e3e8f 100644 --- a/data/c++.m4 +++ b/data/c++.m4 @@ -166,6 +166,9 @@ m4_define([b4_public_types_declare], template struct basic_symbol : Base { + /// Alias to Base. + typedef Base super_type; + /// 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])[); + ~basic_symbol (); /// 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); + /// Steal the type of \a that. + void move (by_type& that); + /// The symbol type. + /// + /// -1 when this symbol is empty. int type; /// The type (corresponding to \a type). + /// + /// -1 when this symbol is empty. 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) - , value ()]b4_locations_if([ + , value (]b4_variant_if([], [v])[)]b4_locations_if([ , 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; - ]b4_variant_if([b4_symbol_variant([this->type_get ()], [value], [copy], - [v])], - [value = v;])[ - } + ]b4_symbol_variant([this->type_get ()], [value], [copy], [v])])[} template ]b4_parser_class_name[::basic_symbol::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)])[ {} + template + inline + ]b4_parser_class_name[::basic_symbol::~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 void ]b4_parser_class_name[::basic_symbol::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])], - [value = s.value;])[ + [value = s.value;])[]b4_locations_if([ + location = s.location;])[ } // 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) @@ -318,6 +342,14 @@ m4_define([b4_public_types_define], : 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 { diff --git a/data/lalr1.cc b/data/lalr1.cc index a195602c..5a05db76 100644 --- a/data/lalr1.cc +++ b/data/lalr1.cc @@ -281,10 +281,18 @@ b4_location_define])])[ /// 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). + /// + /// -1 when empty. 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. - typedef basic_symbol stack_symbol_type; + struct stack_symbol_type : basic_symbol + { + /// Superclass. + typedef basic_symbol 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_type; @@ -512,7 +529,7 @@ m4_if(b4_prefix, [yy], [], // 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) @@ -526,7 +543,33 @@ m4_if(b4_prefix, [yy], [], 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& yysym) const { if (yymsg) - YY_SYMBOL_PRINT (yymsg, yysym); + YY_SYMBOL_PRINT (yymsg, yysym);]b4_variant_if([], [ // 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_variant_if([ - - // Type destructor. - b4_symbol_variant([[yytype]], [[yysym.value]], [[template destroy]])])[ + }])[ } #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) { - 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 @@ -593,14 +624,7 @@ m4_if(b4_prefix, [yy], [], { 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 @@ -821,21 +845,6 @@ b4_dollar_popdef])[]dnl 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 (); @@ -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. */ - 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); diff --git a/data/stack.hh b/data/stack.hh index 7ed55606..037c212f 100644 --- a/data/stack.hh +++ b/data/stack.hh @@ -53,11 +53,15 @@ m4_define([b4_stack_define], return seq_[seq_.size () - 1 - i]; } + /// Steal the contents of \a t. + /// + /// Close to move-semantics. inline void - push (const T& t) + push (T& t) { - seq_.push_back (t); + seq_.push_back (T()); + operator[](0).move (t); } inline diff --git a/data/variant.hh b/data/variant.hh index c96cfeb6..bbae8cb9 100644 --- a/data/variant.hh +++ b/data/variant.hh @@ -100,13 +100,29 @@ m4_define([b4_variant_define], , tname (YY_NULL)])[ {} - /// Instantiate a \a T in here. + /// Construct and fill. + template + 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 T& build () {]b4_parse_assert_if([ - //YYASSERT (!built); - //YYASSERT (!tname); + YYASSERT (!built); + YYASSERT (!tname); 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([ - //YYASSERT (!built); - //YYASSERT (!tname); + YYASSERT (!built); + YYASSERT (!tname); YYASSERT (sizeof (T) <= S); built = true; tname = typeid (T).name ();])[ return *new (buffer.raw) T (t); } - /// Construct and fill. - template - 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 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_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 diff --git a/tests/c++.at b/tests/c++.at index e5cd069c..9b52900f 100644 --- a/tests/c++.at +++ b/tests/c++.at @@ -18,6 +18,91 @@ 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" +%type < std::list > exp + +%printer { yyo << $$; } +%printer + { + for (std::list::const_iterator i = $$.begin (); i != $$.end (); ++i) + { + if (i != $$.begin ()) + yyo << ", "; + yyo << *i; + } + } < std::list > + +%code requires { #include } +%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() << 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() << 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. ## ## ---------- ##