On Thu, Jun 14, 2001 at 05:49:35PM -0500, Raja R Harinath wrote:
> So, I think it's cleaner to do:
> 
>   1. conditions_true_when(\@@) -- takes a list of conditions and one
>      or more "whens", and returns true when all conditions are true
>      for all whens.
> 
>   2. redundant_condition($@) -- check if the given condition
>      subsumed by any of the whens.
> 
>   3. change 'variable_conditions_reduce' to use 'redundant_condition',
>      along with your modification to look at the tail of @conds too.

Yes, that's much better.

Actually, the main reason I felt it reasonable to change the semantic of
conditionals_true_when() was because of the FIXME: the semantics weren't very
well defined to start with.  Looking at the two uses of
conditionals_true_when() outside variable_conditions_reduce(), it is
always called with an array as the first argument, and a list containing a
single scalar as the second argument.  It therefore seems wise to
change conditionals_true_when to accept only a single WHEN, and to follow
your other suggestions.

The attached patch does this.  All tests behaved as expected with this
patch.

-- 
Richard
Index: ChangeLog
===================================================================
RCS file: /cvs/automake/automake/ChangeLog,v
retrieving revision 1.1419
diff -u -r1.1419 ChangeLog
--- ChangeLog   2001/06/12 14:37:44     1.1419
+++ ChangeLog   2001/06/15 10:14:07
@@ -1,3 +1,21 @@
+2001-06-06  Richard Boulton  <[EMAIL PROTECTED]>
+
+       * (conditionals_true_when): Pass first parameters by reference,
+       avoiding bug which put all parameters in @CONDS instead of @WHENS.
+       Report by Kalle Olavi Niemitalo.
+       Take a single WHEN instead of an array of WHENS.
+       Remove FIXME; can't now have an empty @WHENS.
+       * (conditional_is_redundant): New sub.
+       * (variable_conditions_reduce): Check whether each condition is
+       implied by any of the other conditions (other those already
+       discarded), rather than checking only against those already
+       considered (and kept).  Also, fix sense of check: was keeping
+       tautologous terms instead of discarding them.  Use
+       conditional_is_redundant instead of conditionals_true_when.
+       * tests/Makefile.am (TESTS): Added cond11.test and cond12.test.
+       * tests/cond11.test: New file.
+       * tests/cond12.test: New file.
+
 2001-06-12  Tom Tromey  <[EMAIL PROTECTED]>
 
        * automake.texi (ANSI): Minor clarification.
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1137
diff -u -r1.1137 automake.in
--- automake.in 2001/06/09 00:34:28     1.1137
+++ automake.in 2001/06/15 10:14:22
@@ -5315,32 +5315,49 @@
 
 
 # $BOOLEAN
-# &conditionals_true_when (@CONDS, @WHENS)
+# &conditionals_true_when (\@CONDS, $WHEN)
 # ----------------------------------------
-# Same as above, but true if all the @CONDS are true when *ALL*
-# the @WHENS are sufficient.
+# Same as above, but true only if all the @CONDS are true when $WHEN is true.
 #
-# If there are no @CONDS, then return true, of course. *BUT*, even if there
-# are @CONDS but @WHENS is empty, return true.  This is counter intuitive,
-# and against all the rules of logic, but is needed by the current code.
-# FIXME: Do something saner when the logic of conditionals is understood.
-sub conditionals_true_when (@@)
+# If there are no @CONDS, then return true.
+sub conditionals_true_when (\@$)
 {
-    my (@conds, @whens) = @_;
+    my ($condsref, $when) = @_;
 
-    foreach my $cond (@conds)
+    foreach my $cond (@$condsref)
     {
-      foreach my $when (@whens)
-      {
-       return 0
-         unless conditional_true_when ($cond, $when);
-      }
+       return 0 unless conditional_true_when ($cond, $when);
     }
 
     return 1;
 }
 
