* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET: > OK to apply to a temporary branch off of maint, and merge to master?
The patch is ok with nits addressed. > BTW, notice that I'm planning to further extend the Lex/Yacc tests > and make them more "semantic", but that should be better done in a > follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well (or is willing to learn) could look into some of the issues. I should warn that it's not easy though to come up with coherent/consistent and portable improvements. > Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support. > > * tests/lexcpp.test: New test script, on support for Lex + C++. > * tests/lexvpath.test: New test script, test build and rebuild > rules for lexers in VPATH setup. > * tests/yacc-basic.test: New test script, run simple "semantic" > checks on basic Yacc support (similarly to what lex3.test does > for Lex support). > * tests/lex.test: Don't create useless dummy source file joe.l. > Remove extra blank lines. > (Makefile.am): Remove useless definition of $(LDADD). > * tests/lex4.test: Add trailing `:' command. Do not create dummy > useless lex source file. > * tests/lex2.test: Likewise. Call automake with the `-a' option, > so that it doesn't fail for the absence of `ylwrap' script. Make > grepping of automake stderr stricter. > * tests/yacc7.test: Add trailing `:' command. Enable `errexit' > shell flag earlier (just after having sourced ./defs). > * tests/yacc4.test: Likewise. Also ... > (configure.in): Use pre-populated skeleton set up by ./defs, > instead of writing one from scratch. > Other minor cosmetic changes. > * tests/yacc5.test: Likewise. > * tests/yaccvpath.test: Likewise. Also ... > ($distdir): New variable. > Use it throughout. > * tests/lex5.test: Likewise. > * tests/lex3.test: Likewise. Check the distdir, rather than > grepping the distribution tarball. Extend the test on the > created binary, and be sure to avoid hangs. Add some comments. > * tests/yacc.test: Use stricter grepping. Add trailing `:'. > * tests/yacc6.test: Likewise. > * tests/yacc3.test: Likewise. Prefer `cp -f' over plain `cp'. > Do not create unused file `Makefile.sed'. Remove useless rules > from Makefile.am. Other minor cosmetic changes. > * tests/yacc2.test: Make grepping of generated `Makefile.in' and > of automake error messages stricter. Do not redirect output of > grep to /dev/null. Prefer `cp -f' over plain `cp'. Move call to > aclocal earlier. Reduce the number of empty blank lines. Fix a > typo in comments. > * tests/yacc8.test: Fixed bugs that reduced the completeness of > the tests. Added trailing `:' command. > (configure.in): Use pre-populated skeleton set up by ./defs, > instead of writing one from scratch. > * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and > `.yxx', rather than only `.yy'. > * tests/defs.in ($required): Better recognition of requirement > "flex". > * tests/Makefile.am (TESTS): Update. > --- a/tests/defs.in > +++ b/tests/defs.in > @@ -113,6 +113,13 @@ do > echo "$me: running etags --version -o /dev/null" > ( etags --version -o /dev/null ) || exit 77 > ;; > + flex) > + # Since flex is required, we pick LEX for ./configure. > + LEX=flex > + export LEX > + echo "$me: running flex --version" > + ( flex --version ) || exit 77 > + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? > +++ b/tests/lex.test > @@ -26,22 +26,16 @@ END > cat > Makefile.am << 'END' > bin_PROGRAMS = zot > zot_SOURCES = joe.l > -LDADD = @LEXLIB@ Please don't remove that. It is documented to be required. > END > --- a/tests/lex3.test > +++ b/tests/lex3.test > +# Basic semantic checks on Lex support. > # Test associated with PR 19. > # From Matthew D. Langston. > cat > Makefile.am << 'END' > @@ -46,38 +44,44 @@ END > > cat > foo.l << 'END' > %% > -"END" return EOF; > +"GOOD" return EOF; > . > %% > int > main () > { > - while (yylex () != EOF) > - ; > - > - return 0; > + if (yylex () == EOF) > + return 0; > + else > + return 1; That's not "semantic" either. One wouldn't write a lexer like that. > } > END > +# Program should build and run. > $MAKE > -echo 'This is the END' | ./foo > -$MAKE distcheck > +echo GOOD | ./foo > +echo BAD | ./foo && Exit 1 > --- a/tests/lex5.test > +++ b/tests/lex5.test > @@ -33,10 +29,9 @@ AC_OUTPUT > END > > cat > Makefile.am << 'END' > -AUTOMAKE_OPTIONS = subdir-objects > -LDADD = @LEXLIB@ > - > -bin_PROGRAMS = foo/foo > +AUTOMAKE_OPTIONS = subdir-objects > +LDADD = @LEXLIB@ > +bin_PROGRAMS = foo/foo > foo_foo_SOURCES = foo/foo.l Please. Such changes cost you time, me time, clutter up the history, and gain nobody anything. We've been there before. > END > > --- /dev/null > +++ b/tests/lexcpp.test > @@ -0,0 +1,46 @@ > +#! /bin/sh > +# Test to make sure Lex + C++ is supported. > +# Please keep this is sync with sister test yaccpp.test. > + > +. ./defs || Exit 1 > + > +set -e > + > +cat >> configure.in << 'END' > +AC_PROG_CXX > +AC_PROG_LEX > +END > + > +cat > Makefile.am << 'END' > +bin_PROGRAMS = foo bar baz qux > +foo_SOURCES = foo.l++ > +bar_SOURCES = bar.lpp > +baz_SOURCES = baz.ll > +qux_SOURCES = qux.lxx > +END > + > +$ACLOCAL > +$AUTOMAKE -a > + > +sed -e 's/^/ /' -e 's/$/ /' Makefile.in >mk > +$FGREP ' foo.c++ ' mk > +$FGREP ' bar.cpp ' mk > +$FGREP ' baz.cc ' mk > +$FGREP ' qux.cxx ' mk > + > +: > --- /dev/null > +++ b/tests/lexvpath.test > @@ -0,0 +1,117 @@ > +# This test checks that dependent files are updated before including > +# in the distribution. `lexer.c' depends on `lexer.l'. The later is s/\. /. / (several instances) latter > +# updated so that `lexer.c' should be rebuild. Then we are running rebuilt > +# `make' and `make distdir' and check whether the version of `lexer.c' > +# to be distributed is up to date. > + > +# Please keep this in sync with sister test `yaccvapth.test'. > + > +required='gcc flex' > +. ./defs || Exit 1 > + > +set -e > + > +distdir=$me-1.0 > + > +cat > lexoutroot.in << 'END' > +LEX_OUTPUT_ROOT='@LEX_OUTPUT_ROOT@' > +END > + > +cat >> configure.in << 'END' > +AC_CONFIG_FILES([lexoutroot]) > +AC_PROG_CC > +AC_PROG_LEX > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +bin_PROGRAMS = foo > +foo_SOURCES = lexer.l foo.c > +LDADD = $(LEXLIB) > +END > + > +# Original lexer, with a "foobar" comment > +cat > lexer.l << 'END' > +%% > +"END" return EOF; > +. > +%% > +/*foobar*/ > +END > + > +cat > foo.c << 'END' > +int main () { return 0; } > +END > + > +$ACLOCAL > +$AUTOCONF > +$AUTOMAKE -a > + > +mkdir sub > + > +# We must run configure early, to find out whay $LEX_OUTPUT_ROOT is. > +cd sub > +../configure > +. ./lexoutroot > +test -n "$LEX_OUTPUT_ROOT" # sanity check > +cd .. > + > +$LEX lexer.l > +mv "$LEX_OUTPUT_ROOT".c lexer.c > + > +cd sub > + > +# A delay is needed to make sure that the new lexer.l is indeed newer > +# than lexer.c, i.e. the they don't have the same timestamp. > +$sleep > + > +# New lexer, with `fubar' comment. > +cat > ../lexer.l << 'END' > +%% > +"END" return EOF; > +. > +%% > +/*fubar*/ > +END > + > +$MAKE > +$MAKE distdir > +$FGREP '/*fubar*/' $distdir/lexer.c > + > +# > +# Now check to make sure that `make dist' will rebuild the parser. > +# > + > +# A delay is needed to make sure that the new parse.y is indeed newer > +# than parse.c, i.e. the they don't have the same timestamp. The comment was already too verbose before you copy and pasted it. ;-) > +$sleep > + > +# New lexer, with `maude' comment. > +cat > ../lexer.l << 'END' > +%% > +"END" return EOF; > +. > +%% > +/*maude*/ > +END > + > +$MAKE distdir > +$FGREP '/*maude*/' $distdir/lexer.c > + > +: > --- /dev/null > +++ b/tests/yacc-basic.test > @@ -0,0 +1,78 @@ > +# Basic semantic checks on Yacc support. > + > +required=bison > +. ./defs || Exit 1 > + > +set -e > + > +distdir=$me-1.0 > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AC_PROG_YACC > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +bin_PROGRAMS = foo > +foo_SOURCES = parse.y foo.c > +END > + > +cat > parse.y << 'END' > +%{ > +#include <stdio.h> > +#include <stdlib.h> > +int yylex () { return (getchar()); } space before open paren (after getchar). > +void yyerror (char *s) {} > +%} > +%% > +a : 'a' { exit(0); }; > +END > + > +cat > foo.c << 'END' > +int main () { yyparse(); return 1; } see above > +END > + > +$ACLOCAL > +$AUTOCONF > +$AUTOMAKE -a > + > +./configure > +$MAKE > + > +echo a | ./foo > +echo b | ./foo && Exit 1 > + > +# The generated file `parse.c' must be shipped. > +$MAKE distdir > +test -f $distdir/parse.c > + > +# Sanity check on distribution. > +$MAKE distcheck > + > +# While we are at it, make sure that parse.c is erased by > +# maintainer-clean, and not by distclean. > +test -f parse.c > +$MAKE distclean > +test -f parse.c > +./configure # we must re-create `Makefile' > +$MAKE maintainer-clean > +test ! -f parse.c > + > +: > --- a/tests/yacc2.test > +++ b/tests/yacc2.test > @@ -1,5 +1,6 @@ > #! /bin/sh > -# Copyright (C) 1999, 2001, 2002, 2003, 2006 Free Software Foundation, Inc. > +# Copyright (C) 1999, 2001, 2002, 2003, 2006, 2010 Free Software > +# Foundation, Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -26,61 +27,48 @@ AC_PROG_CC > AC_PROG_YACC > END > > +# Run it here once and for all, since we are not going to modify > +# configure.in anymore. > +$ACLOCAL > + > cat > Makefile.am <<'END' > bin_PROGRAMS = zardoz > zardoz_SOURCES = zardoz.y > END > > # Don't redefine several times the same variable. > -cp Makefile.am Makefile.src > +cp -f Makefile.am Makefile.src Why should you need this change? Generally, I don't see why you ever need 'cp -f'. Please undo. (several instances) > > -$ACLOCAL > $AUTOMAKE -a > - > # If zardoz.h IS mentioned, fail > -grep 'zardoz.h' Makefile.in > /dev/null && Exit 1 > - > +$FGREP 'zardoz.h' Makefile.in && Exit 1 > > - > -cp Makefile.src Makefile.am > +cp -f Makefile.src Makefile.am > echo 'AM_YFLAGS = -d' >> Makefile.am > - > $AUTOMAKE > - > # If zardoz.h is NOT mentioned, fail > -grep 'zardoz.h' Makefile.in > /dev/null > - > - > +$FGREP 'zardoz.h' Makefile.in > > -cp Makefile.src Makefile.am > +cp -f Makefile.src Makefile.am > echo 'AM_YFLAGS = ' >> Makefile.am > - > $AUTOMAKE > - > # If zardoz.h IS mentioned, fail > -grep 'zardoz.h' Makefile.in > /dev/null && Exit 1 > - > +$FGREP 'zardoz.h' Makefile.in && Exit 1 > > - > -cp Makefile.src Makefile.am > +cp -f Makefile.src Makefile.am > echo 'YFLAGS = -d' >> Makefile.am > - > -# YFLAGS is a use variable. > +# YFLAGS is a user variable. > AUTOMAKE_fails > -grep 'YFLAGS' stderr > +grep 'YFLAGS.* user variable' stderr > +grep 'AM_YFLAGS.* instead' stderr > $AUTOMAKE -Wno-gnu > - > # If zardoz.h is NOT mentioned, fail > -grep 'zardoz.h' Makefile.in > /dev/null > - > - > +$FGREP 'zardoz.h' Makefile.in > > -cp Makefile.src Makefile.am > +cp -f Makefile.src Makefile.am > echo 'YFLAGS = ' >> Makefile.am > - > $AUTOMAKE -Wno-gnu > - > # If zardoz.h IS mentioned, fail > -grep 'zardoz.h' Makefile.in > /dev/null && Exit 1 > +$FGREP 'zardoz.h' Makefile.in && Exit 1 > > : > --- a/tests/yacc3.test > +++ b/tests/yacc3.test > @@ -30,24 +31,22 @@ cat > Makefile.am <<'END' > AUTOMAKE_OPTIONS = no-dependencies > bin_PROGRAMS = zardoz > zardoz_SOURCES = zardoz.y > -magic: > - @echo $(DIST_COMMON) > END > > $ACLOCAL > $AUTOMAKE -a > > -$FGREP -v @SET_MAKE@ Makefile.in > Makefile.sed > # It should not be disted here > -grep 'zardoz.h' Makefile.in && Exit 1 > +$FGREP 'zardoz.h' Makefile.in && Exit 1 > > -cp Makefile.am Save > +cp -f Makefile.am Makefile.sav > # Test all available flags to make sure header is distributed with > # `-d'. > for flag in YFLAGS AM_YFLAGS zardoz_YFLAGS; do > - cp Save Makefile.am > + cp -f Makefile.sav Makefile.am > echo "$flag = -d" >> Makefile.am > - > $AUTOMAKE -Wno-gnu > - grep 'zardoz.h' Makefile.in > + $FGREP 'zardoz.h' Makefile.in > done > + > +: > diff --git a/tests/yacc4.test b/tests/yacc4.test > index 0435ec0..bb25290 100755 > --- a/tests/yacc4.test > +++ b/tests/yacc4.test > @@ -1,5 +1,5 @@ > #! /bin/sh > -# Copyright (C) 2001, 2002 Free Software Foundation, Inc. > +# Copyright (C) 2001, 2002, 2010 Free Software Foundation, Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -19,15 +19,12 @@ > required='bison gcc' > . ./defs || Exit 1 > > -cat > configure.in << 'END' > -AC_INIT > -AC_CONFIG_AUX_DIR([.]) > -AM_INIT_AUTOMAKE(foo, 0.1) > -PACKAGE=foo > -VERSION=0.1 > +set -e > + > +cat >> configure.in << 'END' > AC_PROG_CC > AC_PROG_YACC > -AC_OUTPUT(Makefile) > +AC_OUTPUT > END > > cat > Makefile.am << 'END' > @@ -60,8 +57,6 @@ cat > foo.c << 'END' > int main () { return 0; } > END > > -set -e > - > $ACLOCAL > $AUTOCONF > $AUTOMAKE -a > @@ -86,6 +81,5 @@ test -f parse.c > $MAKE maintainer-clean > test ! -f bar.c > test ! -f parse.c > -: > > -Exit 0 > +: > diff --git a/tests/yacc5.test b/tests/yacc5.test > index 8b00ed8..77de2e9 100755 > --- a/tests/yacc5.test > +++ b/tests/yacc5.test > @@ -1,5 +1,5 @@ > #! /bin/sh > -# Copyright (C) 2001, 2002, 2003 Free Software Foundation, Inc. > +# Copyright (C) 2001, 2002, 2003, 2010 Free Software Foundation, Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -21,13 +21,10 @@ > > set -e > > -cat > configure.in << 'END' > -AC_INIT > -AM_INIT_AUTOMAKE(nonesuch, nonesuch) > +cat >> configure.in << 'END' > AC_PROG_CC > AM_PROG_CC_C_O > AC_PROG_YACC > -AC_OUTPUT(Makefile) > END > > cat > Makefile.am << 'END' > @@ -69,8 +66,8 @@ maude_SOURCES = sub/maude.y > maude_YFLAGS = -d > END > > -$ACLOCAL || Exit 1 > -$AUTOMAKE -a || Exit 1 > +$ACLOCAL > +$AUTOMAKE -a > > # Rule should use maude_YFLAGS. > grep 'AM_YFLAGS.*maude' Makefile.in && Exit 1 > @@ -80,3 +77,5 @@ grep 'maudec' Makefile.in && Exit 1 > > # Make sure the .o file is required. > grep '^am_maude_OBJECTS.*maude' Makefile.in > + > +: > diff --git a/tests/yacc6.test b/tests/yacc6.test > index f10effd..b9b259b 100755 > --- a/tests/yacc6.test > +++ b/tests/yacc6.test > @@ -1,5 +1,5 @@ > #! /bin/sh > -# Copyright (C) 2001, 2002, 2003, 2004, 2006, 2007 Free Software > +# Copyright (C) 2001, 2002, 2003, 2004, 2006, 2007, 2010 Free Software > # Foundation, Inc. > # > # This program is free software; you can redistribute it and/or modify > @@ -27,13 +27,13 @@ set -e > > cat > configure.in << 'END' > AC_INIT([yacc6], [1.0]) > +# `aux' is not an acceptable file/directory name on Windows systems > AC_CONFIG_AUX_DIR([aux1]) > AM_INIT_AUTOMAKE > -AC_CONFIG_FILES([Makefile]) > +AC_CONFIG_FILES([Makefile sub/Makefile]) > AC_PROG_CC > AM_PROG_CC_C_O > AC_PROG_YACC > -AC_CONFIG_FILES([sub/Makefile]) > AC_OUTPUT > END > > @@ -89,11 +89,11 @@ $AUTOMAKE -a > test -f aux1/ylwrap > test ! -f ylwrap > test ! -f sub/ylwrap > -$FGREP '(top_srcdir)/aux1/ylwrap' sub/Makefile.in > +$FGREP '$(top_srcdir)/aux1/ylwrap' sub/Makefile.in > ./configure > $MAKE > -grep '#.*line.*foo.y' sub/foo.c > -grep '#.*line.*bar.y' sub/bar.c > +grep '#.*line.*foo\.y' sub/foo.c > +grep '#.*line.*bar\.y' sub/bar.c > > $sleep > : > z > @@ -106,3 +106,5 @@ sed s/TOKEN/TEKON/g sub/bar.y >sub/bar.yt > mv -f sub/bar.yt sub/bar.y > $MAKE > $MAKE test-time-changed > + > +: > diff --git a/tests/yacc7.test b/tests/yacc7.test > index 2edd15c..2b866bc 100755 > --- a/tests/yacc7.test > +++ b/tests/yacc7.test > @@ -1,5 +1,6 @@ > #! /bin/sh > -# Copyright (C) 2001, 2002, 2003, 2004 Free Software Foundation, Inc. > +# Copyright (C) 2001, 2002, 2003, 2004, 2010 Free Software Foundation, > +# Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -23,6 +24,8 @@ > required=bison > . ./defs || Exit 1 > > +set -e > + > cat >> configure.in << 'END' > AC_PROG_CC > AC_PROG_YACC > @@ -52,8 +55,6 @@ WORD: "up"; > %% > END > > -set -e > - > $ACLOCAL > $AUTOMAKE -a > $AUTOCONF > @@ -83,8 +84,9 @@ $MAKE distclean > test -f foo.h > test -f foo.c > # ... but maintainer-clean should. > -./configure > +./configure # we must re-create `Makefile' > $MAKE maintainer-clean > test ! -f foo.h > test ! -f foo.c > + > : > diff --git a/tests/yacc8.test b/tests/yacc8.test > index 979415c..797f4e7 100755 > --- a/tests/yacc8.test > +++ b/tests/yacc8.test > @@ -1,5 +1,6 @@ > #! /bin/sh > -# Copyright (C) 2002, 2003, 2004, 2006 Free Software Foundation, Inc. > +# Copyright (C) 2002, 2003, 2004, 2006, 2010 Free Software Foundation, > +# Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -22,11 +23,7 @@ required="gcc bison" > > set -e > > -cat > configure.in << 'END' > -AC_INIT([yacc8], [1.0]) > -AC_CONFIG_AUX_DIR([.]) > -AM_INIT_AUTOMAKE > -AC_CONFIG_FILES([Makefile]) > +cat >> configure.in << 'END' > AC_PROG_CC > AM_PROG_CC_C_O > AC_PROG_YACC > @@ -39,15 +36,16 @@ bin_PROGRAMS = foo/foo > foo_foo_SOURCES = foo/parse.y > AM_YFLAGS = -d > > +.PHONY: obj > obj: foo/parse.$(OBJEXT) > > -test1: obj > - test -f foo/parse.c > - test -f foo/parse.$(OBJEXT) > - > -test2: obj > +.PHONY: test1 test2 > +test1: foo/parse.$(OBJEXT) > test -f foo/parse.c > test -f foo/parse.$(OBJEXT) > +test2: foo/parse2.$(OBJEXT) > + test -f foo/parse2.c > + test -f foo/parse2.$(OBJEXT) > END > > mkdir foo > @@ -104,9 +102,11 @@ EXTRA_foo_foo_SOURCES = foo/parse2.y > END > > $AUTOMAKE -a > -test -f ./ylwrap || Exit 1 > +test -f ./ylwrap > > cd sub > # Regenerate Makefile (automatic in GNU Make, but not in other Makes) > ./config.status > $MAKE test2 Thanks, Ralf