autoconf-patches
[Top][All Lists]
Advanced

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

Re: O(n) patches and side effects


From: Eric Blake
Subject: Re: O(n) patches and side effects
Date: Fri, 22 Aug 2008 04:23:38 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Eric Blake <ebb9 <at> byu.net> writes:

> 
> Hmm, I guess that means I should beef up the testsuite, as well as
> patching these misfits to guarantee exactly one expansion of side effects.
> Then the manual should indeed call this out as a design rule of thumb when
> working with quoted lists - all side effects in the list are expanded
> once, up front, before the list elements are visited.  Thanks for the report!

Done as follows.  Meanwhile, I discovered a slight flaw in m4_init that I'm not 
sure how to fix yet - if you freeze m4sugar.m4f with m4 1.6, but then reload it 
with 1.4.x, then m4_init sees the __m4_version__ from the frozen file and 
proceeds to use the quadratic methods, rather than realizing that foreach.m4 is 
needed.  On the other hand, why would people build with newer m4 then downgrade 
to an older m4?  So I'm not even sure whether this scenario needs to work.

I could always fix it on the m4 side of things, by making m4 1.6 use version2 
frozen files instead of version1, so that m4 1.4.x refuses to load files frozen 
by 1.6.  In fact, I want to do that for other reasons (the ability to freeze 
which macros are being traced, for instance).  But it would be nice to fix it 
on the autoconf side of things.

From: Eric Blake <address@hidden>
Date: Thu, 21 Aug 2008 22:17:33 -0600
Subject: [PATCH] Avoid extra side effects in m4sugar list expansion.

* lib/m4sugar/m4sugar.m4 (m4_mapall_sep, m4_list_cmp): Wrap
around...
(_m4_mapall_sep, _m4_list_cmp_raw): ...new helpers, to avoid
duplicate side effects.
(m4_version_compare): Adjust caller.
* lib/m4sugar/foreach.m4 (m4_list_cmp): Rename...
(_m4_list_cmp_raw): ...to match m4sugar.
* doc/autoconf.texi (Looping constructs): Document the behavior of
side effects.
* tests/m4sugar.at (M4 loops, m4@&address@hidden, m4@&address@hidden):
Ensure only one side effect.
(recursion): Fix test typo.
Reported by Ralf Wildenhues.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |   17 +++++++++++++++++
 doc/autoconf.texi      |   16 ++++++++++++++++
 lib/m4sugar/foreach.m4 |    8 +++++---
 lib/m4sugar/m4sugar.m4 |   22 ++++++++++++++++------
 tests/m4sugar.at       |   46 +++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7db4f57..99a0e05 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2008-08-21  Eric Blake  <address@hidden>
+
+       Avoid extra side effects in m4sugar list expansion.
+       * lib/m4sugar/m4sugar.m4 (m4_mapall_sep, m4_list_cmp): Wrap
+       around...
+       (_m4_mapall_sep, _m4_list_cmp_raw): ...new helpers, to avoid
+       duplicate side effects.
+       (m4_version_compare): Adjust caller.
+       * lib/m4sugar/foreach.m4 (m4_list_cmp): Rename...
+       (_m4_list_cmp_raw): ...to match m4sugar.
+       * doc/autoconf.texi (Looping constructs): Document the behavior of
+       side effects.
+       * tests/m4sugar.at (M4 loops, m4@&address@hidden, m4@&address@hidden):
+       Ensure only one side effect.
+       (recursion): Fix test typo.
+       Reported by Ralf Wildenhues.
+
 2008-08-21  Ralf Wildenhues  <address@hidden>
 
        * TODO: Add item for compiler default flags.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 5742f7e..75c110e 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -10756,6 +10756,22 @@ Looping constructs
 not likely to be macro names, as in @samp{[fputc_unlocked,
 fgetc_unlocked]}.
 
+Although not generally recommended, it is possible for quoted lists to
+have side effects; all side effects are expanded only once, and prior to
+visiting any list element.  On the other hand, the fact that unquoted
+macros are expanded exactly once means that macros without side effects
+can be used to generate lists.  For example,
+
address@hidden
+m4_foreach([i], [[1], [2], [3]m4_errprintn([hi])], [i])
address@hidden
address@hidden
+m4_define([list], [[1], [2], [3]])
address@hidden
+m4_foreach([i], [list], [i])
address@hidden
address@hidden example
+
 @defmac m4_car (@var{list})
 @msindex{car}
 Expands to the quoted first element of the comma-separated quoted
