bug#7804: [PATCH] yacc: warn about conditional content in *YFLAGS variables (was: bug#7804: Automake does not warn if AM_YFLAGS is conditionally extended)

[Stefano Lattarini]

On Friday 07 January 2011, Stefano Lattarini wrote:
> Hello automakers.
> 
> Due to current implementation details, when dealing with Yacc sources, 
> automake
> must know the contents of the `$(AM_YFLAGS)' variable (or similar 
> `$(foo_YFLAGS)'
> variables) *statically and unconditionally* in order to always generate proper
> code.
> 
> This is due to the special handling of the `-d' yacc flag; on this issue, read
> at <http://www.gnu.org/software/automake/manual/html_node/Yacc-and-Lex.html>:
>  ``AM_YFLAGS is usually used to pass the -d option to yacc. Automake knows 
> what
>    this means and will automatically adjust its rules to update and distribute
>    the header file built by 'yacc -d'. ''
> 
> And while automake correctly warns if AM_YFLAGS is conditionally *defined*:
> 
>   $ cat configure.ac
>   AC_INIT(x,0)
>   AM_INIT_AUTOMAKE(foreign)
>   AC_PROG_CC
>   AC_PROG_YACC
>   AC_CONFIG_FILES(Makefile)
>   AM_CONDITIONAL(COND,:)
>   $ aclocal
>   $ cat > Makefile.am <<'END'
>   bin_PROGRAMS = foo bar
>   foo_SOURCES = foo.y
>   bar_SOURCES = bar.y
>   if COND
>   AM_YFLAGS = -d
>   endif
>   END
>   $ automake -a -Werror; echo status=$?
>   Makefile.am:5: automake does not support AM_YFLAGS being defined 
> conditionally
>   status=1
> 
> it erronously doesn't warn if AM_YFLAGS is conditionally *extended*:
> 
>   $ cat configure.ac
>   AC_INIT(x,0)
>   AM_INIT_AUTOMAKE(foreign)
>   AC_PROG_CC
>   AC_PROG_YACC
>   AC_CONFIG_FILES(Makefile)
>   AM_CONDITIONAL(COND,:)
>   $ aclocal
>   $ cat > Makefile.am <<'END'
>   bin_PROGRAMS = foo bar
>   foo_SOURCES = foo.y
>   bar_SOURCES = bar.y
>   AM_YFLAGS =
>   if COND
>   AM_YFLAGS += -d
>   endif
>   END
>   $ automake -a -Werror; echo status=$?
>   status=0
> 
> I think this bug shouldn't be difficult to fix, and I'll attempt a fix
> soonish; but as usual, having it in the bug tracker doesn't hurt IMHO.
> 
> Regards,
>   Stefano
> 
The attached patch should fix the bug.  OK for the temporary branch
'yacc-work', to be merged to master afterwards?

Regards,
  Stefano

From 2ffb8db20b74a319e63fe2d8eda12a075117272b Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Mon, 10 Jan 2011 15:50:35 +0100
Subject: [PATCH] yacc: warn about conditional content in *YFLAGS variables

This change fixes automake bug#7804.

* automake.in (lang_yacc_target_hook): Warn if any of the relevant
*YFLAGS variables has conditional contents (not only a conditional
definition).  Related refactoring.
* tests/yflags-conditional.test: Updated and extended.
---
 ChangeLog                     |    9 +++
 automake.in                   |   34 ++++++++----
 tests/yflags-conditional.test |  117 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 140 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 06f9b84..e40f608 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-01-10   Stefano Lattarini  <address@hidden>
+
+       yacc: warn about conditional content in *YFLAGS variables
+       This change fixes automake bug#7804.
+       * automake.in (lang_yacc_target_hook): Warn if any of the relevant
+       *YFLAGS variables has conditional contents (not only a conditional
+       definition).  Related refactoring.
+       * tests/yflags-conditional.test: Updated and extended.
+
 2011-01-08   Stefano Lattarini  <address@hidden>
 
        yacc: support variable expansions in *YFLAGS definition.
diff --git a/automake.in b/automake.in
index 2bffe48..fa458d6 100755
--- a/automake.in
+++ b/automake.in
@@ -6061,15 +6061,29 @@ sub lang_yacc_target_hook
 {
     my ($self, $aggregate, $output, $input, %transform) = @_;
 
-    my $flagvar = var ($aggregate . "_YFLAGS");
-    my $YFLAGSvar = var ('YFLAGS');
-    # We cannot work reliably with conditionally-defined YFLAGS.
-    $flagvar->check_defined_unconditionally if $flagvar;
-    $YFLAGSvar->check_defined_unconditionally if $YFLAGSvar;
-    my @flags = $flagvar ? $flagvar->value_as_list_recursive : ();
-    my @YFLAGS = $YFLAGSvar ? $YFLAGSvar->value_as_list_recursive : ();
-    if (grep (/^-d$/, @flags) || grep (/^-d$/, @YFLAGS))
-    {
+    # If some relevant *YFLAGS variable contains the `-d' flag, we'll
+    # have to to generate special code.
+    my $yflags_contains_minus_d = 0;
+
+    foreach my $pfx ("", "${aggregate}_")
+      {
+       my $yflagsvar = var ("${pfx}YFLAGS");
+       next unless $yflagsvar;
+       # We cannot work reliably with conditionally-defined YFLAGS.
+       if ($yflagsvar->has_conditional_contents)
+         {
+           msg_var ('unsupported', $yflagsvar,
+                    "`${pfx}YFLAGS' cannot have conditional contents");
+         }
+       else
+         {
+           $yflags_contains_minus_d = 1
+             if grep (/^-d$/, $yflagsvar->value_as_list_recursive);
+         }
+      }
+
+    if ($yflags_contains_minus_d)
+      {
        (my $output_base = $output) =~ s/$KNOWN_EXTENSIONS_PATTERN$//;
        my $header = $output_base . '.h';
 
@@ -6098,7 +6112,7 @@ sub lang_yacc_target_hook
        # then we want to remove them with "make clean"; otherwise,
        # "make distcheck" will fail.
        $clean_files{$header} = $transform{'DIST_SOURCE'} ? MAINTAINER_CLEAN : 
CLEAN;
-    }
+      }
     # See the comment above for $HEADER.
     $clean_files{$output} = $transform{'DIST_SOURCE'} ? MAINTAINER_CLEAN : 