+# $BOOLEAN
+# &conditional_is_redundant ($COND, @WHENS)
+# ----------------------------------------
+# Determine whether $COND is redundant with respect to @WHENS.
+#
+# Returns true if $COND is true for any of the conditions in @WHENS.
+#
+# If there are no @WHENS, then behave as if @WHENS contained a single empty
+# condition.
+sub conditional_is_redundant ($@)
+{
+    my ($cond, @whens) = @_;
 
+    if (@whens == 0) {
+       return 1 if conditional_true_when($cond, "");
+    } else {
+       foreach my $when (@whens)
+       {
+           return 1 if conditional_true_when ($cond, $when);
+       }
+    }
+
+    return 0;
+}
+
+
 # $NEGATION
 # condition_negate ($COND)
 # ------------------------
@@ -5845,7 +5862,7 @@
        # If this condition cannot be true when the parent conditions
        # are true, then skip it.
        next
-         if ! conditionals_true_when ((@parent_conds), ($vcond));
+         if ! conditionals_true_when (@parent_conds, $vcond);
 
        push (@this_conds, $vcond);
 
@@ -5918,7 +5935,7 @@
            next if ! $ok;
 
            next
-             if ! conditionals_true_when ((@parent_conds), ($perm));
+             if ! conditionals_true_when (@parent_conds, $perm);
 
            # This permutation was not already handled, and is valid
            # for the parents.
@@ -5933,23 +5950,28 @@
 # Filter a list of conditionals so that only the exclusive ones are
 # retained.  For example, if both `COND1_TRUE COND2_TRUE' and
 # `COND1_TRUE' are in the list, discard the latter.
+# If the list is empty, return TRUE
 sub variable_conditions_reduce
 {
     my (@conds) = @_;
     my @ret = ();
-    foreach my $cond (@conds)
+    my $cond;
+    while(@conds > 0)
     {
+       $cond = shift(@conds);
+
         # FALSE is absorbent.
         if ($cond eq 'FALSE')
          {
            return ('FALSE');
          }
-       elsif (conditionals_true_when (($cond), (@ret)))
+       elsif (!conditional_is_redundant ($cond, @ret, @conds))
          {
            push (@ret, $cond);
          }
     }
 
+    return "TRUE" if @ret == 0;
     return @ret;
 }
 
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.307
diff -u -r1.307 Makefile.am
--- Makefile.am 2001/06/09 00:34:29     1.307
+++ Makefile.am 2001/06/15 10:14:22
@@ -60,6 +60,8 @@
 cond8.test \
 cond9.test \
 cond10.test \
+cond11.test \
+cond12.test \
 condincl.test \
 condincl2.test \
 condlib.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.406
diff -u -r1.406 Makefile.in
--- Makefile.in 2001/06/09 00:34:30     1.406
+++ Makefile.in 2001/06/15 10:14:23
@@ -126,6 +126,8 @@
 cond8.test \
 cond9.test \
 cond10.test \
+cond11.test \
+cond12.test \
 condincl.test \
 condincl2.test \
 condlib.test \
