Re: Bison 3.4.90 released [beta]

[Akim Demaille]

Hi Adrian,

> Le 5 nov. 2019 à 16:40, Adrian Vogelsgesang <address@hidden> a écrit :
> 
> Hi Akim,
> 
> I quickly upgraded Hyper (having a SQL grammar, similar to Postgres’ grammar)
> to use Bison 3.4.90.
> Things are looking good, our internal test cases are successful.
> I did not go through all our tests, though, e.g. I did not check address 
> sanitizer and friends.

I'd feel safer but I already ran it on the CI, so we should be safe enough :)

> I also tested LAC, and it works :)

Good :)

>> positions moved from unsigned (for line and column numbers) to int.
> Yes, I ran into this. Our build was failing at first due to this.
> 
> …/Debug/codegen/hyper/cts/parser/combinedsql.ypp: In function ‘int 
> yylex(hyper::astql::AST**, sqlparser::SQLParser::location_type*, 
> hyper::SQLLexer&, hyper::ASTContainer&)’:
> …/Debug/codegen/hyper/cts/parser/combinedsql.ypp:2504:79: error: narrowing 
> conversion of ‘(unsigned int)(((((long 
> int)token.hyper::SQLLexer::Token::begin) - ((long int)scannerBegin)) (ceiling 
> /) 1) + 1)’ from ‘unsigned int’ to ‘int’ inside { } [-Werror=narrowing]
>    
> (*yyloc)=sqlparser::SQLParser::location_type{sqlparser::position{nullptr,1,static_cast<unsigned>(token.begin-scannerBegin+1)},sqlparser::position{nullptr,1,static_cast<unsigned>(token.end-scannerBegin+1)}};
>                                                                               
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fortunately, it was easy to fix by changing the static_cast<unsigned> to 
> static_cast<int>.
> We are abusing the bison-provided location class to store byte-offsets here, 
> so I guess this falls into the category of “breakage due to API abuse”.

If you're allowed modern c++, decltype should provide you with portability 
across versions of Bison.  Yet we should definitely expose our type in 
locations.  WDYT about this commit?

Cheers!

commit 7bdf7246fbc510fd13e4feeb95d568a6b7eac65f
Author: Akim Demaille <address@hidden>
Date:   Wed Nov 6 08:33:44 2019 +0100

    c++: expose the type used to store line and column numbers
    
    * data/skeletons/location.cc (position::counter_type)
    (location::counter_type): New.
    Use them.
    * doc/bison.texi (C++ position, C++ location): Adjust.

diff --git a/NEWS b/NEWS
index 441d0fca..8e6aed95 100644
--- a/NEWS
+++ b/NEWS
@@ -14,7 +14,8 @@ GNU Bison NEWS
   In C++, line numbers and columns are now represented as 'int' not
   'unsigned', so that integer overflow on positions is easily checkable via
   'gcc -fsanitize=undefined' and the like.  This affects the API for
-  positions.
+  positions.  The default position and location classes now expose
+  'counter_type' (int), used to define line and column numbers.
 
 ** Bug fixes
 
