bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Akim Demaille
Subject: Re: RFC: c++: provide control over the stack.hh file name
Date: Mon, 15 Oct 2018 20:10:21 +0200

Hi Paul,

Thanks for the answer!

> Le 15 oct. 2018 à 04:34, Paul Eggert <address@hidden> a écrit :
> 
> Akim Demaille wrote:
>>>> 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!
>>> Seehttps://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?
> 
> Sure, that sounds good. Old uses of Bison will still work, since they won’t 
> use '%require 3.2'.

Excellent.  I think it does simplify plenty of things
(implementation, documentation, tests, grammar files).

Here is my proposal.

commit ccd088293f068fcc385896238fccecc91ad3e52f
Author: Akim Demaille <address@hidden>
Date:   Mon Oct 15 14:04:00 2018 +0200

    C++: let %require "3.2" disable the generation of obsolete files
    
    The files stack.hh and position.hh are deprecated.  Rather than
    devoting specify %define variables to discard them (api.position.file
    and api.stack.file), and rather than having to use special rules when
    api.location.file is used, let's simply decide that from %require
    "3.2" onwards, these files will not be generated.
    
    The only noticeable thing here is that, in order to be able to check
    the behavior of %require "3.2", to have this version (which is still
    3.1-*) to accept %require "3.2".
    
    * src/gram.h, src/gram.c (required_version): New.
    * src/parse-gram.y (version_check): Set it.
    * src/output.c (prepare): Pass it m4.
    * data/bison.m4 (b4_required_version_if): Receive it and use it.
    * data/location.cc, data/stack.hh: Replace the api.*.file with only
    required version comparison.
    * tests/input.at: No longer check api.stack.file and api.position.file.
    * NEWS, doc/bison.texi: Don't mention them.
    Document the %require 3.2 behavior.
    * tests/output.at: Use %require 3.2 instead.

diff --git a/NEWS b/NEWS
index d8a21e53..42c9b201 100644
--- a/NEWS
+++ b/NEWS
@@ -135,21 +135,12 @@ GNU Bison NEWS
   made useless: its content is included in the parser definition.  It is
   still generated for backward compatibility.
 
-  To stop generating it, use the following directive:
-
-    %define api.stack.file none
-
   When in addition to %defines, location support is requested (%locations),
   the file position.hh is also generated.  It is now also useless: its
   content is now included in location.hh.
 
-  To stop generating it, use the following directive:
-
-    %define api.position.file none
-
-  Any definition of api.location.file eliminates both position.hh and
-  stack.hh (i.e., implies '%define api.position.file none' and '%define
-  api.stack.file none').
+  These files are no longer generated when your grammar file requires at
+  least Bison 3.2 (%require "3.2").
 
 ** Bug fixes
 
diff --git a/data/bison.m4 b/data/bison.m4
index 6dde067d..cb777d07 100644
--- a/data/bison.m4
+++ b/data/bison.m4
@@ -61,6 +61,16 @@ This special exception was added by the Free Software 
Foundation in
 version 2.2 of Bison.])])
 
 
+# b4_required_version_if(VERSION, IF_NEWER, IF_OLDER)
+# ---------------------------------------------------
+# If the version %require'd by the user is VERSION (or newer) expand
+# IF_NEWER, otherwise IF_OLDER.  VERSION should be an integer, e.g.,
+# 302 for 3.2.
+m4_define([b4_required_version_if],
+[m4_if(m4_eval($1 <= b4_required_version),
+              [1], [$2], [$3])])
+
+
 ## -------- ##
 ## Output.  ##
 ## -------- ##
