Hi Dennis, I'm about to push the patches in master.
Le 3 juin 2013 à 18:33, Dennis Heimbigner <[email protected]> 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 `@xref' 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!