CLEAN;
 }
diff --git a/tests/yflags-conditional.test b/tests/yflags-conditional.test
index 8c673b1..91e3da4 100755
--- a/tests/yflags-conditional.test
+++ b/tests/yflags-conditional.test
@@ -14,7 +14,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Check that automake complains about conditionally-defined *_YFLAGS.
+# Check that automake complains about *_YFLAGS variables which have
+# conditional content.
 
 . ./defs || Exit 1
 
@@ -22,25 +23,121 @@ set -e
 
 cat >> configure.in <<'END'
 AC_PROG_CC
-AC_PROG_YACC
+
+# `YFLAGS' is AC_SUBST'd by AC_PROG_YACC by default, but we
+# don't want this, since it might confuse our error messages.
+# Also, AM_SUBST_NOTMAKE seems not to help about this.
+# So we simply define $(YACC) by hand.
+AC_SUBST([YACC], [yacc])
+
 AM_CONDITIONAL([COND], [:])
 END
 
+$ACLOCAL
+
 cat > Makefile.am <<'END'
-bin_PROGRAMS = foo bar
+bin_PROGRAMS = foo zardoz
 foo_SOURCES = foo.y
-bar_SOURCES = bar.y
+zardoz_SOURCES = zardoz.y
 if COND
-AM_YFLAGS = $(YFLAGS)
-bar_YFLAGS = -v
+AM_YFLAGS = -v
+zardoz_YFLAGS = -v
 endif COND
 END
 
+cat > Makefile1.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+## dummy comment to keep line count right
+if COND
+YFLAGS = foo
+endif COND
+END
+
+cat > Makefile2.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+AM_YFLAGS = am_yflags
+if COND
+YFLAGS = yflags
+endif COND
+END
+
+cat > Makefile3.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+foo_YFLAGS = foo_yflags
+if COND
+YFLAGS = yflags
+endif COND
+END
+
+cat > Makefile4.am <<'END'
+bin_PROGRAMS = foo zardoz
+
+foo_SOURCES = foo.y
+zardoz_SOURCES = $(foo_SOURCES)
+
+YFLAGS =
+AM_YFLAGS = $(COND_VAR1)
+zardoz_YFLAGS = $(COND_VAR2:z=r)
+
+COND_VAR2 = foo
+if COND
+YFLAGS += -v
+COND_VAR2 += bar
+else !COND
+COND_VAR1 = -d
+endif !COND
+END
+
+cat > Makefile5.am <<'END'
+bin_PROGRAMS = foo zardoz
+foo_SOURCES = foo.y
+zardoz_SOURCES = zardoz.y
+YFLAGS = -v
+AM_YFLAGS = -v
+if COND
+zardoz_YFLAGS = -v
+endif
+END
+
+cat > Makefile6.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+foo_YFLAGS = -v
+if COND
+quux_YFLAGS = -v
+AM_YFLAGS = -v
+endif
+END
+
 : > ylwrap
 
-$ACLOCAL
-AUTOMAKE_fails
-grep "Makefile\.am:5:.*AM_YFLAGS.* defined conditionally" stderr
-grep "Makefile\.am:6:.*bar_YFLAGS.* defined conditionally" stderr
+LC_ALL=C; export LC_ALL  # For grep regexes below.
+
+AUTOMAKE_fails -Wnone -Wunsupported Makefile
+grep '^Makefile\.am:5:.*AM_YFLAGS.* conditional contents' stderr
+grep '^Makefile\.am:6:.*zardoz_YFLAGS.* conditional contents' stderr
+
+for i in 1 2 3; do
+  AUTOMAKE_fails -Wnone -Wunsupported Makefile$i
+  grep "^Makefile$i\\.am:5:.*[^a-zA-Z0-9_]YFLAGS.* conditional contents" \
+       stderr
+done
+
+AUTOMAKE_fails -Wnone -Wunsupported Makefile4
+grep '^Makefile4\.am:6:.*[^a-zA-Z0-9_]YFLAGS.* conditional contents' stderr
+grep '^Makefile4\.am:7:.*AM_YFLAGS.* conditional contents' stderr
+grep '^Makefile4\.am:8:.*zardoz_YFLAGS.* conditional contents' stderr
+
+# Now let's check we avoid false positives.
+
+# Disable `gnu' warnings because we override the user variable `YFLAGS'.
+AUTOMAKE_fails -Wno-gnu Makefile5
+grep -v '^Makefile5\.am:.*zardoz_YFLAGS' stderr | grep . && Exit 1
+
+# Disable `gnu' warnings because we override the user variable `YFLAGS'.
+$AUTOMAKE -Wno-gnu Makefile6
 
 :
-- 
1.7.2.3