diff --git a/data/location.cc b/data/location.cc
index 1ff61b7d..3f0f97fb 100644
--- a/data/location.cc
+++ b/data/location.cc
@@ -22,12 +22,8 @@ m4_pushdef([b4_copyright_years],
 # b4_position_file
 # ----------------
 # Name of the file containing the position class, if we want this file.
-b4_percent_define_ifdef([[api.position.file]],
-[b4_percent_define_check_values([[[[api.position.file]],
-                                  [[none]]]])],
-[b4_defines_if([b4_percent_define_ifdef([[api.location.file]],
-                                        [],
-                                        [m4_define([b4_position_file], 
[position.hh])])])])
+b4_defines_if([b4_required_version_if([302], [],
+                                      [m4_define([b4_position_file], 
[position.hh])])])])
 
 
 # b4_location_file
@@ -330,12 +326,9 @@ m4_ifdef([b4_position_file], [[
 // used to define is now defined in "]b4_location_file[".
 //
 // To get rid of this file:
-// 1. add '%define api.position.file none'
-//     or '%define api.location.file none'
-//     or '%define api.location.file "my-loc.hh"' to your grammar file
-// 2. add 'require "3.2"' to your grammar file
-// 3. remove references to this file from your build system
-// 4. if you used to include it, include "]b4_location_file[" instead.
+// 1. add 'require "3.2"' (or newer) to your grammar file
+// 2. remove references to this file from your build system
+// 3. if you used to include it, include "]b4_location_file[" instead.
 
 #include ]b4_location_include[
 ]b4_output_end[
diff --git a/data/stack.hh b/data/stack.hh
index efe85c0d..1036eea1 100644
--- a/data/stack.hh
+++ b/data/stack.hh
@@ -19,12 +19,8 @@
 # b4_stack_file
 # -------------
 # Name of the file containing the stack class, if we want this file.
-b4_percent_define_ifdef([[api.stack.file]],
-[b4_percent_define_check_values([[[[api.stack.file]],
-                                  [[none]]]])],
-[b4_defines_if([b4_percent_define_ifdef([[api.location.file]],
-                                        [],
-                                        [m4_define([b4_stack_file], 
[stack.hh])])])])
+b4_defines_if([b4_required_version_if([302], [],
+                                      [m4_define([b4_stack_file], 
[stack.hh])])])
 
 
 # b4_stack_define
@@ -138,17 +134,13 @@ m4_define([b4_stack_define],
   };
 ]])
 
-
 m4_ifdef([b4_stack_file],
 [b4_output_begin([b4_dir_prefix], [b4_stack_file])[
 // 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'
-//     or '%define api.location.file none'
-//     or '%define api.location.file "my-loc.hh"' to your grammar file
-// 2. add 'require "3.2"' to your grammar file
-// 3. remove references to this file from your build system.
+// 1. add 'require "3.2"' (or newer) to your grammar file
+// 2. remove references to this file from your build system.
 ]b4_output_end[
 ]])
diff --git a/doc/bison.texi b/doc/bison.texi
index 3fddfe50..8f11640a 100644
--- a/doc/bison.texi
+++ b/doc/bison.texi
@@ -4788,6 +4788,14 @@ status 63).
 %require "@var{version}"
 @end example
 
+Some deprecated behaviors are disabled for some required @var{version}:
address@hidden @code
address@hidden "3.2"
+The C++ deprecated files @file{position.hh} and @file{stack.hh} are no
+longer generated.
address@hidden table
+
+
 @node Token Decl
 @subsection Token Type Names
 @cindex declaring token type names
@@ -5834,9 +5842,6 @@ in @var{file}.  This file name can be relative (to where 
the parser file is
 output) or absolute.
 @end table
 
-Defining it implies @samp{%define api.position.file none} and @samp{%define
-api.stack.file none}.
-
 @item Default Value:
 Not applicable if locations are not enabled, or if a user location type is
 specified (see @code{api.location.type}).  Otherwise, Bison's
@@ -5893,28 +5898,6 @@ Introduced in Bison 2.7 for C, C++ and Java.  Introduced 
under the name
 @end itemize
 @end deffn
 
