Re: [PATCH 1/2] glr2.cc: fork glr.cc to a c++ version

Hi Valentin,

> Le 26 août 2020 à 16:57, Valentin Tolmer <[email protected]> a écrit :
> 
> This is a fork of glr.cc to be c++-first instead of a wrapper around
> glr.c.

Great news!

I'm very happy this can go forward.  It will deserve its own release,
say 3.8.

I have installed your commit with a few changes.  In particular, beware
that you are probably using obsolete submodules.  Be sure to rerun
bootstrap when, say, gnulib's reference was changed.  Also, please
write more traditional commit messages (you don't need to go into much
details).

I have installed your changes, but doing so, I'm breaking my rules:
the CI is red, and make check fails when you have --enable-gcc-warnings
set.  Please, fix this asap: I feel very uncomfortable when master is
not all green.  But we must go forward.

One of the issues you should address very early is that you still pass
yyparser as if it were a user param in many places, although you already
have it as a member of your objects.  So I get conflicts such as:

> 231. conflicts.at:1096: testing Syntax error in consistent error state: glr2.cc ...
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: COLUMNS=1000; export COLUMNS; NO_TERM_HYPERLINKS=1; export NO_TERM_HYPERLINKS;  bison --color=no -fno-caret -o input.cc input.y
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: COLUMNS=1000; export COLUMNS; NO_TERM_HYPERLINKS=1; export NO_TERM_HYPERLINKS; VALGRIND_OPTS="$VALGRIND_OPTS --leak-check=summary --show-reachable=no"; export VALGRIND_OPTS;  bison --color=no -fno-caret -o input.cc input.y -Werror
> stderr:
> input.y:17.5-25: error: rule useless in parser due to conflicts [-Werror=other]
> input.y:18.5-29: error: rule useless in parser due to conflicts [-Werror=other]
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: sed 's,.*/$,,' stderr 1>&2
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: COLUMNS=1000; export COLUMNS; NO_TERM_HYPERLINKS=1; export NO_TERM_HYPERLINKS; VALGRIND_OPTS="$VALGRIND_OPTS --leak-check=summary --show-reachable=no"; export VALGRIND_OPTS;  bison --color=no -fno-caret -o input.cc input.y --warnings=error
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: COLUMNS=1000; export COLUMNS; NO_TERM_HYPERLINKS=1; export NO_TERM_HYPERLINKS; VALGRIND_OPTS="$VALGRIND_OPTS --leak-check=summary --show-reachable=no"; export VALGRIND_OPTS;  bison --color=no -fno-caret -o input.cc input.y -Wnone,none -Werror --trace=none
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: COLUMNS=1000; export COLUMNS; NO_TERM_HYPERLINKS=1; export NO_TERM_HYPERLINKS; VALGRIND_OPTS="$VALGRIND_OPTS --leak-check=summary --show-reachable=no"; export VALGRIND_OPTS;  bison --color=no -fno-caret -o input.cc input.y --warnings=none -Werror --trace=none
> /Users/akim/src/gnu/bison/tests/conflicts.at:1096: $CXX $CXXFLAGS $CPPFLAGS  $LDFLAGS -o input input.cc $LIBS
> stderr:
> input.cc: In member function 'void yyGLRStack::yyFail(const char*, yy::parser&)':
> input.cc:2076:3: error: declaration of 'yyparser' shadows a member of 'yyGLRStack' [-Werror=shadow]
>  2076 |   {
>       |   ^
> input.cc:2793:17: note: shadowed declaration is here
>  2793 |     yy::parser& yyparser;
>       |                 ^~~~~~~~
> input.cc: In member function 'void yyGLRStack::yyreportSyntaxError(yy::parser&)':
> input.cc:2114:3: error: declaration of 'yyparser' shadows a member of 'yyGLRStack' [-Werror=shadow]
>  2114 |   {
>       |   ^
> input.cc:2793:17: note: shadowed declaration is here
>  2793 |     yy::parser& yyparser;
>       |                 ^~~~~~~~

etc. etc. etc.

Passing yyparser as a user param was needed to trick glr.c, we must
get rid of that trick.

The attached patch gives a hint of what is needed.



You can have a look at https://travis-ci.org/github/akimd/bison/builds/722440141
to get more similar logs.

Cheers!

diff --git a/data/skeletons/glr2.cc b/data/skeletons/glr2.cc
index dbdc84e5..7bd50bc7 100644
--- a/data/skeletons/glr2.cc
+++ b/data/skeletons/glr2.cc
@@ -62,17 +62,6 @@ m4_define([b4_parser_class],
 # Save the parse parameters.
 m4_define([b4_parse_param_orig], m4_defn([b4_parse_param]))
 
-# b4_parse_param_wrap
-# -------------------
-# New ones.
-m4_ifset([b4_parse_param],
-[m4_define([b4_parse_param_wrap],
-           [[b4_namespace_ref::b4_parser_class[& yyparser], [[yyparser]]],]
-m4_defn([b4_parse_param]))],
-[m4_define([b4_parse_param_wrap],
-           [[b4_namespace_ref::b4_parser_class[& yyparser], [[yyparser]]]])
-])
-
 # b4_yy_symbol_print_define
 # -------------------------
 # Bypass the default implementation to generate the "yy_symbol_print"
@@ -381,10 +370,6 @@ b4_copyright([Skeleton interface for Bison GLR parsers in C++],
 ]b4_cpp_guard_close([b4_spec_mapped_header_file])[
 ]b4_output_end])
 
-# Let glr.c (and b4_shared_declarations) believe that the user
-# arguments include the parser itself.
-m4_pushdef([b4_parse_param], m4_defn([b4_parse_param_wrap]))
-
 ## ---------------- ##
 ## Default values.  ##
 ## ---------------- ##