diff --git a/data/skeletons/location.cc b/data/skeletons/location.cc
index e544f707..02583798 100644
--- a/data/skeletons/location.cc
+++ b/data/skeletons/location.cc
@@ -62,11 +62,14 @@ m4_define([b4_location_define],
 [[  /// A point in a source file.
   class position
   {
-  public:]m4_ifdef([b4_location_constructors], [[
+  public:
+    /// 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,
-                       int l = ]b4_location_initial_line[,
-                       int c = ]b4_location_initial_column[)
+                       counter_type l = ]b4_location_initial_line[,
+                       counter_type c = ]b4_location_initial_column[)
       : filename (f)
       , line (l)
       , column (c)
@@ -75,8 +78,8 @@ m4_define([b4_location_define],
 ]])[
     /// Initialization.
     void initialize (]b4_percent_define_get([[filename_type]])[* fn = 
YY_NULLPTR,
-                     int l = ]b4_location_initial_line[,
-                     int c = ]b4_location_initial_column[)
+                     counter_type l = ]b4_location_initial_line[,
+                     counter_type c = ]b4_location_initial_column[)
     {
       filename = fn;
       line = l;
@@ -86,7 +89,7 @@ m4_define([b4_location_define],
     /** \name Line and Column related manipulators
      ** \{ */
     /// (line related) Advance to the COUNT next lines.
-    void lines (int count = 1)
+    void lines (counter_type count = 1)
     {
       if (count)
         {
@@ -96,7 +99,7 @@ m4_define([b4_location_define],
     }
 
     /// (column related) Advance to the COUNT next columns.
-    void columns (int count = 1)
+    void columns (counter_type count = 1)
     {
       column = add_ (column, count, ]b4_location_initial_column[);
     }
@@ -105,13 +108,13 @@ m4_define([b4_location_define],
     /// File name to which this position refers.
     ]b4_percent_define_get([[filename_type]])[* filename;
     /// Current line number.
-    int line;
+    counter_type line;
     /// Current column number.
-    int column;
+    counter_type column;
 
   private:
     /// Compute max (min, lhs+rhs).
-    static int add_ (int lhs, int rhs, int min)
+    static counter_type add_ (counter_type lhs, counter_type rhs, counter_type 
min)
     {
       return lhs + rhs < min ? min : lhs + rhs;
     }
@@ -119,7 +122,7 @@ m4_define([b4_location_define],
 
   /// Add \a width columns, in place.
   inline position&
-  operator+= (position& res, int width)
+  operator+= (position& res, position::counter_type width)
   {
     res.columns (width);
     return res;
@@ -127,21 +130,21 @@ m4_define([b4_location_define],
 
   /// Add \a width columns.
   inline position
-  operator+ (position res, int width)
+  operator+ (position res, position::counter_type width)
   {
     return res += width;
   }
 
   /// Subtract \a width columns, in place.
   inline position&
-  operator-= (position& res, int width)
+  operator-= (position& res, position::counter_type width)
   {
     return res += -width;
   }
 
   /// Subtract \a width columns.
   inline position
-  operator- (position res, int width)
+  operator- (position res, position::counter_type width)
   {
     return res -= width;
   }
@@ -181,6 +184,8 @@ m4_define([b4_location_define],
   class location
   {
   public:
+    /// Type for line and column numbers.
+    typedef position::counter_type counter_type;
 ]m4_ifdef([b4_location_constructors], [
     /// Construct a location from \a b to \a e.
     location (const position& b, const position& e)
@@ -196,8 +201,8 @@ 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,
-                       int l = ]b4_location_initial_line[,
-                       int c = ]b4_location_initial_column[)
+                       counter_type l = ]b4_location_initial_line[,
+                       counter_type c = ]b4_location_initial_column[)
       : begin (f, l, c)
       , end (f, l, c)
     {}
@@ -205,8 +210,8 @@ m4_define([b4_location_define],
 ])[
     /// Initialization.
     void initialize (]b4_percent_define_get([[filename_type]])[* f = 
YY_NULLPTR,
-                     int l = ]b4_location_initial_line[,
-                     int c = ]b4_location_initial_column[)
+                     counter_type l = ]b4_location_initial_line[,
+                     counter_type c = ]b4_location_initial_column[)
     {
       begin.initialize (f, l, c);
       end = begin;
@@ -222,13 +227,13 @@ m4_define([b4_location_define],
     }
 
     /// Extend the current location to the COUNT next columns.
-    void columns (int count = 1)
+    void columns (counter_type count = 1)
     {
       end += count;
     }
 
     /// Extend the current location to the COUNT next lines.
-    void lines (int count = 1)
+    void lines (counter_type count = 1)
     {
       end.lines (count);
     }
@@ -243,39 +248,45 @@ m4_define([b4_location_define],
   };
 
   /// Join two locations, in place.
-  inline location& operator+= (location& res, const location& end)
+  inline location&
+  operator+= (location& res, const location& end)
   {
     res.end = end.end;
     return res;
   }
 
   /// Join two locations.
-  inline location operator+ (location res, const location& end)
+  inline location
+  operator+ (location res, const location& end)
   {
     return res += end;
   }
 
   /// Add \a width columns to the end position, in place.
-  inline location& operator+= (location& res, int width)
+  inline location&
+  operator+= (location& res, location::counter_type width)
   {
     res.columns (width);
     return res;
   }
 
   /// Add \a width columns to the end position.
-  inline location operator+ (location res, int width)
+  inline location
+  operator+ (location res, location::counter_type width)
   {
     return res += width;
   }
 
   /// Subtract \a width columns to the end position, in place.
-  inline location& operator-= (location& res, int width)
+  inline location&
+  operator-= (location& res, location::counter_type width)
   {
     return res += -width;
   }
 
   /// Subtract \a width columns to the end position.
-  inline location operator- (location res, int width)
+  inline location
+  operator- (location res, location::counter_type width)
   {
     return res -= width;
   }
@@ -304,7 +315,8 @@ m4_define([b4_location_define],
   std::basic_ostream<YYChar>&
   operator<< (std::basic_ostream<YYChar>& ostr, const location& loc)
   {
-    int end_col = 0 < loc.end.column ? loc.end.column - 1 : 0;
+    location::counter_type end_col
+      = 0 < loc.end.column ? loc.end.column - 1 : 0;
     ostr << loc.begin;
     if (loc.end.filename
         && (!loc.begin.filename
diff --git a/doc/bison.texi b/doc/bison.texi
index 0f5838c2..fbd3edfb 100644
--- a/doc/bison.texi
+++ b/doc/bison.texi
@@ -11453,8 +11453,8 @@ 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 - class position
+@c - class location
 @c - %define filename_type "const symbol::Symbol"
 
 When the directive @code{%locations} is used, the C++ parser supports
@@ -11462,9 +11462,9 @@ location tracking, see @ref{Tracking Locations}.
 
 By default, two auxiliary classes define a @code{position}, a single point
 in a file, and a @code{location}, a range composed of a pair of
-@code{position}s (possibly spanning several files).  But if the
-@code{%define} variable @code{api.location.type} is defined, then these
-classes will not be generated, and the user defined type will be used.
+@code{position}s (possibly spanning several files).  If the @code{%define}
+variable @code{api.location.type} is defined, then these classes will not be
+generated, and the user defined type will be used.
 
 @menu
 * C++ position::         One point in the source file
@@ -11477,13 +11477,17 @@ classes will not be generated, and the user defined 
type will be used.
 @node C++ position
 @subsubsection C++ @code{position}
 
-@deftypeop {Constructor} {position} {} position (std::string* @var{file} = 
nullptr, int @var{line} = 1, int @var{col} = 1)
+@defcv {Type} {position} {counter_type}
+The type used to store line and column numbers.  Defined as @code{int}.
+@end defcv
+
+@deftypeop {Constructor} {position} {} position (std::string* @var{file} = 
nullptr, counter_type @var{line} = 1, 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 (std::string* @var{file} = 
nullptr, int @var{line} = 1, int @var{col} = 1)
+@deftypemethod {position} {void} initialize (std::string* @var{file} = 
nullptr, counter_type @var{line} = 1, counter_type @var{col} = 1)
 Reset the position to the given values.
 @end deftypemethod
 
@@ -11494,28 +11498,28 @@ change it to @samp{@var{type}*} using @samp{%define 
filename_type
 "@var{type}"}.
 @end deftypeivar
 
-@deftypeivar {position} {int} line
+@deftypeivar {position} {counter_type} line
 The line, starting at 1.
 @end deftypeivar
 
-@deftypemethod {position} {void} lines (int @var{height} = 1)
+@deftypemethod {position} {void} lines (counter_type @var{height} = 1)
 If @var{height} is not null, advance by @var{height} lines, resetting the
 column number.  The resulting line number cannot be less than 1.
 @end deftypemethod
 
-@deftypeivar {position} {int} column
+@deftypeivar {position} {counter_type} column
 The column, starting at 1.
 @end deftypeivar
 
-@deftypemethod {position} {void} columns (int @var{width} = 1)
+@deftypemethod {position} {void} columns (counter_type @var{width} = 1)
 Advance by @var{width} columns, without changing the line number. The
 resulting column number cannot be less than 1.
 @end deftypemethod
 
-@deftypemethod {position} {position&} operator+= (int @var{width})
-@deftypemethodx {position} {position} operator+ (int @var{width})
-@deftypemethodx {position} {position&} operator-= (int @var{width})
-@deftypemethodx {position} {position} operator- (int @var{width})
+@deftypemethod {position} {position&} operator+= (counter_type @var{width})
+@deftypemethodx {position} {position} operator+ (counter_type @var{width})
+@deftypemethodx {position} {position&} operator-= (counter_type @var{width})
+@deftypemethodx {position} {position} operator- (counter_type @var{width})
 Various forms of syntactic sugar for @code{columns}.
 @end deftypemethod
 
@@ -11538,11 +11542,11 @@ Create a @code{Location} from the endpoints of the 
range.
 @end deftypeop
 
 @deftypeop {Constructor} {location} {} location (const position& @var{pos} = 
position())
-@deftypeopx {Constructor} {location} {} location (std::string* @var{file}, int 
@var{line}, int @var{col})
+@deftypeopx {Constructor} {location} {} location (std::string* @var{file}, 
counter_type @var{line}, counter_type @var{col})
 Create a @code{Location} denoting an empty range located at a given point.
 @end deftypeop
 
-@deftypemethod {location} {void} initialize (std::string* @var{file} = 
nullptr, int @var{line} = 1, int @var{col} = 1)
+@deftypemethod {location} {void} initialize (std::string* @var{file} = 
nullptr, counter_type @var{line} = 1, counter_type @var{col} = 1)
 Reset the location to an empty range at the given values.
 @end deftypemethod
 
@@ -11551,15 +11555,15 @@ Reset the location to an empty range at the given 
values.
 The first, inclusive, position of the range, and the first beyond.
 @end deftypeivar
 
-@deftypemethod {location} {void} columns (int @var{width} = 1)
-@deftypemethodx {location} {void} lines (int @var{height} = 1)
+@deftypemethod {location} {void} columns (counter_type @var{width} = 1)
+@deftypemethodx {location} {void} lines (counter_type @var{height} = 1)
 Forwarded to the @code{end} position.
 @end deftypemethod
 
-@deftypemethod  {location} {location} operator+  (int @var{width})
-@deftypemethodx {location} {location} operator+= (int @var{width})
-@deftypemethodx {location} {location} operator-  (int @var{width})
-@deftypemethodx {location} {location} operator-= (int @var{width})
+@deftypemethod  {location} {location} operator+  (counter_type @var{width})
+@deftypemethodx {location} {location} operator+= (counter_type @var{width})
+@deftypemethodx {location} {location} operator-  (counter_type @var{width})
+@deftypemethodx {location} {location} operator-= (counter_type @var{width})
 Various forms of syntactic sugar for @code{columns}.
 @end deftypemethod