address@hidden ================================================== 
api.position.file
address@hidden {Directive} {%define api.position.file} @code{none}
-
address@hidden @bullet
address@hidden Language(s): C++
-
address@hidden Purpose:
-Disable the generation of the legacy file @file{position.hh}.  The class
address@hidden is now defined where the class @code{location} is.
-
address@hidden Accepted Values: @code{none}
-
address@hidden Default Value:
address@hidden if @code{api.location.file} is defined, otherwise a useless
address@hidden is generated for backward compatibility.
-
address@hidden History:
-Introduced in Bison 3.2.
address@hidden itemize
address@hidden deffn
-
-
 @c ================================================== api.prefix
 @deffn {Directive} {%define api.prefix} @address@hidden@}
 
@@ -6001,27 +5984,6 @@ the @code{full} value was introduced in Bison 2.7
 
 
 
address@hidden ================================================== api.stack.file
address@hidden {Directive} {%define api.stack.file} @code{none}
-
address@hidden @bullet
address@hidden Language(s): C++
-
address@hidden Purpose:
-Disable the generation of the legacy file @file{stack.hh}.
-
address@hidden Accepted Values: @code{none}
-
address@hidden Default Value:
address@hidden if @code{api.location.file} is defined, otherwise a useless
address@hidden is generated for backward compatibility.
-
address@hidden History:
-Introduced in Bison 3.2.
address@hidden itemize
address@hidden deffn
-
-
 @c ================================================== api.token.constructor
 @deffn Directive {%define api.token.constructor}
 
@@ -10681,12 +10643,9 @@ extension of these two files follow the same rules as 
with regular C parsers
 (@pxref{Invocation}).
 
 @item position.hh
-A useless legacy file.  To get rid of it, use @samp{%define api.stack.file
-none}, or define @code{api.location.file}.
-
address@hidden stack.hh
-A useless legacy file.  To get rid of it, use @samp{%define
-api.position.file none}, or define @code{api.location.file}.
address@hidden stack.hh
+Useless legacy files.  To get rid of then, use @samp{%require "3.2"} or
+newer.
 @end table
 
 All these files are documented using Doxygen; run @command{doxygen} for a
diff --git a/src/gram.c b/src/gram.c
index 4b4171f4..fd07719e 100644
--- a/src/gram.c
+++ b/src/gram.c
@@ -46,6 +46,8 @@ symbol_number *token_translations = NULL;
 
 int max_user_token_number = 256;
 
