[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] m4: generate a basic_symbol constructor for each symbol
From: |
Akim Demaille |
Subject: |
Re: [PATCH 2/2] m4: generate a basic_symbol constructor for each symbol type |
Date: |
Mon, 28 Jan 2013 20:08:00 +0100 |
Le 28 janv. 2013 à 20:29, Theophile Ranquet <address@hidden> a écrit :
> Recently, there was a slightly vicious bug hidden in the make_ functions:
>
> parser::symbol_type
> parser::make_TEXT (const ::std::string& v)
> {
> return symbol_type (token::TOK_TEXT, v);
> }
>
> The constructor for symbol_type doesn't take an ::std::string& as
> argument, but a constant variant. However, because there is a variant
> constructor which takes an ::std::string&, this caused the implicit
> construction of a built variant. Considering that the variant argument
> for the symbol_type constructor was cv-qualified, this temporary variant
> was never destroyed.
>
> As a temporay solution, the symbol was built in two stages:
Temporary.
>
> symbol_type res (token::TOK_TEXT);
> res.value.build< ::std::string&> (v);
> return res;
>
> However, the solution introduced in this patch contributes to letting
> the symbols handle themselves, by supplying them with constructors which
I'd go for "that" here.
> take a non-variant value and build the symbol's own variant with that
> value.
>
> * data/variant.hh (b4_symbol_constructor_define_): Use the new
> constructors rather than building in a temporary symbol.
> (b4_basic_symbol_constructor_declare,
> b4_basic_symbol_constructor_define): New, macros generating the
Remove the comma.
> constructors.
> * data/c++.m4 (basic_symbol): Invoke the macros here.
> ---
> data/c++.m4 | 12 +++++++++---
> data/variant.hh | 38 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/data/c++.m4 b/data/c++.m4
> index d134eb8..d9f86cd 100644
> --- a/data/c++.m4
> +++ b/data/c++.m4
> @@ -174,10 +174,12 @@ m4_define([b4_public_types_declare],
>
> /// Copy constructor.
> basic_symbol (const basic_symbol& other);
> -
> +]b4_variant_if([[
> + /// Constructor for valueless symbols, and symbols from each type.
> +]b4_type_foreach([b4_basic_symbol_constructor_declare])], [[
> /// Constructor for valueless symbols.
> basic_symbol (typename Base::value_type t]b4_locations_if([,
> - const location_type& l])[);
> + const location_type& l])[);]])[
>
> /// Constructor for symbols with semantic value.
> basic_symbol (typename Base::value_type t,
> @@ -279,6 +281,10 @@ m4_define([b4_public_types_define],
> (void) v;
> ]b4_symbol_variant([this->type_get ()], [value], [copy], [v])])[}
>
> +]b4_variant_if([[
> + // Implementation of basic_symbol constructor for each type.
> +]b4_type_foreach([b4_basic_symbol_constructor_define])], [[
> + /// Constructor for valueless symbols.
> template <typename Base>
> inline
> ]b4_parser_class_name[::basic_symbol<Base>::basic_symbol (]b4_join(
> @@ -287,7 +293,7 @@ m4_define([b4_public_types_define],
> : Base (t)
> , value ()]b4_locations_if([
> , location (l)])[
> - {}
> + {}]])[
>
> template <typename Base>
> inline
> diff --git a/data/variant.hh b/data/variant.hh
> index 84ea779..0c9eec5 100644
> --- a/data/variant.hh
> +++ b/data/variant.hh
> @@ -311,15 +311,43 @@ 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])))
> {
> - symbol_type res (token::]b4_symbol([$1], [id])[]b4_locations_if([, l])[);
> - ]b4_symbol_if([$1], [has_type], [res.value.build (v);
> - ])[return res;
> + return symbol_type (b4_join([token::b4_symbol([$1], [id])],
> + b4_symbol_if([$1], [has_type], [v]),
> + b4_locations_if([l])));
> +
> }
>
> -]])])])
> +])])])
> +
>
> +# b4_basic_symbol_constructor_declare
> +# ----------------------------------
> +# Generate a constructor declaration for basic_symbol from given type.
> +m4_define([b4_basic_symbol_constructor_declare],
> +[[
> + basic_symbol (]b4_join(
> + [typename Base::value_type t],
> + b4_symbol_if([$1], [has_type], const b4_symbol([$1], [type])[ v]),
> + b4_locations_if([const location_type& l]))[);
> +]])
So you really handle this at the basic_symbol level. Why not,
Indeed. Fine, please install!
> +# b4_basic_symbol_constructor_define
> +# ----------------------------------
> +# Generate a constructor implementation for basic_symbol from given type.
> +m4_define([b4_basic_symbol_constructor_define],
> +[[
> + template <typename Base>
> + ]b4_parser_class_name[::basic_symbol<Base>::basic_symbol (]b4_join(
> + [typename Base::value_type t],
> + b4_symbol_if([$1], [has_type], const b4_symbol([$1], [type])[ v]),
> + b4_locations_if([const location_type& l]))[)
> + : Base (t)
> + , value (]b4_symbol_if([$1], [has_type], [v])[)]b4_locations_if([
> + , location (l)])[
> + {}
> +]])
>
> # b4_symbol_constructor_define
> # ----------------------------
> --
> 1.8.1.1
>
>