Re: RFC: c++: provide control over the stack.hh file name

[Akim Demaille]

Hi Paul,

I’d like your opinion, see below.

> Le 30 sept. 2018 à 19:14, Akim Demaille <address@hidden> a écrit :
> 
> 
> 
>> Le 30 sept. 2018 à 17:47, Paul Eggert <address@hidden> a écrit :
>> 
>> Akim Demaille wrote:
>>>   So offer the user a means to (i) decide what the name of the output
>>>   file should be, and (ii) not generate this file at all (its content
>>>   will be inline where the parser is defined).
>> 
>> Another idea would be to generate an empty stack.hh file for now (actually, 
>> a file saying only "/* This file is present only to cater to obsolescent 
>> build procedures that expect a stack.hh file.  */", and do this even if the 
>> user does not specify any option at all. This will help warn users that 
>> eventually the file will go away, something that should also be put into the 
>> documentation of course.
> 
> I agree with this!
> See https://lists.gnu.org/archive/html/bison-patches/2018-09/msg00151.html.
> 
> I would also be happy to get your opinion about using %require
> as a means to change the default between versions.  And for
> instance %require 3.2 could imply don’t generate stack.hh at
> all.

Paul, what do you think about the previous paragraph?


Here’s a proposal to make stack.hh completely useless.

commit 34be76ffe25d8e0718a0c1aed4ffc29538b958b0
Author: Akim Demaille <address@hidden>
Date:   Tue Oct 2 07:01:13 2018 +0200

    c++: make stack.hh completely useless
    
    Let's completely deprecate stack.hh.  Don't provide a means to give it
    a new name, allow only its removal.
    
    See https://lists.gnu.org/archive/html/bison-patches/2018-09/msg00151.html
    and https://lists.gnu.org/archive/html/bison-patches/2018-09/msg00182.html.
    
    * data/stack.hh: Reduce stack.hh to a simple comment explaining how to
    get rid of it.
    * data/lalr1.cc: Adjust
    (_b4_percent_define_check_file_warn): Rename as...
    (_b4_percent_define_check_file_complain): this: make it an error.
    * tests/input.at (%define file variables): Adjust.
    * tests/output.at: Remove cases where stack.hh was removed.

diff --git a/data/lalr1.cc b/data/lalr1.cc
index e28f3667..822f1d7f 100644
--- a/data/lalr1.cc
+++ b/data/lalr1.cc
@@ -21,14 +21,14 @@ m4_include(b4_pkgdatadir/[c++.m4])
 m4_define([b4_value_type_setup_variant])
 
 
-# _b4_percent_define_check_file_warn(VARIABLE)
-# --------------------------------------------
+# _b4_percent_define_check_file_complain(VARIABLE)
+# ------------------------------------------------
 # Warn about %define variable VARIABLE having an incorrect
 # value.
-m4_define([_b4_percent_define_check_file_warn],
-[b4_warn_at(b4_percent_define_get_loc([$1]),
-            [[%%define variable '%s' requires 'none' or '"..."' values]],
-            [$1])])
+m4_define([_b4_percent_define_check_file_complain],
+[b4_complain_at(b4_percent_define_get_loc([$1]),
+                [[%%define variable '%s' requires 'none' or '"..."' values]],
+                [$1])])
 
 
 # _b4_percent_define_check_file(MACRO, VARIABLE, DEFAULT)
@@ -45,8 +45,8 @@ m4_define([_b4_percent_define_check_file],
          [m4_define([$1], b4_percent_define_get([$2]))],
     [keyword],
          [m4_if(b4_percent_define_get([$2]), [none], [],
-                [_b4_percent_define_check_file_warn([$2])])],
-    [_b4_percent_define_check_file_warn([$2])])
+                [_b4_percent_define_check_file_complain([$2])])],
+    [_b4_percent_define_check_file_complain([$2])])
    ],
    [m4_ifval([$3],
              [errprint(DEFINE)m4_define([$1], [$3])])])
