bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: --report


From: Akim Demaille
Subject: Re: --report
Date: 25 May 2002 09:04:59 +0200
User-agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Honest Recruiter)

Hi Paul,

That's a lot for the feedback, and the English lecture :)  I really
needed the latter :(

| > From: Akim Demaille <address@hidden>
| > Date: 20 May 2002 20:11:47 +0200
| 
| > address@hidden 1
| > +
| > +The textual file is generated when the options @option{--report} or
| > address@hidden are specified, see @xref{Invocation, , Invoking
| > +Bison}.
| 
| I assume the "@sp 1" is a typo?

It was not, it's probably related to my LaTeX style where I like to
use \medskip more than people seem to use sp in Texinfo.  The point
was detaching the introduction from the body, but I'll remove it.


 
| > +As expected, @command{bison} reports that @samp{calc.y contains 1
| 
| I don't understand why the two words "As expected" are there.
| Perhaps they should be removed?

Well, given the grammar, it really is to be expected :)  OK, removed.


| > +Then states which still have conflicts are listed.
| 
| Change to:
| The next section lists states that still have conflicts.
| 
| (I prefer active voice to passive.)

I prefer too, but I've always been told, in particular in scientific
papers, to promote passive :(


| > +no such transition on a nonterminal symbol, and the lookahead is a
| 
| Sorry, I don't understand what the phrase "If there is no such
| transition on a nonterminal symbol" is getting at.  NUM and exp are
| both lookaheads; one is a nonterminal, the other a terminal.  Perhaps
| the point should be made that exp is a lookahead too?

Arg.  We disagree on the way to express gotos.  To me, it is an heresy
to name `exp' a lookahead here, both from the theoretical point of
view, and the implementation point of view.

| > +#define report_none        (0)
| > +#define report_states      (1 << 0)
| > +#define report_itemsets    (1 << 1 | report_states)
| > +#define report_lookaheads  (1 << 2 | report_states)
| > +#define report_all         (~0)
| > +extern int report_flag;
| > +/* Return TRUE iff THING ought to be reported.  */
| > +#define report_p(Thing)   ((report_flag & report_##Thing) == 
report_##Thing)
| 
| This is a fairly minor point, but I would parenthesize 1<<1 and 1<<2
| to avoid warnings with some compilers.  

Thanks.

| Better yet, I would use an enum,

I refrain using enums when I play bitwise tricks.  In such a
framework, the advantage of enums vs #defines is unclear to me (as, I
doubt that debuggers are able to report the value `a|b' as the string
`a|b' in case of enums.  Put after all, why not.).

Also, I remember I found once a compiler that complained about
operations on enums, which I appreciated.  ISTR it was SGI.

| and remove the funny dependency of report_itemsets on
| report_states.  I.e.:
| 
|   enum
|   {
|     report_none = 0,
|     report_states = 1 << 0,
|     report_itemsets = 1 << 1,
|     report_lookaheads = 1 << 2,
|     report_all = ~0
|   };
|   extern int report_flag;
| 
| and then in the rest of the code, uniformly replace:
| 
| report_p (states) -> report_flag
| report_p (itemsets) -> report_flag & itemsets
| report_p (lookaheads) -> report_flag & lookaheads
| 
| This will be easier to understand, for me at least

I agree, but it was not implementing the dependency between `state'
and the others.  That's why I preferred this way.

But I can see your point: this dependency is best expressed in the
argmatch tables.  I'll do that.

Thanks for all your useful comments!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]