Re: [PATCH 0/4] RFC: add stats about parsing

[Adrian Vogelsgesang]

Hi Akim,

collecting statistics about a parse sounds like a great idea to me
and I am looking forward to it.
Thanks for the time and effort you spent on this! :)

For my projects, I don’t care which exact option is pursued since
I never use `yydebug` in commited code. I only add debug
code temporarily, fix the bug, and then remove the debug code
again before commiting anything to master.

I don’t think anyone should have a dependency in production
code on `yydebug` behavior. Hence, I guess breaking the
POSIX standard here would be fine.
Still, I share your concerns about breaking standards.

Hence, I would be in favour of variant 2 or 3.

One question regarding variant 3a:
What would be the outcome of the combination
> %define parse.trace false
> %define parse.stats true

Best,
Adrian

From: bison-patches <bison-patches-bounces+avogelsgesang=address@hidden> on 
behalf of Akim Demaille <address@hidden>
Date: Wednesday, 3 July 2019 at 07:26
To: "address@hidden" <address@hidden>
Cc: Paul Eggert <address@hidden>, Akim Demaille <address@hidden>
Subject: [PATCH 0/4] RFC: add stats about parsing

For an ongoing work, I find useful to provide a means for the parser
to emit a brief summary of what was done: number of shifts, number of
reductions, etc. (there could/should be more: number of states, number
of error actions...).

It goes naturally with %debug (aka %define parse.trace), which is
controlled by the yydebug variable. I don't want yet another
variable, yydebug should be enough to decide what kind of traces one
wants: it can be used as a bitfield instead of a Boolean coded in an
int.

One problem is that POSIX reads:
(http://pubs.opengroup.org/onlinepubs/007904875/utilities/yacc.html<http://pubs.opengroup.org/onlinepubs/007904875/utilities/yacc.html>)

> In parsers where the debugging code has been included, the external
> int yydebug can be used to turn debugging on (with a non-zero value)
> and off (zero value) at runtime. The initial value of yydebug shall be
> zero.

and Bison's doc says so far:

> Each step taken by the parser when @code{yydebug} is nonzero
> produces a line or two of trace information, written on
> @code{stderr}.

and

> @deffn {Variable} yydebug
> External integer variable set to zero by default. If @code{yydebug}
> is given a nonzero value, the parser will output information on input
> symbols and parser action. @xref{Tracing, ,Tracing Your Parser}.
> @end deffn

which goes against my willing to use yydebug as a bit field, as I wish
yydebug = 2, for instance, to mean: "report the summary, but not the
debug traces".


I see several ways to address this (there may be more of course):

1. we don't care about what POSIX says: people don't assign 2 to
yydebug to enable traces, they use 1, or possibly -1, but not 2.
So use yydebug as it is, without any particular precaution.

2. we care about what POSIX says, but agree to change what bison.texi
says:
- if %yacc/--yacc is enabled, then yydebug is a Boolean and
2a. whatever its value, we dump only the traditional traces
2b. whatever its value we dump all the existing traces (old and new)
- if %yacc is disabled, then yydebug is a bitfield

3. we refuse to depart from what the current documentation says so we
can't change the nature of yydebug this way, so let's use another
variable for this.

3a. The patch below explores this with a new variable: %define
parse.stats.

3b. Let's reuse %define parse.trace with a new value: %define
parse.trace full


I dislike option 3, it's a serious maintenance burden. The skeleton
is more complex, the documentation needs to address that new %define
variable, the test suite must face yet another alternative (which is a
really problem: we simply cannot explore all the combinatorics of the
existing options, yet the test suite is already quite big and does
much to cover the main cases), etc. This is however the path that the
following commits explore. Now that I've done it, I can really tell I
don't like that: the ratio benefit/cost is low.

My favorite option is 1, on the ground that "does anybody seriously
use something else that 1/-1 vs. 0 here???".

But after all, a spec is a spec, even it wastes 31 to 63 bits
(actually, I would also feel safer if yydebug were an unsigned instead
of an int, but I don't think we can change this). So my second choice
is 2b: in Yacc mode, flatten all the kinds of traces to yydebug being
0 or not; in Bison mode, use yydebug as a bitfield and adjust the
documentation.

I really need opinions on this matter, thanks in advance to all!


Akim Demaille (4):
parse.stats: new feature of yacc.c
parse.stats: add --trace=parse-stats
regen
parse.stats: documentation

NEWS | 9 ++++++
data/skeletons/c.m4 | 17 ++++++++++-
data/skeletons/yacc.c | 50 +++++++++++++++++++++++--------
doc/bison.texi | 43 ++++++++++++++++++++++++++
src/getargs.c | 38 ++++++++++++-----------
src/getargs.h | 35 +++++++++++-----------
src/parse-gram.c | 70 +++++++++++++++++++++++++++++--------------
src/parse-gram.h | 10 ++++++-
src/parse-gram.y | 1 +
src/reader.c | 4 ++-
10 files changed, 205 insertions(+), 72 deletions(-)

--
2.22.0