+int required_version = 0;
+
 bool
 rule_useful_in_grammar_p (rule const *r)
 {
diff --git a/src/gram.h b/src/gram.h
index bbae54b4..4ecf4bf2 100644
--- a/src/gram.h
+++ b/src/gram.h
@@ -271,4 +271,8 @@ void grammar_rules_useless_report (const char *message);
 /* Free the packed grammar. */
 void grammar_free (void);
 
+/* The version %required by the grammar file, as an int (100 * major +
+   minor).  0 if unspecified.  */
+extern int required_version;
+
 #endif /* !GRAM_H_ */
diff --git a/src/output.c b/src/output.c
index ae5f2d6d..96295d17 100644
--- a/src/output.c
+++ b/src/output.c
@@ -625,6 +625,8 @@ prepare (void)
   char const *cp = getenv ("BISON_USE_PUSH_FOR_PULL");
   bool use_push_for_pull_flag = cp && *cp && strtol (cp, 0, 10);
 
+  MUSCLE_INSERT_INT ("required_version", required_version);
+
   /* Flags. */
   MUSCLE_INSERT_BOOL ("defines_flag", defines_flag);
   MUSCLE_INSERT_BOOL ("glr_flag", glr_parser);
diff --git a/src/parse-gram.y b/src/parse-gram.y
index 3fb6b199..1962835e 100644
--- a/src/parse-gram.y
+++ b/src/parse-gram.y
@@ -32,6 +32,7 @@
 %code
 {
   #include "system.h"
+  #include <errno.h>
 
   #include "c-ctype.h"
   #include "complain.h"
@@ -822,10 +823,37 @@ add_param (param_type type, char *decl, location loc)
 static void
 version_check (location const *loc, char const *version)
 {
-  if (strverscmp (version, PACKAGE_VERSION) > 0)
-    {
-      complain (loc, complaint, "require bison %s, but have %s",
-                version, PACKAGE_VERSION);
+  /* Changes of behavior are only on minor version changes, so "3.0.5"
+     is the same as "3.0". */
+  errno = 0;
+  char* cp = NULL;
+  unsigned long major = strtoul (version, &cp, 10);
+  if (errno || *cp != '.')
+    {
+      complain (loc, complaint, _("invalid version requirement: %s"),
+                version);
+      return;
+    }
+  ++cp;
+  unsigned long minor = strtoul (cp, NULL, 10);
+  if (errno)
+    {
+      complain (loc, complaint, _("invalid version requirement: %s"),
+                version);
+      return;
+    }
+  required_version = major * 100 + minor;
+  /* Pretend to be at least 3.2, even if we are only 3.1-211, as it
+     allows us to check features published in 3.2 while developping
+     3.2.  */
+  const char* api_version = "3.2";
+  const char* package_version =
+    strverscmp (api_version, PACKAGE_VERSION) > 0
+    ? api_version : PACKAGE_VERSION;
+  if (strverscmp (version, package_version) > 0)
+    {
+      complain (loc, complaint, _("require bison %s, but have %s"),
+                version, package_version);
       exit (EX_MISMATCH);
     }
 }
diff --git a/tests/input.at b/tests/input.at
index bbe5e27a..11bbe37e 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -1660,24 +1660,14 @@ AT_SETUP([["%define" file variables]])
 AT_DATA([[input.y]],
 [[%skeleton "lalr1.cc"
 %locations
-%define api.stack.file bogus
 %define api.location.file {bogus}
-%define api.position.file bogus
 %%
 start: %empty;
 ]])
 AT_BISON_CHECK([[-fcaret input.y]], [[1]], [[]],
-[[input.y:5.9-25: error: invalid value for %define variable 
'api.position.file': 'bogus'
- %define api.position.file bogus
-         ^^^^^^^^^^^^^^^^^
-input.y:5.9-25:     accepted value: 'none'
-input.y:4.9-25: error: %define variable 'api.location.file' requires 'none' or 
'"..."' values
+[[input.y:3.9-25: error: %define variable 'api.location.file' requires 'none' 
or '"..."' values
  %define api.location.file {bogus}
          ^^^^^^^^^^^^^^^^^
-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 2c2cd9b8..e1137002 100644
--- a/tests/output.at
+++ b/tests/output.at
@@ -183,37 +183,20 @@ 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.
+# %require "3.2" => no position.hh not stack.hh.
 AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %define api.stack.file none],
+                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file none %require "3.2"],
                 [],
                 [foo.tab.cc foo.tab.hh])
 
 AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %define api.stack.file none 
%locations],
-                [],
-                [foo.tab.cc foo.tab.hh location.hh position.hh])
-
-AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %define api.stack.file none 
%locations %define api.position.file none],
-                [],
-                [foo.tab.cc foo.tab.hh location.hh])
-
-# api.location.file=none => api.position.file=none and api.stack.file=none.
-AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file none],
-                [],
-                [foo.tab.cc foo.tab.hh])
-
-# api.location.file="*" => api.position.file=none and api.stack.file=none.
-AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file "foo.loc.hh"],
+                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file "foo.loc.hh" %require "3.2"],
                 [],
                 [foo.loc.hh foo.tab.cc foo.tab.hh])
 
 # Absolute paths.
 AT_CHECK_OUTPUT([foo.yy],
-                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file "$at_dir/foo.loc.hh"],
+                [%skeleton "lalr1.cc" %defines %locations %define 
api.location.file "$at_dir/foo.loc.hh" %require "3.2"],
                 [],
                 [foo.loc.hh foo.tab.cc foo.tab.hh])
 





reply via email to

[Prev in Thread] Current Thread [Next in Thread]