From 550a9fa1e1f12b297730bf6db132fc4818686a33 Mon Sep 17 00:00:00 2001 From: Eric Blake 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@&t@_map, m4@&t@_version_compare): Ensure only one side effect. (recursion): Fix test typo. Reported by Ralf Wildenhues. Signed-off-by: Eric Blake --- 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 7db4f57ba..99a0e0584 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2008-08-21 Eric Blake + + 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@&t@_map, m4@&t@_version_compare): + Ensure only one side effect. + (recursion): Fix test typo. + Reported by Ralf Wildenhues. + 2008-08-21 Ralf Wildenhues * TODO: Add item for compiler default flags. diff --git a/doc/autoconf.texi b/doc/autoconf.texi index 5742f7ef8..75c110e55 100644 --- a/doc/autoconf.texi +++ b/doc/autoconf.texi @@ -10756,6 +10756,22 @@ It is common to see lists with unquoted elements when those elements are 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, + +@example +m4_foreach([i], [[1], [2], [3]m4_errprintn([hi])], [i]) +@error{}hi +@result{}123 +m4_define([list], [[1], [2], [3]]) +@result{} +m4_foreach([i], [list], [i]) +@result{}123 +@end 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 91a964306..c35d5d36c 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 b33737087..f2fde7ac8 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 a8cecf904..75c505729 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@&t@_for m4@&t@_foreach m4@&t@_foreach_w]) + 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) -- 2.47.3