On Sun, Jan 4, 2015 at 5:28 AM, Akim Demaille <a...@lrde.epita.fr> wrote:
>
>> Le 29 sept. 2014 à 03:43, Stephen Cameron <stephenmcame...@gmail.com> a
>> écrit :
>>
>> Hi,
>>
>> I think I have found a bug in bison v. 3.0.2.
>>
>> scameron@spacenerd ~/github/simple-expression-parser $ yacc --version
>> bison (GNU Bison) 3.0.2
>> Written by Robert Corbett and Richard Stallman.
>>
>> Copyright (C) 2013 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions. There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> scameron@spacenerd ~/github/simple-expression-parser $
>>
>> Note that is the version of bison shipped with e.g. Linux Mint 17.
>>
>> Below is some output (y.tab.h) of yacc (bison) v 3.0.2
>>
>> Notice the '#line' directive which does not start at the beginning of a
>> line:
>>
>> "union #line 49 "expression-parser.y" /* yacc.c:1909 */"
>>
>> gcc will not compile this. I think that means it's a bug in bison.
>> Earlier versions of bison produce correct output (e.g. v. 2.5 works
>> correctly, and the '#line' directive begins on a new line.)
>>
>> -------8<--------8<-------8<-------8<--------8<-------8<-------8<--------8<-------8<
>> [...]
>> /* Value type. */
>> #if ! defined YYSTYPE && ! defined YYSTYPE_IS_DECLARED
>> typedef union #line 49 "expression-parser.y" /* yacc.c:1909 */
>> valtype
>> YYSTYPE;
>> union #line 49 "expression-parser.y" /* yacc.c:1909 */
>> valtype
>>
>> {
>> #line 49 "expression-parser.y" /* yacc.c:1909 */
>>
>> struct parser_value_type {
>> double dval;
>> long long ival;
>> int has_dval;
>> int has_error;
>> } v;
>>
>> #line 77 "y.tab.h" /* yacc.c:1909 */
>> };
>> # define YYSTYPE_IS_TRIVIAL 1
>> [...]
>> -------8<--------8<-------8<-------8<--------8<-------8<-------8<--------8<-------8<
>>
>> The program which caused this is a simple arithmetic expression parser
>> (basic yacc 101 stuff). It is here:
>> https://github.com/smcameron/simple-expression-parser
>
> Hi!
>
> I'm sorry about this issue. I will apply the following patch,
> which should address your issue. I'm curious though: why exactly
> do you provide a tag name to the %union? What is wrong with just
> using YYSTYPE?
Heh. It's been long enough now (3 months) that I don't quite remember.
Prior to this little program, the last time I used bison was maybe 10
years before that, so I am by no means a bison expert. There may not
be a good reason, it may be just the way I tried to do it based on
some examples I found in an O'Reilly book or somewhere else.
The real code that we ran into this with (rather than the simplified
example I sent you -- not that the two are all that different) -- is
here: http://git.kernel.dk/?p=fio.git;a=tree;f=exp;hb=HEAD
Your question still stands though, and I don't really have a good
answer. I vaguely suspect that there may be a reason, but I don't
recall what it might have been, and it might only be my brain playing
tricks on me.
-- steve
>
> commit 9e0f2feae1ca02c97c56ca03dd974afdf87748d6
> Author: Akim Demaille <a...@lrde.epita.fr>
> Date: Sun Jan 4 14:16:23 2015 +0100
>
> %union: fix the support for named %union
>
> Bison supports a union tag, for obscure reasons. But it does a poor
> job at it, especially since Bison 3.0.
> Reported by Stephen Cameron and Tobias Frost.
>
> It did not ensure that the name was not given several times. An easy
> way to do this is to make the %union tag be handled as a %define
> variable, as they cannot be defined several times.
>
> Since Bison 3.0, the synclines were wrongly placed, resulting in
> invalid code. Addressing this issue, because of the way the union tag
> was stored (as a code muscle), would have been tedious. Unless we
> rather define the %union tag as a %percent variable, whose synclines
> are easier to manipulate.
>
> So replace the b4_union_name muscle by the api.value.union.name
> %define variable, document, and check.
>
> * data/bison.m4: Make sure that api.value.union.name has a keyword value.
> * data/c++.m4: Make sure that api.value.union.name is not defined.
> * data/c.m4 (b4_union_name): No longer use it, use api.value.union.name.
> * doc/bison.texi (%define Summary): Document it.
> * src/parse-gram.y (union_name): No longer define b4_uion_name, but
> api.value.union.name.
> * tests/input.at (Redefined %union name): New.
> * tests/synclines.at (%union name syncline): New.
> * tests/types.at: Check named %unions.
>
> diff --git a/THANKS b/THANKS
> index 4559a90..943cbb1 100644
> --- a/THANKS
> +++ b/THANKS
> @@ -126,6 +126,7 @@ Sebastien Fricker sebastien.fric...@gmail.com
> Sergei Steshenko sergst...@yahoo.com
> Shura debil_u...@ngs.ru
> Stefano Lattarini stefano.lattar...@gmail.com
> +Stephen Cameron stephenmcame...@gmail.com
> Steve Murphy m...@parsetree.com
> Sum Wu s...@geekhouse.org
> Théophile Ranquet theophile.ranq...@gmail.com
> @@ -133,6 +134,7 @@ Thiru Ramakrishnan thiru.ramakrish...@gmail.com
> Tim Josling t...@melbpc.org.au
> Tim Landscheidt t...@tim-landscheidt.de
> Tim Van Holder tim.van.hol...@pandora.be
> +Tobias Frost t...@debian.org
> Tom Lane t...@sss.pgh.pa.us
> Tom Tromey tro...@cygnus.com
> Tommy Nordgren tommy.nordg...@chello.se
> diff --git a/data/bison.m4 b/data/bison.m4
> index c443032..967aaaa 100644
> --- a/data/bison.m4
> +++ b/data/bison.m4
> @@ -1061,3 +1061,6 @@ b4_percent_define_ifdef([api.value.type],
> [['%s' and '%s' cannot be used together]],
> [%yacc],
> [%define api.value.type "union"])])])])
> +
> +# api.value.union.name.
> +b4_percent_define_check_kind([api.value.union.name], [keyword])
> diff --git a/data/c++.m4 b/data/c++.m4
> index 4927899..e4559be 100644
> --- a/data/c++.m4
> +++ b/data/c++.m4
> @@ -17,6 +17,11 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> +# Sanity checks, before defaults installed by c.m4.
> +b4_percent_define_ifdef([[api.value.union.name]],
> + [b4_complain_at(b4_percent_define_get_loc([[api.value.union.name]]),
> + [named %union is invalid in C++])])
> +
> m4_include(b4_pkgdatadir/[c.m4])
>
> # b4_comment(TEXT, [PREFIX])
> diff --git a/data/c.m4 b/data/c.m4
> index 1f1a23f..16713de 100644
> --- a/data/c.m4
> +++ b/data/c.m4
> @@ -97,7 +97,8 @@ m4_define([b4_api_PREFIX],
> m4_define_default([b4_prefix], [b4_api_prefix])
>
> # If the %union is not named, its name is YYSTYPE.
> -m4_define_default([b4_union_name], [b4_api_PREFIX[]STYPE])
> +b4_percent_define_default([[api.value.union.name]],
> + [b4_api_PREFIX[][STYPE]])
>
>
> ## ------------------------ ##
> @@ -608,10 +609,9 @@ m4_copy_force([b4_symbol_value_union], [b4_symbol_value])
> ])
>
>
> -# ---------------- #
> -# api.value.type. #
> -# ---------------- #
> -
> +# -------------------------- #
> +# api.value.type = variant. #
> +# -------------------------- #
>
> # b4_value_type_setup_variant
> # ---------------------------
> @@ -686,11 +686,12 @@ typedef ]b4_percent_define_get([[api.value.type]])[
> ]b4_api_PREFIX[STYPE;
> [m4_bmatch(b4_percent_define_get([[api.value.type]]),
> [union\|union-directive],
> [[#if ! defined ]b4_api_PREFIX[STYPE && ! defined
> ]b4_api_PREFIX[STYPE_IS_DECLARED
> -typedef union ]b4_union_name[ ]b4_api_PREFIX[STYPE;
> -union ]b4_union_name[
> +]b4_percent_define_get_syncline([[api.value.union.name]])[
> +union ]b4_percent_define_get([[api.value.union.name]])[
> {
> ]b4_user_union_members[
> };
> +typedef union ]b4_percent_define_get([[api.value.union.name]])[
> ]b4_api_PREFIX[STYPE;
> # define ]b4_api_PREFIX[STYPE_IS_TRIVIAL 1
> # define ]b4_api_PREFIX[STYPE_IS_DECLARED 1
> #endif
> diff --git a/doc/bison.texi b/doc/bison.texi
> index 665664b..f33fc15 100644
> --- a/doc/bison.texi
> +++ b/doc/bison.texi
> @@ -3851,7 +3851,7 @@ example:
> @noindent
> specifies the union tag @code{value}, so the corresponding C type is
> @code{union value}. If you do not specify a tag, it defaults to
> -@code{YYSTYPE}.
> +@code{YYSTYPE} (@pxref{%define Summary,,api.value.union.name}).
>
> As another extension to POSIX, you may specify multiple @code{%union}
> declarations; their contents are concatenated. However, only the first
> @@ -6014,12 +6014,12 @@ Use this @var{type} as semantic value.
> @item Default Value:
> @itemize @minus
> @item
> -@code{%union} if @code{%union} is used, otherwise @dots{}
> +@code{union-directive} if @code{%union} is used, otherwise @dots{}
> @item
> @code{int} if type tags are used (i.e., @samp{%token <@var{type}>@dots{}} or
> -@samp{%token <@var{type}>@dots{}} is used), otherwise @dots{}
> +@samp{%type <@var{type}>@dots{}} is used), otherwise @dots{}
> @item
> -@code{""}
> +undefined.
> @end itemize
>
> @item History:
> @@ -6030,6 +6030,30 @@ introduced in Bison 3.0. Was introduced for Java only
> in 2.3b as
> @c api.value.type
>
>
> +@c ================================================== api.value.union.name
> +@deffn Directive {%define api.value.union.name} @var{name}
> +@itemize @bullet
> +@item Language(s):
> +C
> +
> +@item Purpose:
> +The tag of the generated @code{union} (@emph{not} the name of the
> +@code{typedef}). This variable is set to @code{@var{id}} when @samp{%union
> +@var{id}} is used. There is no clear reason to give this union a name.
> +
> +@item Accepted Values:
> +Any valid identifier.
> +
> +@item Default Value:
> +@code{YYSTYPE}.
> +
> +@item History:
> +Introduced in Bison 3.0.3.
> +@end itemize
> +@end deffn
> +@c api.value.type
> +
> +
> @c ================================================== location_type
> @deffn Directive {%define location_type}
> Obsoleted by @code{api.location.type} since Bison 2.7.
> diff --git a/src/parse-gram.y b/src/parse-gram.y
> index 741d743..6a68071 100644
> --- a/src/parse-gram.y
> +++ b/src/parse-gram.y
> @@ -421,7 +421,9 @@ code_props_type:
>
> union_name:
> %empty {}
> -| ID { muscle_code_grow ("union_name", $1, @1); }
> +| ID { muscle_percent_define_insert ("api.value.union.name",
> + @1, muscle_keyword, $1,
> +
> MUSCLE_PERCENT_DEFINE_GRAMMAR_FILE); }
> ;
>
> grammar_declaration:
> diff --git a/tests/input.at b/tests/input.at
> index cec82dd..33da53d 100644
> --- a/tests/input.at
> +++ b/tests/input.at
> @@ -1904,6 +1904,51 @@ m4_popdef([AT_TEST])
> AT_CLEANUP
>
>
> +## ----------------------- ##
> +## Redefined %union name. ##
> +## ----------------------- ##
> +
> +AT_SETUP([[Redefined %union name]])
> +
> +# AT_TEST(DIRECTIVES, ERROR)
> +# --------------------------
> +m4_pushdef([AT_TEST],
> +[AT_DATA([[input.y]],
> +[$1
> +%%
> +exp: %empty;
> +])
> +
> +AT_BISON_CHECK([[input.y]], [[1]], [[]],
> +[$2])
> +])
> +
> +AT_TEST([[%union foo {};
> +%union {};
> +%union foo {};
> +%define api.value.union.name foo]],
> +[[input.y:3.8-10: error: %define variable 'api.value.union.name' redefined
> +input.y:1.8-10: previous definition
> +input.y:4.9-28: error: %define variable 'api.value.union.name' redefined
> +input.y:3.8-10: previous definition
> +]])
> +
> +AT_TEST([[%define api.value.union.name {foo}]],
> +[[input.y:1.9-28: error: %define variable 'api.value.union.name' requires
> keyword values
> +input.y:1.9-28: error: %define variable 'api.value.union.name' is not used
> +]])
> +
> +AT_TEST([[%define api.value.union.name "foo"]],
> +[[input.y:1.9-28: error: %define variable 'api.value.union.name' requires
> keyword values
> +input.y:1.9-28: error: %define variable 'api.value.union.name' is not used
> +]])
> +
> +m4_popdef([AT_TEST])
> +AT_CLEANUP
> +
> +
> +
> +
> ## -------------- ##
> ## Stray $ or @. ##
> ## -------------- ##
> diff --git a/tests/synclines.at b/tests/synclines.at
> index 72d5d85..77499b7 100644
> --- a/tests/synclines.at
> +++ b/tests/synclines.at
> @@ -182,6 +182,35 @@ exp: '0';
> ])
>
>
> +## ---------------------- ##
> +## %union name syncline. ##
> +## ---------------------- ##
> +
> +# Check that invalid union names are properly reported in the
> +# source file.
> +AT_SETUP([%union name syncline])
> +AT_BISON_OPTION_PUSHDEFS
> +AT_DATA([[input.y]],
> +[[%union break
> +{
> + char dummy;
> +}
> +%{
> +]AT_YYERROR_DECLARE_EXTERN[
> +]AT_YYLEX_DECLARE_EXTERN[
> +%}
> +%%
> +exp: '0';
> +%%
> +]])
> +
> +AT_BISON_CHECK([-o input.c input.y])
> +AT_SYNCLINES_COMPILE([input.c])
> +AT_CHECK([[grep '^input.y:1' stdout]], 0, [ignore])
> +AT_BISON_OPTION_POPDEFS
> +AT_CLEANUP
> +
> +
> ## ----------------------- ##
> ## Postprologue syncline. ##
> ## ----------------------- ##
> diff --git a/tests/types.at b/tests/types.at
> index 29f51e2..8ddf694 100644
> --- a/tests/types.at
> +++ b/tests/types.at
> @@ -197,18 +197,23 @@ m4_foreach([b4_skel], [[yacc.c], [glr.c], [lalr1.cc],
> [glr.cc]],
> AT_VAL.fval = .2f],
> [10 0.2])
>
> - # A %union.
> - AT_TEST([%skeleton "]b4_skel["
> - %union { float fval; int ival; };],
> - [%token <ival> '1';
> - %token <fval> '2';],
> - ['1' '2' { printf ("%d %2.1f\n", $1, $2); }],
> - ["12"],
> - [if (res == '1')
> - AT_VAL.ival = 10;
> - else
> - AT_VAL.fval = 0.2f],
> - [10 0.2])
> + # A %union and a named %union. In C++ named %union is an error.
> + m4_foreach([b4_union],
> + [m4_bmatch(b4_skel, [\.cc$],
> + [[%union]],
> + [[%union], [%union foo],
> + [%define api.value.union.name foo; %union]])],
> + [AT_TEST([%skeleton "]b4_skel["
> + ]b4_union[ { float fval; int ival; };],
> + [%token <ival> '1';
> + %token <fval> '2';],
> + ['1' '2' { printf ("%d %2.1f\n", $1, $2); }],
> + ["12"],
> + [if (res == '1')
> + AT_VAL.ival = 10;
> + else
> + AT_VAL.fval = 0.2f],
> + [10 0.2])])
>
> # A Bison-defined union.
> # The tokens names are not available directly in C++, we use their
> @@ -249,3 +254,22 @@ m4_foreach([b4_skel], [[yacc.c], [glr.c], [lalr1.cc],
> [glr.cc]],
>
> m4_popdef([AT_TEST])
> m4_popdef([_AT_TEST])
> +
> +
> +## ------------------- ##
> +## C++: Named %union. ##
> +## ------------------- ##
> +
> +m4_foreach([b4_skel], [[lalr1.cc], [glr.cc]],
> +[AT_SETUP([b4_skel: Named %union])
> +AT_DATA([input.y],
> +[%skeleton "]b4_skel["
> +%union foo { float fval; int ival; };
> +%%
> +exp: %empty;
> +])
> +AT_BISON_CHECK([input.y], 1, [],
> +[[input.y:2.8-10: error: named %union is invalid in C++
> +]])
> +AT_CLEANUP
> +])
>
