Re: FYI: Adjust @$ for empty reductions

[Paul Eggert]

Akim Demaille <address@hidden> writes:

> I don't read locations as you do.  For me positions (two of which
> are referring to the space in between characters.  The first
> position is immediately pointing before the first character, and
> the second is immediately the last.

But Bison itself doesn't work that way, in the locations that it reports.
For example:

   $ echo foo >t.y
   $ bison t.y
   t.y:1.1-3: syntax error, unexpected "identifier"

Here the location contains three characters, numbered 1.1 through 1.3,
and the error message mentions the first and the last characters.
Bison doesn't mention 1.4, which it would if it were following the
intepretation that the location's upper bound is the column number of
the first character after the location.

The Bison manual also uses this tradition in its example, which
contains this code:

                       printf("Division by zero, l%d,c%d-l%d,c%d",
                              @3.first_line, @3.first_column,
                              @3.last_line, @3.last_column);

Again, this uses last_column as the column of the last character
in the location, not the first character after the location.

> An empty location is a location such that begin = end.

I prefer this sort of solution as well; that is why I used it in
parse-gram.y.  Also, on further thought I don't think it's that it's a
huge backwards-compatibility issue.  I don't know of any real-world
grammars that use Bison locations, other than Bison itself.  I suppose
there are some, but they're relatively rare.

However, we should make it clear that it is an incompatible change to
Bison.  We need to update the documentation, and particularly the NEWS
file, accordingly.

This raises the issue of whether the current default struct is all
that useful.  Better would be the sort of struct defined in
location.h.  But that is an even bigger change.

Perhaps it would be better to put off all incompatible changes like
this until Bison 2.0 is released.  That is, we could release Bison 2.0
shortly, without changing the upper bounds of locations; then we could
design and implement a better location scheme for Bison 2.1.  I don't
particularly care what the version numbers are called, but I do think
it'd be nice to have a patched version before making any incompatible
changes.

>> The patch replaces an expression with a statement
>> (omitting its trailing ";").  This isn't an upward-compatible change,
>
> It has never been documented as such, and frankly, I don't think it
> matters at all.

OK.  It's not a big deal.

>> I suspect that you'll get more-efficient code if you revamp the macro
>> to merge duplicate code.  Certainly the code will be shorter.
>
> That's the compiler's job, not mine.

The code is also clearer, in my opinion, as it highlights the parts
of the actions that actually differ.

> I should double checked, but I did not mean to escape from GNU
> style.  What did I miss?

A space between the while and the (0), and the overall indentation
of do-while.

How about this definition instead?  It attempts to answer
your objections.

# define YYLLOC_DEFAULT(Current, Rhs, N)                                \
    do                                                                  \
      {                                                                 \
        (Current).last_line   = (Rhs)[N].last_line;                     \
        (Current).last_column = (Rhs)[N].last_column;                   \
        if (N)                                                          \
          {                                                             \
            (Current).first_line   = (Rhs)[1].first_line;               \
            (Current).first_column = (Rhs)[1].first_column;             \
          }                                                             \
        else                                                            \
          {                                                             \
            (Current).first_line   = (Current).last_line;               \
            (Current).first_column = (Current).last_column;             \
          }                                                             \
      }                                                                 \
    while (0)