Hi Akim! Thanks for the prompt reply and patch! Somehow I missed %define filename_type in your documentation, otherwise I would have jumped on that. It does make sense to have const to be the default though, so the change would be great!
Speaking of Bison documentation, I was wondering if there is a way to generate a C++ push parser? It seems like that is not the case at the moment, although I don’t see why given that all the machinery already exists for C. Can you confirm this please? Thank you! Yuriy > On Jun 27, 2020, at 01:08, Akim Demaille <a...@lrde.epita.fr> wrote: > > Hi Yuriy, hi Martin, > > We accept visitors :) > >> Le 26 juin 2020 à 00:22, Yuriy Solodkyy <solo...@gmail.com> a écrit : >> >> Hi, >> >> I’m using bison 3.6.3 on MacOS via brew. I see that in the C++ generated >> parser location and position take pointer to std::string with filename via a >> pointer to modifiable std::string. I don’t see the filename itself being >> modified anywhere in the parser, so I was wondering why not accept a pointer >> to const std::string instead? Without this, I need to either copy filename >> first somewhere or const_cast when I can ensure the lifetime. The thing is >> that when I invoke my own function passing it a filename in const string&, >> which in turn calls parse (in pull mode) I can ensure that the filename will >> be valid during the entire duration of the call to parse yet I cannot use a >> pointer to it to initialize location because location demands the pointer to >> non-const string, which in my opinion is unnecessary. Can you please change >> your source for position and location take pointer to const std::string >> instead? > > Martin asked the same thing very recently: > > https://lists.gnu.org/r/help-bison/2020-05/msg00011.html > > > Ok, let's do it. I hope it won't break too many things (it's quite easy for > people who relied on the previous default to add `%define filename_type > "std::string"` is get a consistent behavior before/after this change). > > > WDYT about the following patch? > > Cheers! > > commit eeafc706e87dab7e84d0056c852e4579e6c3cf53 > Author: Akim Demaille <akim.demai...@gmail.com> > Date: Sat Jun 27 09:43:14 2020 +0200 > > c++: by default, use const std::string for file names > > Reported by Martin Blais and Yuriy Solodkyy. > https://lists.gnu.org/r/help-bison/2020-05/msg00011.html > https://lists.gnu.org/r/bug-bison/2020-06/msg00038.html > > While at it, modernize filename_type as api.filename.type and document > it properly. > > * data/skeletons/c++.m4 (filename_type): Rename as... > (api.filename.type): this. > Default to const std::string. > * data/skeletons/location.cc (position, location): Expose the > filename_type type. > Use api.filename.type. > * doc/bison.texi (%define Summary): Document api.filename.type. > (C++ Location Values): Document position::filename_type. > * src/muscle-tab.c (muscle_percent_variable_update): Ensure backward > compatibility. > * tests/c++.at: Check that using const file names is ok. > tests/input.at: Check backward compat. > > diff --git a/NEWS b/NEWS > index d9da8192..29425bca 100644 > --- a/NEWS > +++ b/NEWS > @@ -2,11 +2,38 @@ GNU Bison NEWS > > * Noteworthy changes in release ?.? (????-??-??) [?] > > +** New features > + > +*** File prefix mapping > + > + Bison learned a new argument, '--file-prefix-map OLD=NEW'. Any file path in > + the output (specifically #line directives and #ifdef header guards) that > + being with the prefix OLD will have it replace with the prefix NEW, similar > + to the -ffile-prefix-map in GCC. This option can be used to make bison > output > + reproducible. > + > ** Changes > > - When installed to be relocatable (via configure --enable-relocatable), > +*** Relocatable installation > + > + When installed to be relocatable (via `configure --enable-relocatable`), > bison will now also look for a relocated m4. > > +*** C++ file names > + > + The `filename_type` %define variable was renamed `api.filename.type`. > + Instead of > + > + %define filename_type "symbol" > + > + write > + > + %define api.filename.type {symbol} > + > + (Or let `bison --update` do it for you). > + > + It now defaults to `const std::string` instead of `std::string`. > + > ** Bug fixes > > *** Include the generated header (yacc.c) > @@ -48,16 +75,6 @@ GNU Bison NEWS > > An old, well hidden, bug in the generation of IELR parsers was fixed. > > -** New features > - > -*** File prefix mapping > - > - Bison learned a new argument, '--file-prefix-map OLD=NEW'. Any file path in > - the output (specifically #line directives and #ifdef header guards) that > - being with the prefix OLD will have it replace with the prefix NEW, similar > - to the -ffile-prefix-map in GCC. This option can be used to make bison > output > - reproducible. > - > * Noteworthy changes in release 3.6.4 (2020-06-15) [stable] > > ** Bug fixes > diff --git a/THANKS b/THANKS > index af24ceaa..e9bc2762 100644 > --- a/THANKS > +++ b/THANKS > @@ -112,6 +112,7 @@ Marc Autret autre...@epita.fr > Marc Mendiola mmend...@usc.edu > Marc Schönefeld marc.schoenef...@gmx.org > Mark Boyall wolfeinst...@gmail.com > +Martin Blais bl...@furius.ca > Martin Jacobs martin.jac...@arcor.de > Martin Mokrejs mmokr...@natur.cuni.cz > Martin Nylin martin.ny...@linuxmail.org > @@ -215,6 +216,7 @@ Wolfram Wagner w...@mpi-sb.mpg.de > Wwp subscr...@free.fr > xolodho xolo...@gmail.com > Yuichiro Kaneko spikete...@gmail.com > +Yuriy Solodkyy solo...@gmail.com > Zack Weinberg z...@codesourcery.com > 江 祖铭 jjzum...@outlook.com > 長田偉伸 cbh34...@iret.co.jp > diff --git a/data/skeletons/c++.m4 b/data/skeletons/c++.m4 > index 1d41c4c8..5536b1a0 100644 > --- a/data/skeletons/c++.m4 > +++ b/data/skeletons/c++.m4 > @@ -105,7 +105,7 @@ b4_percent_define_default([[api.parser.class]], > [[parser]]) > # > # b4_percent_define_default([[api.location.type]], [[location]]) > > -b4_percent_define_default([[filename_type]], [[std::string]]) > +b4_percent_define_default([[api.filename.type]], [[const std::string]]) > # Make it a warning for those who used betas of Bison 3.0. > b4_percent_define_default([[api.namespace]], m4_defn([b4_prefix])) > > diff --git a/data/skeletons/location.cc b/data/skeletons/location.cc > index dff984e7..33c9e50d 100644 > --- a/data/skeletons/location.cc > +++ b/data/skeletons/location.cc > @@ -63,11 +63,13 @@ m4_define([b4_location_define], > class position > { > public: > + /// Type for file name. > + typedef ]b4_percent_define_get([[api.filename.type]])[ filename_type; > /// Type for line and column numbers. > typedef int counter_type; > ]m4_ifdef([b4_location_constructors], [[ > /// Construct a position. > - explicit position (]b4_percent_define_get([[filename_type]])[* f = > YY_NULLPTR, > + explicit position (filename_type* f = YY_NULLPTR, > counter_type l = ]b4_location_initial_line[, > counter_type c = ]b4_location_initial_column[) > : filename (f) > @@ -77,7 +79,7 @@ m4_define([b4_location_define], > > ]])[ > /// Initialization. > - void initialize (]b4_percent_define_get([[filename_type]])[* fn = > YY_NULLPTR, > + void initialize (filename_type* fn = YY_NULLPTR, > counter_type l = ]b4_location_initial_line[, > counter_type c = ]b4_location_initial_column[) > { > @@ -106,7 +108,7 @@ m4_define([b4_location_define], > /** \} */ > > /// File name to which this position refers. > - ]b4_percent_define_get([[filename_type]])[* filename; > + filename_type* filename; > /// Current line number. > counter_type line; > /// Current column number. > @@ -184,6 +186,8 @@ m4_define([b4_location_define], > class location > { > public: > + /// Type for file name. > + typedef position::filename_type filename_type; > /// Type for line and column numbers. > typedef position::counter_type counter_type; > ]m4_ifdef([b4_location_constructors], [ > @@ -200,7 +204,7 @@ m4_define([b4_location_define], > {} > > /// Construct a 0-width location in \a f, \a l, \a c. > - explicit location (]b4_percent_define_get([[filename_type]])[* f, > + explicit location (filename_type* f, > counter_type l = ]b4_location_initial_line[, > counter_type c = ]b4_location_initial_column[) > : begin (f, l, c) > @@ -209,7 +213,7 @@ m4_define([b4_location_define], > > ])[ > /// Initialization. > - void initialize (]b4_percent_define_get([[filename_type]])[* f = > YY_NULLPTR, > + void initialize (filename_type* f = YY_NULLPTR, > counter_type l = ]b4_location_initial_line[, > counter_type c = ]b4_location_initial_column[) > { > diff --git a/doc/bison.texi b/doc/bison.texi > index 5a81202b..ac71fe6d 100644 > --- a/doc/bison.texi > +++ b/doc/bison.texi > @@ -5922,6 +5922,31 @@ Unaccepted @var{variable}s produce an error. Some of > the accepted > @var{variable}s are described below. > > > +@c ================================================== api.filename.file > +@anchor{api-filename-type} > +@deffn {Directive} {%define api.filename.type} @{@var{type}@} > + > +@itemize @bullet > +@item Language(s): C++ > + > +@item Purpose: > +Define the type of file names in Bison's default location and position > +types. @xref{Exposing the Location Classes}. > + > +@item Accepted Values: > +Any type that is printable (via streams) and comparable (with @code{==} and > +@code{!=}). > + > +@item Default Value: @code{const std::string}. > + > +@item History: > +Introduced in Bison 2.0 as @code{filename_type} (with @code{std::string} as > +default), renamed as @code{api.filename.type} in Bison 3.7 (with @code{const > +std::string} as default). > +@end itemize > +@end deffn > + > + > @c ================================================== api.header.include > @deffn Directive {%define api.header.include} @{"header.h"@} > @deffnx Directive {%define api.header.include} @{<header.h>@} > @@ -6052,7 +6077,8 @@ Introduced in Bison 3.2. > @item Default Value: none > > @item History: > -Introduced in Bison 2.7 for C++ and Java, in Bison 3.4 for C. > +Introduced in Bison 2.7 for C++ and Java, in Bison 3.4 for C. Was > +originally named @code{location_type} in Bison 2.5 and 2.6. > @end itemize > @end deffn > > @@ -6555,12 +6581,6 @@ Introduced in Bison 3.0.3. > @c api.value.type > > > -@c ================================================== location_type > -@deffn Directive {%define location_type} > -Obsoleted by @code{api.location.type} since Bison 2.7. > -@end deffn > - > - > @c ================================================== lr.default-reduction > > @deffn Directive {%define lr.default-reduction} @var{when} > @@ -11898,10 +11918,6 @@ is some time and/or some talented C++ hacker willing > to contribute to Bison. > > @node C++ Location Values > @subsection C++ Location Values > -@c - %locations > -@c - class position > -@c - class location > -@c - %define filename_type "const symbol::Symbol" > > When the directive @code{%locations} is used, the C++ parser supports > location tracking, see @ref{Tracking Locations}. > @@ -11923,25 +11939,28 @@ generated, and the user defined type will be used. > @node C++ position > @subsubsection C++ @code{position} > > +@defcv {Type} {position} {filename_type} > +The base type for file names. Defaults to @code{const std::string}. > +@xref{api-filename-type,,@code{api.filename.type}}, to change its definition. > +@end defcv > + > @defcv {Type} {position} {counter_type} > The type used to store line and column numbers. Defined as @code{int}. > @end defcv > > -@deftypeop {Constructor} {position} {} position (@code{std::string*} > @var{file} = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} > @var{col} = 1) > +@deftypeop {Constructor} {position} {} position (@code{filename_type*} > @var{file} = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} > @var{col} = 1) > Create a @code{position} denoting a given point. Note that @code{file} is > not reclaimed when the @code{position} is destroyed: memory managed must be > handled elsewhere. > @end deftypeop > > -@deftypemethod {position} {void} initialize (@code{std::string*} @var{file} > = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} @var{col} > = 1) > +@deftypemethod {position} {void} initialize (@code{filename_type*} > @var{file} = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} > @var{col} = 1) > Reset the position to the given values. > @end deftypemethod > > -@deftypeivar {position} {std::string*} file > +@deftypeivar {position} {filename_type*} file > The name of the file. It will always be handled as a pointer, the parser > -will never duplicate nor deallocate it. As an experimental feature you may > -change it to @samp{@var{type}*} using @samp{%define filename_type > -"@var{type}"}. > +will never duplicate nor deallocate it. > @end deftypeivar > > @deftypeivar {position} {counter_type} line > @@ -11988,11 +12007,11 @@ Create a @code{Location} from the endpoints of the > range. > @end deftypeop > > @deftypeop {Constructor} {location} {} location (@code{const position&} > @var{pos} = position()) > -@deftypeopx {Constructor} {location} {} location (@code{std::string*} > @var{file}, @code{counter_type} @var{line}, @code{counter_type} @var{col}) > +@deftypeopx {Constructor} {location} {} location (@code{filename_type*} > @var{file}, @code{counter_type} @var{line}, @code{counter_type} @var{col}) > Create a @code{Location} denoting an empty range located at a given point. > @end deftypeop > > -@deftypemethod {location} {void} initialize (@code{std::string*} @var{file} > = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} @var{col} > = 1) > +@deftypemethod {location} {void} initialize (@code{filename_type*} > @var{file} = nullptr, @code{counter_type} @var{line} = 1, @code{counter_type} > @var{col} = 1) > Reset the location to an empty range at the given values. > @end deftypemethod > > diff --git a/src/muscle-tab.c b/src/muscle-tab.c > index 096c93ca..2cc136a1 100644 > --- a/src/muscle-tab.c > +++ b/src/muscle-tab.c > @@ -447,6 +447,7 @@ muscle_percent_variable_update (char const *variable, > { "api.push_pull", "api.push-pull", > muscle_keyword }, > { "api.tokens.prefix", "api.token.prefix", muscle_code > }, > { "extends", "api.parser.extends", > muscle_keyword }, > + { "filename_type", "api.filename.type", muscle_code > }, > { "final", "api.parser.final", > muscle_keyword }, > { "implements", "api.parser.implements", > muscle_keyword }, > { "lex_symbol", "api.token.constructor", -1 }, > diff --git a/tests/c++.at b/tests/c++.at > index adc41f31..5cbbc91e 100644 > --- a/tests/c++.at > +++ b/tests/c++.at > @@ -46,38 +46,45 @@ template <typename T> > bool > check (const T& in, const std::string& s) > { > + const static bool verbose = getenv ("VERBOSE"); > std::stringstream os; > os << in; > - if (os.str () != s) > + if (os.str () == s) > + { > + if (verbose) > + std::cerr << os.str () << '\n'; > + return true; > + } > + else > { > std::cerr << "fail: " << os.str () << ", expected: " << s << '\n'; > return false; > } > - return true; > } > > int > main (void) > { > + const std::string fn = "foo.txt"; > int fail = 0; > - ]AT_YYLTYPE[ loc; fail += check (loc, "1.1"); > - fail += check (loc + 10, "1.1-10"); > - loc += 10; fail += check (loc, "1.1-10"); > - loc += -5; fail += check (loc, "1.1-5"); > - fail += check (loc - 5, "1.1"); > - loc -= 5; fail += check (loc, "1.1"); > + ]AT_YYLTYPE[ loc (&fn); fail += check (loc, "foo.txt:1.1"); > + fail += check (loc + 10, "foo.txt:1.1-10"); > + loc += 10; fail += check (loc, "foo.txt:1.1-10"); > + loc += -5; fail += check (loc, "foo.txt:1.1-5"); > + fail += check (loc - 5, "foo.txt:1.1"); > + loc -= 5; fail += check (loc, "foo.txt:1.1"); > // Check that we don't go below. > // http://lists.gnu.org/archive/html/bug-bison/2013-02/msg00000.html > - loc -= 10; fail += check (loc, "1.1"); > + loc -= 10; fail += check (loc, "foo.txt:1.1"); > > - loc.columns (10); loc.lines (10); fail += check (loc, "1.1-11.0"); > - loc.lines (-2); fail += check (loc, "1.1-9.0"); > - loc.lines (-10); fail += check (loc, "1.1"); > + loc.columns (10); loc.lines (10); fail += check (loc, "foo.txt:1.1-11.0"); > + loc.lines (-2); fail += check (loc, "foo.txt:1.1-9.0"); > + loc.lines (-10); fail += check (loc, "foo.txt:1.1"); > > ]AT_YYLTYPE[ loc2 (YY_NULLPTR, 5, 10); > fail += check (loc2, "5.10"); > - fail += check (loc + loc2, "1.1-5.9"); > - loc += loc2; fail += check (loc, "1.1-5.9"); > + fail += check (loc + loc2, "foo.txt:1.1-5.9"); > + loc += loc2; fail += check (loc, "foo.txt:1.1-5.9"); > return !fail; > } > ]]) > diff --git a/tests/input.at b/tests/input.at > index d152de31..f7afee2e 100644 > --- a/tests/input.at > +++ b/tests/input.at > @@ -2220,6 +2220,7 @@ AT_DATA([[input.y]], > %define namespace "foo" > %define variant > %define parser_class_name {parser} > +%define filename_type {filename} > %% > start: %empty; > ]]) > @@ -2244,6 +2245,10 @@ input.y:5.1-34: warning: deprecated directive: > '%define parser_class_name {parse > 5 | %define parser_class_name {parser} > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | %define api.parser.class {parser} > +input.y:6.1-32: warning: deprecated directive: '%define filename_type > {filename}', use '%define api.filename.type {filename}' [-Wdeprecated] > + 6 | %define filename_type {filename} > + | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + | %define api.filename.type {filename} > input.y:2.1-40: error: invalid value for %define Boolean variable > 'lr.keep-unreachable-state' > 2 | %define lr.keep_unreachable_states maybe > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >