Re: Destructor miscompilation with C++ variants?

[Akim Demaille]

Hi Michael,

> Le 28 sept. 2014 à 19:54, Michael Catanzaro <address@hidden> a écrit :
> 
> Hi,
> 
> With Bison 3.0.2, the following trivial grammar using the lalr1.cc
> skeleton and variants for the semantic types compiles fine only if the
> destructor is commented out. If the destructor is present, I get the
> following error when running g++:
> 
> In file included from test.tab.cc:46:0:
> test.yy: In destructor
> ‘yy::parser::basic_symbol<Base>::~basic_symbol()’:
> test.yy:8:18: error: ‘yysym’ was not declared in this scope
> %destructor { delete $$; } nonterminal
> 
> Here is the grammar:
> 
> // test.yy
> //
> // Compile: bison test.yy
> //          g++ test.tab.cc
> 
> %skeleton "lalr1.cc"
> %define api.token.constructor
> %define api.value.type variant
> %defines
> 
> %token TERMINAL
> %type <int*> nonterminal
> %destructor { delete $$; } nonterminal
> 
> %code
> {
>    yy::parser::symbol_type yylex()
>    {
>        return yy::parser::make_TERMINAL();
>    }
> }
> 
> %%
> nonterminal: TERMINAL { $$ = new int; }
> 
> %%
> void yy::parser::error(const std::string&) {}
> int main() {}
> // End of file

Thanks for the report!  I'm planning to install (in the "maint"
branch) the following patch to address the issue, if you have
some time to give it a shot.

Cheers!

        Akim

commit ee028dceff390a23da391900b5b7fe651cfe320c
Author: Akim Demaille <address@hidden>
Date:   Fri Jan 9 12:01:22 2015 +0100

    c++: fix the use of destructors when variants are enabled
    
    When using variants, destructors generate invalid code.
    <http://lists.gnu.org/archive/html/bug-bison/2014-09/msg00005.html>
    Reported by Michael Catanzaro.
    
    * data/c++.m4 (~basic_symbol): b4_symbol_foreach works on yysym:
    define it.
    * tests/c++.at (Variants): Check it.

diff --git a/NEWS b/NEWS
index 490e89e..1877194 100644
--- a/NEWS
+++ b/NEWS
@@ -4,19 +4,23 @@ GNU Bison NEWS
 
 ** Bug fixes
 
-*** Named %union support
+*** C++ with Variants (lalr1.cc)
+
+  Problems with %destructor and '%define parse.assert' have been fixed.
+
+*** Named %union support (yacc.c, glr.c)
 
   Bison 3.0 introduced a regression on named %union such as
 
     %union foo { int ival; };
 
-  The possibility to use a name was introduced ``for Yacc compatibility''.
+  The possibility to use a name was introduced "for Yacc compatibility".
   It is however not required by POSIX Yacc, and its usefulness is not clear.
 
-*** %define api.value.type union with %defines
+*** %define api.value.type union with %defines (yacc.c, glr.c)
 
-  The yacc.c and glr.c parsers were broken when %defines was used
-  together with "%define api.value.type union".
+  The C parsers were broken when %defines was used together with "%define
+  api.value.type union".
 
 *** Redeclarations are reported in proper order
 
diff --git a/THANKS b/THANKS
index 98ec269..d5feb16 100644
--- a/THANKS
+++ b/THANKS
@@ -83,6 +83,7 @@ Martin Mokrejs            address@hidden
 Martin Nylin              address@hidden
 Matt Kraai                address@hidden
 Matt Rosing               address@hidden
+Michael Catanzaro         address@hidden
 Michael Felt              address@hidden
 Michael Hayes             address@hidden
 Michael Raskin            address@hidden
diff --git a/data/c++.m4 b/data/c++.m4
index 8494d21..4321c30 100644
--- a/data/c++.m4
+++ b/data/c++.m4
@@ -346,6 +346,7 @@ m4_define([b4_public_types_define],
   {]b4_variant_if([[
     // User destructor.
     symbol_number_type yytype = this->type_get ();
+    basic_symbol<Base>& yysym = *this;
     switch (yytype)
     {
 ]b4_symbol_foreach([b4_symbol_destructor])dnl
diff --git a/tests/c++.at b/tests/c++.at
index e6d6983..737037d 100644
--- a/tests/c++.at
+++ b/tests/c++.at
@@ -252,10 +252,12 @@ typedef std::list<std::string> strings_type;
 // Using the template type to exercize its parsing.
 // Starting with :: to ensure we don't output "<::" which starts by the
 // digraph for the left square bracket.
-%type <::std::list<std::string>> list result;
+%type <::std::list<std::string>> list;
 
 %printer { yyo << $$; }
   <int> <::std::string> <::std::list<std::string>>;
+%destructor { std::cerr << "Destroy: " << $$ << '\n'; } <*>;
+%destructor { std::cerr << "Destroy: \"" << $$ << "\"\n"; } <::std::string>;
 %%
 
 result:
@@ -336,8 +338,31 @@ namespace yy
 
 AT_FULL_COMPILE([list])
 AT_PARSER_CHECK([./list], 0,
-[(0, 1, 2, 4, 6)
-])
+[[(0, 1, 2, 4, 6)
+]],
+[[Destroy: ""
+Destroy: "0"
+Destroy: (0)
+Destroy: 1
+Destroy: "1"
+Destroy: ()
+Destroy: ""
+Destroy: "2"
+Destroy: ()
+Destroy: ""
+Destroy: 3
+Destroy: ()
+Destroy: ""
+Destroy: "4"
+Destroy: ()
+Destroy: ()
+Destroy: 5
+Destroy: ()
+Destroy: ""
+Destroy: "6"
+Destroy: ()
+Destroy: (0, 1, 2, 4, 6)
+]])
 
 AT_BISON_OPTION_POPDEFS
 AT_CLEANUP
@@ -826,10 +851,10 @@ list:
 
 item:
   'a'     { $$ = $][1; }
-| 'e'     { YYUSE ($$); YYUSE($][1); error ("syntax error"); }
+| 'e'     { YYUSE ($$); YYUSE ($][1); error ("syntax error"); }
 // Not just 'E', otherwise we reduce when 'E' is the lookahead, and
 // then the stack is emptied, defeating the point of the test.
-| 'E' 'a' { YYUSE($][1); $$ = $][2; }
+| 'E' 'a' { YYUSE ($][1); $$ = $][2; }
 | 'R'     { ]AT_VARIANT_IF([], [$$ = YY_NULLPTR; delete $][1]; )[YYERROR; }
 | 'p'     { $$ = $][1; }
 | 's'     { $$ = $][1; throw std::runtime_error ("reduction"); }