Re: Latest set of patches to the java-push branch

[Akim Demaille]

Hi Dennis,

I'm about to push the patches in master.

Le 3 juin 2013 à 18:33, Dennis Heimbigner <address@hidden> a écrit :

> I deliberately did not do the following suggested changes.
> 
> * data/lalr1.java
> 
>  1. >It looks like this is a single b4_push_if, but with two branches
>> (if you fuse it with the previous b4_push_if).
>    Rationale: I have chosen not to do this because the code
>    with its escapes is confusing enough. My principle is that
>    using a parenthese matching editor(e.g. emacs) it should
>    be possible to have a closing macro ')' show a
>    corresponding b4_...  This deliberately entails not using
>    both arms of a two arm conditional macro (such as
>    b4_push_if).

I don't have such problems with Emacs, it does match braces,
brackets and parens appropriately.

> * doc/bison.texi
> 
>  1. >I'm not sure this use of @pxref{} is really conform to the
>> Texinfo specification.  Have you checked what it looks like
>> in PDF and HTML?
>     Rationale: code like this was already in bison.texit. I verified
>     that it looks ok in html.

The documentation reads:

> 8.4.4 address@hidden' with Three Arguments
> ----------------------------------
> 
> A third argument replaces the node name in the TeX output.  The third
> argument should be the name of the section in the printed output, or
> else state the topic discussed by that section.  Often, you will want to
> use initial upper case letters so it will be easier to read when the
> reference is printed.  Use a third argument when the node name is
> unsuitable because of syntax or meaning.

and you wrote

   +parser (@pxref{%define Summary,,api.push-pull}):

twice, although the third argument is not the name of the section.
But indeed, it accepts "the topic discussed by that section", so
I guess it's OK.  However in the rest of the Bison documentation,
I do believe we stick to the section name.  But I might be wrong.
Forget about it if the result is ok (I would expect @code{} though
for the variable name).

> * tests/javapush.at
>>  1. >If this piece of code is alike something that is already available
>> in C, consider factoring all this in the c-like.m4 file, which is
>> common to C, C++ and Java.
>     Rationale: I tried this but it caused another test (push.at) to fail.

I'll give it a try later.

>>  2. >You don't need to do that: if java.at is loaded before, the
>> definition is still available.  It was probably needed because
>> you had removed all the other files from testsuite.at.
>     Rationale: I assume this is with respect to the AT_CHECK_JAVA_GREP
>     macro.  The reason I included it is because I needed to change it to
>     support matching zero lines.

Sorry, I missed the fact that the macro body is different.  Using the
same name is a bit confusing.

>>  3. >Is there a means to factor this calculator from another test?
>>     Rationale: I was not able to find any way to do this. Autotest
>>     (deliberately, I think) makes each test completely independent,
>>     so code sharing appears to be difficult.

I was referring to making a macro that generates all these common
tests.  Have a look at calc.at: it is used by many different
types of parsers.  But I can live with it as is.

> * doc/bison.texi
>> I don't get it here.  Are you saying that yyerror is provided
>> by the scanner?
>  Answer: Yes, the lex interface defines yyerror(), so in order
>  to use yyerror(), an implementation of the scanner interface is required.
>  Fixing this seems like a good idea, but have no idea what
>  the appropriate change should be. In any case, I added some
>  discussion about this to the bison.texi file.

Do you mean that the fact that yyerror is part of the scanner
interface dates back to the original Java parser skeleton?


> * tests/javapush.at
>  3. > +AT_CHECK([[sed -e 's/(line[ ][0-9][0-9]*)[,]/,/' 
> stderr]],[ignore],[expout])
>> You don't seem to be checking anything at all here.
>     Answer: AT_CHECK has a feature that if you insert expout or experr
>     in the stdout or stderr arguments, then it compares the output
>     of the command to the file named "expout" or "experr". So this
>     line actually both computes and compares.

My bad, thanks!