@@ -194,8 +194,6 @@ m4_define([b4_shared_declarations],
 # include <vector>
 
 ]b4_cxx_portability[
-]m4_ifdef([b4_stack_file],
-          [[# include "]b4_stack_file["]])[
 ]m4_ifdef([b4_position_file],
           [[# include "]b4_position_file["]])[
 ]m4_ifdef([b4_location_file],
@@ -209,7 +207,7 @@ m4_define([b4_shared_declarations],
 
 ]b4_namespace_open[
 
-]m4_ifndef([b4_stack_file], [b4_stack_define])[
+]b4_stack_define[
 ]b4_bison_locations_if([m4_ifndef([b4_location_file],
                                   [m4_ifndef([b4_position_file], 
[b4_position_define
 
diff --git a/data/stack.hh b/data/stack.hh
index 0e6fca31..348ef527 100644
--- a/data/stack.hh
+++ b/data/stack.hh
@@ -15,15 +15,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-m4_pushdef([b4_copyright_years],
-           [2002-2015, 2018])
 
 # b4_stack_file
 # -------------
 # Name of the file containing the stack class, if we want this file.
-_b4_percent_define_check_file([b4_stack_file],
-                              [[api.stack.file]],
-                              b4_defines_if([[stack.hh]]))
+b4_percent_define_ifdef([[api.stack.file]],
+[b4_percent_define_check_values([[[[api.stack.file]],
+                                  [[none]]]])],
+[m4_define([b4_stack_file], [stack.hh])])
 
 
 # b4_stack_define
@@ -140,25 +139,13 @@ m4_define([b4_stack_define],
 
 m4_ifdef([b4_stack_file],
 [b4_output_begin([b4_dir_prefix[]b4_stack_file])[
-]b4_copyright([Stack handling for Bison parsers in C++])[
-
-/**
- ** \file ]b4_dir_prefix[]b4_stack_file[
- ** Define the ]b4_namespace_ref[::stack class.
+/* Starting with Bison 3.2, this file is useless: the structure it
+ * used to define is now defined with the parser itself.
+ *
+ * To get rid of this file:
+ * 1. add '%define api.stack.file none' to your grammar file
+ * 2. add 'require "3.2"' to your grammar file
+ * 3. remove references to this file from your build system.
  */
-
-]b4_cpp_guard_open([b4_dir_prefix[]b4_stack_file])[
-
-# include <vector>
-
-]b4_cxx_portability[
-
-]b4_namespace_open[
-]b4_stack_define[
-]b4_namespace_close[
-
-]b4_cpp_guard_close([b4_dir_prefix[]b4_stack_file])[
 ]b4_output_end[
 ]])
-
-m4_popdef([b4_copyright_years])
diff --git a/tests/input.at b/tests/input.at
index 7b8a6426..e196bd7b 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -1641,16 +1641,17 @@ AT_DATA([[input.y]],
 %%
 start: %empty;
 ]])
-AT_BISON_CHECK([[-fcaret input.y]], [[0]], [[]],
-[[input.y:5.9-25: warning: %define variable 'api.position.file' requires 
'none' or '"..."' values [-Wother]
+AT_BISON_CHECK([[-fcaret input.y]], [[1]], [[]],
+[[input.y:5.9-25: error: %define variable 'api.position.file' requires 'none' 
or '"..."' values
  %define api.position.file bogus
          ^^^^^^^^^^^^^^^^^
-input.y:4.9-25: warning: %define variable 'api.location.file' requires 'none' 
or '"..."' values [-Wother]
+input.y:4.9-25: error: %define variable 'api.location.file' requires 'none' or 
'"..."' values
  %define api.location.file {bogus}
          ^^^^^^^^^^^^^^^^^
-input.y:3.9-22: warning: %define variable 'api.stack.file' requires 'none' or 
'"..."' values [-Wother]
+input.y:3.9-22: error: invalid value for %define variable 'api.stack.file': 
'bogus'
  %define api.stack.file bogus
          ^^^^^^^^^^^^^^
+input.y:3.9-22:     accepted value: 'none'
 ]])
 
 AT_CLEANUP
diff --git a/tests/output.at b/tests/output.at
index 2fb638fe..39e0c706 100644
--- a/tests/output.at
+++ b/tests/output.at
@@ -180,11 +180,6 @@ AT_CHECK_OUTPUT([gram_dir/foo.yy],
                 [output_dir/foo.output output_dir/foo.tab.cc 
output_dir/foo.tab.hh output_dir/location.hh output_dir/position.hh 
output_dir/stack.hh])
 
 # api.stack.file.
-AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %define api.stack.file "foo.sta.hh"],
-                [],
-                [foo.sta.hh foo.tab.cc])
-
 AT_CHECK_OUTPUT([foo.yy],
                 [%skeleton "lalr1.cc" %defines %define api.stack.file none],
                 [],
@@ -216,9 +211,9 @@ AT_CHECK_OUTPUT([foo.yy],
                 [foo.loc.hh foo.pos.hh foo.tab.cc foo.tab.hh])
 
 AT_CHECK_OUTPUT([gram_dir/foo.yy],
-                [%skeleton "lalr1.cc" %verbose %defines %define api.stack.file 
"foo.sta.hh" %file-prefix "output_dir/foo"],
+                [%skeleton "lalr1.cc" %verbose %defines %file-prefix 
"output_dir/foo"],
                 [],
-                [output_dir/foo.output output_dir/foo.sta.hh 
output_dir/foo.tab.cc output_dir/foo.tab.hh])
+                [output_dir/foo.output output_dir/foo.tab.cc 
output_dir/foo.tab.hh])
 
 
 # AT_CHECK_CONFLICTING_OUTPUT(INPUT-FILE, DIRECTIVES, FLAGS, STDERR,