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