bison-patches
[Top][All Lists]
Advanced

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

Re: [SPAM] Re: [updated PATCH] %language declaration


From: Joel E. Denny
Subject: Re: [SPAM] Re: [updated PATCH] %language declaration
Date: Tue, 12 Dec 2006 21:13:55 -0500 (EST)

On Mon, 11 Dec 2006, Paolo Bonzini wrote:

> > 2. The version number 2.3a+ is a temporary version number
> 
> Ok, but then I have to change extexi to drop the %require version.  See the
> attached patch.

Sorry, I didn't think of that.  Thanks for bringing it up.  This is 
another case where Paul Eggert's opinion would be especially helpful.

I'm afraid your solution has a different problem: it doesn't test the 
examples as written.  Imagine the case where the current version needed to 
be %require'd, but the examples as written in the documentation still 
%require'd an old version.  We'd never be warned because the extracted 
version of the examples would always %require the current version.

What about the following?  Revert your change to examples/extexi and put 
your original 2.3a+ back into the %require in the manual.  Then let's add 
something like:

Index: tests/Makefile.am
===================================================================
RCS file: /sources/bison/bison/tests/Makefile.am,v
retrieving revision 1.43
diff -p -u -r1.43 Makefile.am
--- tests/Makefile.am   8 Nov 2006 20:28:57 -0000       1.43
+++ tests/Makefile.am   12 Dec 2006 23:54:04 -0000
@@ -69,6 +69,13 @@ clean-local:
 
 check-local: atconfig atlocal $(TESTSUITE)
        $(SHELL) $(TESTSUITE) $(TESTSUITEFLAGS)
+       @if grep '%require[     ][      ]*".*+"' \
+         $(top_srcdir)/doc/bison.texinfo > /dev/null; then \
+           echo ''; \
+           echo 'ERROR: $(top_srcdir)/doc/bison.texinfo has a %require 
specifying a development version.'; \
+           echo ''; \
+           exit 1; \
+       fi
 
 check_SCRIPTS = bison

We'll get this warning for every `make check' during development until we 
update the version number, but we can ignore it.  This warning will go 
away upon our next release (when we'll be forced to update it) and until 
the minimum required version for these examples changes again.

I'm not sure exactly how the tarball is rolled for Bison, but in many 
projects a failed `make check' prevents the tarball from being rolled.  
Or is it `make maintainer-check' in our case?  Is there a better place for 
consistency checks?  Paul?

You could even consider changing these lines in your patch:

  language name is case-insensitive}.  These were introduced
  in Bison 2.3b; for compatibility with earlier versions, you

to these:

  language name is case-insensitive}.  These were introduced
  @comment %require "2.3a+"
  in Bison 2.3a+; for compatibility with earlier versions, you

Then our guess about the next version number can't bite us here either.

> > 3. I think we should document this %language feature as experimental

> All fixed.

What about the manual?

> > 6. I think your new m4_fatal messages should follow the format of the
> > existing ones in glr.cc and lalr1.cc.
> 
> I think there is no reason to mention the skeleton name in the error messages
> -- and I didn't put it here because it is as ugly as "c-skel.m4".  The real
> fix would be to use another "@foo" directive, parsed by scan-skel.l and
> executed as a "complain".
>
> > By the way, can internationalization work with m4_fatal?
> 
> No.  Calling the gettext function could be achieved with the same solution I
> outlined above.  But this is part of a bigger problem, which is that xgettext
> does not know about m4 syntax.

I know little about gettext, but an @complain sounds like the right way to 
go to me.  Thanks.  I think you should keep what you have here for now, 
and we can replace all the m4_fatal's in a separate patch some day.

> > 7. I seem to recall that Akim dislikes the ".tab" that appears in output
> > file names.  I don't care for it, and Automake prefers to remove it.  Of
> > course, we hang on to it anyway for backward-compatibility.  Since %language
> > is new, is this perhaps a good opportunity to get rid of ".tab" for all
> > languages?
> 
> I don't know, I view it as more consistent %language "C" is a no-op. But I'm
> not going to argue much on this. :-)

I'd still like to drop the ".tab".  As long as we say %language is 
experimental, we can change our minds later if it causes trouble.

> What do you think of the attached patch?

I'd like to see the NEWS entry for %language a little higher... maybe in 
the first four entries or so.  It's kinda buried under all my stuff (which 
I should probably abbreviate anyway).

I see you've mentioned Java in the manual in a few places, but your patch 
doesn't yet add support for Java.

What if we merge the valid_languages table (appearing in src/getargs.c) 
with data/c++-skel.m4 and data/c-skel.m4 to produce a single 
data/language.m4?  This kind of single config script for %language would 
make it easier for users and developers to add support for new languages 
and skeletons without rebuilding Bison.  I think it would also simplify 
your patch and reduce %language's footprint in the Bison front-end source. 
(That might help quell some objections to %language, but I don't know.)

The implementation shouldn't be tough.  Bison would always process 
language.m4.  If the user didn't specify %language, the front-end would 
leave b4_language undefined and language.m4 would select the default 
language, C.  If the user didn't specify %skeleton, the front-end would 
leave b4_skeleton undefined, and language.m4 would select the appropriate 
skeleton.  If the user *did* specify %language and didn't specify the name 
for an output file, the front-end would leave it (b4_spec_defines_file or 
b4_parser_file_name) undefined and language.m4 would define it using 
b4_file_name_all_but_ext and the appropriate extension for the language.

What do you think?




reply via email to

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