Hi Adrian,
> Le 5 nov. 2019 à 16:40, Adrian Vogelsgesang <avogelsges...@tableau.com> 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 <akim.demai...@gmail.com>
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