diff --git a/lib/m4sugar/foreach.m4 b/lib/m4sugar/foreach.m4
index 91a9643..c35d5d3 100644
--- a/lib/m4sugar/foreach.m4
+++ b/lib/m4sugar/foreach.m4
@@ -338,6 +338,8 @@ m4_define([m4_joinall],
 # -----------------
 # Compare the two lists of integer expressions A and B.
 #
+# m4_list_cmp takes care of any side effects; we only override
+# _m4_list_cmp_raw, where we can safely expand lists multiple times.
 # First, insert padding so that both lists are the same length; the
 # trailing +0 is necessary to handle a missing list.  Next, create a
 # temporary macro to perform pairwise comparisons until an inequality
@@ -346,9 +348,9 @@ m4_define([m4_joinall],
 #         m4_eval([($2) != ($4)]), [1], [m4_cmp([$2], [$4])],
 #         [0]_m4_popdef([_m4_cmp], [_m4_size]))
 # then calls _m4_cmp([1+0], [0], [1], [2+0])
-m4_define([m4_list_cmp],
-[m4_if([$1], [$2], 0,
-  [m4_pushdef([_m4_size])_$0($1+0_m4_list_pad(m4_count($1), m4_count($2)),
+m4_define([_m4_list_cmp_raw],
+[m4_if([$1], [$2], 0, [m4_pushdef(
+     [_m4_size])_m4_list_cmp($1+0_m4_list_pad(m4_count($1), m4_count($2)),
                             $2+0_m4_list_pad(m4_count($2), m4_count($1)))])])
 
 m4_define([_m4_list_pad],
diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index b337370..f2fde7a 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -1043,7 +1043,8 @@ m4_define([m4_mapall],
 # arguments.
 #
 # For m4_mapall_sep, merely expand the first iteration without the
-# separator, then include separator as part of subsequent recursion.
+# separator, then include separator as part of subsequent recursion;
+# but avoid extra expansion of LIST's side-effects via a helper macro.
 # For m4_map_sep, things are trickier - we don't know if the first
 # list element is an empty sublist, so we must define a self-modifying
 # helper macro and use that as the separator instead.
@@ -1052,8 +1053,10 @@ m4_define([m4_map_sep],
 [_m4_map([_m4_apply([m4_Sep([$2])[]$1]], [], $3)m4_popdef([m4_Sep])])
 
 m4_define([m4_mapall_sep],
-[m4_if([$3], [], [],
-       [m4_apply([$1], m4_car($3))_m4_map([m4_apply([$2[]$1]], $3)])])
+[m4_if([$3], [], [], [_$0([$1], [$2], $3)])])
+
+m4_define([_m4_mapall_sep],
+[m4_apply([$1], [$3])_m4_map([m4_apply([$2[]$1]], m4_shift2($@))])
 
 # _m4_map(PREFIX, IGNORED, SUBLIST, ...)
 # --------------------------------------
@@ -2235,15 +2238,19 @@ m4_define([m4_cmp],
 # long lists, since less text is being expanded for deciding when to end
 # recursion.  The recursion is between a pair of macros that alternate
 # which list is trimmed by one element; this is more efficient than
-# calling m4_cdr on both lists from a single macro.
+# calling m4_cdr on both lists from a single macro.  Guarantee exactly
+# one expansion of both lists' side effects.
 m4_define([m4_list_cmp],
+[_$0_raw(m4_dquote($1), m4_dquote($2))])
+
+m4_define([_m4_list_cmp_raw],
 [m4_if([$1], [$2], [0], [_m4_list_cmp_1([$1], $2)])])
 
 m4_define([_m4_list_cmp],
 [m4_if([$1], [], [0m4_ignore], [$2], [0], [m4_unquote], [$2m4_ignore])])
 
 m4_define([_m4_list_cmp_1],
-[_m4_list_cmp_2([$2], m4_dquote(m4_shift2($@)), $1)])
+[_m4_list_cmp_2([$2], [m4_shift2($@)], $1)])
 
 m4_define([_m4_list_cmp_2],
 [_m4_list_cmp([$1$3], m4_cmp([$3+0], [$1+0]))(
@@ -2335,8 +2342,11 @@ m4_dquote(m4_dquote(m4_defn([m4_cr_Letters])))[[+],
 #  -1 if VERSION-1 < VERSION-2
 #   0 if           =
 #   1 if           >
+#
+# Since _m4_version_unletter does not output side effects, we can
+# safely bypass the overhead of m4_version_cmp.
 m4_define([m4_version_compare],
-[m4_list_cmp(_m4_version_unletter([$1]), _m4_version_unletter([$2]))])
+[_m4_list_cmp_raw(_m4_version_unletter([$1]), _m4_version_unletter([$2]))])
 
 
 # m4_PACKAGE_NAME
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index a8cecf9..75c5057 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -527,6 +527,11 @@ m4_version_compare([1.1a], [1.1A])
 m4_version_compare([1z], [1aa])
 m4_version_compare([2.61a], [2.61a-248-dc51])
 m4_version_compare([2.61b], [2.61a-248-dc51])
+dnl Test that side effects to m4_list_cmp occur exactly once
+m4_list_cmp([[0], [0], [0]m4_errprintn([hi])],
+            [[0], [0], [0]m4_errprintn([hi])])
+m4_list_cmp([[0], [0], [0]m4_errprintn([hi])],
+            [[0], [0], [0]m4_errprintn([bye])])
 ]],
 [[-1
 1
@@ -542,6 +547,12 @@ m4_version_compare([2.61b], [2.61a-248-dc51])
 -1
 -1
 1
+0
+0
+]], [[hi
+hi
+hi
+bye
 ]])
 
 AT_CLEANUP
@@ -640,6 +651,8 @@ AT_CLEANUP
 
 AT_SETUP([M4 loops])
 
+AT_KEYWORDS([m4@&address@hidden m4@&address@hidden m4@&address@hidden)
+
 AT_CHECK_M4SUGAR_TEXT([[dnl
 m4_define([myvar], [outer value])dnl
 m4_for([myvar], 1, 3, 1, [ myvar])
@@ -683,6 +696,8 @@ m4_foreach([myvar], [[a], [b, c], [d], [e
 m4_foreach_w([myvar], [a  b c, d,e f
 g], [ myvar|])
 myvar
+dnl only one side effect expansion, prior to visiting list elements
+m4_foreach([i], [[1], [2], [3]m4_errprintn([hi])], [m4_errprintn(i)])dnl
 ]],
 [[ 1 2 3
  1 2 3
@@ -716,7 +731,11 @@ myvar
 | f|
  a| b| c,| d,e| f| g|
 outer value
-]], [])
+]], [[hi
+1
+2
+3
+]])
 
 AT_DATA_M4SUGAR([script.4s],
 [[m4_init
@@ -790,6 +809,11 @@ m4_define([a1], [oops])dnl
 m4_define([pass1], [oops])dnl
 m4_map([a], [[[a]]])1
 m4_map([m4_unquote([a])], [m4_dquote([a])])
+dnl only one side effect expansion, prior to visiting list elements
+m4_map([m4_errprintn], [[[1]], [[2]], [[3]]m4_errprintn([hi])])dnl
+m4_map_sep([m4_errprintn], [], [[[1]], [[2]], [[3]]m4_errprintn([hi])])dnl
+m4_mapall([m4_errprintn], [[[1]], [[2]], [[3]]m4_errprintn([hi])])dnl
+m4_mapall_sep([m4_errprintn], [], [[[1]], [[2]], [[3]]m4_errprintn([hi])])dnl
 ]],
 [[
  1 2
@@ -806,7 +830,23 @@ m4_map([m4_unquote([a])], [m4_dquote([a])])
 -
 pass1
 pass
-]], [])
+]], [[hi
+1
+2
+3
+hi
+1
+2
+3
+hi
+1
+2
+3
+hi
+1
+2
+3
+]])
 
 AT_CLEANUP
 
@@ -1010,7 +1050,7 @@ m4_undefine(1m4_for([i], [2], [10000], [], [,i]))dnl
 m4_bpatsubsts([a1]m4_for([i], [1], [10000], [], [,i]), [a2], [A])
 m4_bmatch([9997]m4_for([i], [1], [10000], [], [,^i$]))
 m4_define([up], [m4_define([$1], m4_incr($1))$1])m4_define([j], 0)dnl
-m4_cond(m4_for([i], [1], [10000], [], [[up([j])], [9990], i,]) [oops])
+m4_cond(m4_for([i], [1], [10000], [], [[up([j])], [9990], i,]) [oops]) j
 m4_count(m4_transform_pair([,m4_quote], []m4_transform([,m4_echo]m4_for([i],
   [1], [10000], [], [,i]))))
 m4_divert_pop(0)
-- 
1.6.0








reply via email to

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