Index: tests/cond11.test
===================================================================
RCS file: cond11.test
diff -N cond11.test
--- /dev/null   Tue May  5 13:32:27 1998
+++ cond11.test Fri Jun 15 03:14:23 2001
@@ -0,0 +1,41 @@
+#! /bin/sh
+
+# Test for bug in conditionals.  From Richard Boulton.
+# This checks that, if LDADD is set from a conditional variable
+# and an AC_SUBST, the _DEPENDENCIES variable is set correctly.
+
+. $srcdir/defs || exit 1
+
+cat > configure.in << 'END'
+AC_INIT(Makefile.am)
+AM_INIT_AUTOMAKE(foo,0.0)
+AC_PROG_CC
+AM_CONDITIONAL(USE_A,[test x = x])
+AC_OUTPUT(Makefile)
+AC_SUBST(SUBSTVAR)
+END
+
+cat > Makefile.am << 'END'
+
+if USE_A
+foolibs=faz.la
+else
+foolibs=
+endif
+
+noinst_PROGRAMS = foo
+foo_SOURCES = foo.c
+LDADD = $(SUBSTVAR) $(foolibs)
+END
+
+: > config.guess
+: > config.sub
+: > compile
+
+$ACLOCAL || exit 1
+$AUTOMAKE || exit 1
+
+#Should be two dependency setting lines
+count=`grep 'foo_DEPENDENCIES =' Makefile.in | wc -l|sed 's/ //g'`
+test "x$count" == "x2" &&
+  grep '^.USE_A_TRUE.foo_DEPENDENCIES =' Makefile.in
Index: tests/cond12.test
===================================================================
RCS file: cond12.test
diff -N cond12.test
--- /dev/null   Tue May  5 13:32:27 1998
+++ cond12.test Fri Jun 15 03:14:23 2001
@@ -0,0 +1,80 @@
+#! /bin/sh
+
+# Test behaviour of variable_conditions_reduce()
+# This checks the result of variable_conditions_reduce() for a wide variety
+# of cases.
+
+. $srcdir/defs || exit 1
+
+head -n 1 $srcdir/../automake >>automake_tmp
+cat << 'END' >> automake_tmp
+my $failed = 0;
+sub check_reduce($$) {
+ my ($inref, $outref) = @_;
+ my @result = sort &Automake::variable_conditions_reduce(@$inref);
+ my $correct = 1;
+ $correct = 0 if (join(",", @result) ne join(",", @$outref));
+ 
+ if (! $correct) {
+   print '"'.join(",", @$inref) . '" => "' .
+        join(",", @result) . '" expected "' .
+        join(",", @$outref) . '"' . "\n";
+   $failed = 1;
+ }
+}
+
+# If no conditions are given, TRUE should be returned
+check_reduce([""], ["TRUE"]);
+# A single condition should be passed through unchanged
+check_reduce(["FOO"], ["FOO"]);
+check_reduce(["FALSE"], ["FALSE"]);
+check_reduce(["TRUE"], ["TRUE"]);
+
+# TRUE and false should be discarded and overwhelm the result, respectively
+check_reduce(["FOO", "TRUE"], ["FOO"]);
+check_reduce(["FOO", "FALSE"], ["FALSE"]);
+
+# Repetitions should be removed
+check_reduce(["FOO", "FOO"], ["FOO"]);
+check_reduce(["TRUE", "FOO", "FOO"], ["FOO"]);
+check_reduce(["FOO", "TRUE", "FOO"], ["FOO"]);
+check_reduce(["FOO", "FOO", "TRUE"], ["FOO"]);
+
+# Two different conditions should be preserved, but TRUEs should be removed
+check_reduce(["FOO", "BAR"], ["BAR,FOO"]);
+check_reduce(["TRUE", "FOO", "BAR"], ["BAR,FOO"]);
+check_reduce(["FOO", "TRUE", "BAR"], ["BAR,FOO"]);
+check_reduce(["FOO", "BAR", "TRUE"], ["BAR,FOO"]);
+
+# A condition implied by another condition should be removed.
+check_reduce(["FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["BAR", "FOO BAR"], ["FOO BAR"]);
+check_reduce(["TRUE", "FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "TRUE", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "BAR", "TRUE"], ["FOO BAR"]);
+
+check_reduce(["BAR FOO", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR", "BAR FOO"], ["BAR FOO"]);
+check_reduce(["TRUE", "BAR FOO", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR FOO", "TRUE", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR FOO", "BAR", "TRUE"], ["BAR FOO"]);
+
+# Check that reduction happens even when there are two conditionals to remove.
+check_reduce(["FOO", "FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO", "FOO BAR", "BAZ", "FOO BAZ"], ["FOO BAR", "FOO BAZ"]);
+check_reduce(["FOO", "FOO BAR", "BAZ", "FOO BAZ", "FOO BAZ BAR"], ["FOO BAZ BAR"]);
+
+# Duplicated condionals should be removed
+check_reduce(["FOO", "BAR", "BAR"], ["BAR,FOO"]);
+
+# Equivalent conditionals in different forms should be reduced: which one is
+# left is unfortunately order dependent.
+check_reduce(["BAR FOO", "FOO BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "BAR FOO"], ["BAR FOO"]);
+
+exit $failed;
+END
+cat $srcdir/../automake >>automake_tmp
+chmod +x automake_tmp
+
+./automake_tmp

Reply via email to