Re: [PATCH] add m4_stack_foreach and m4_stack_foreach_lifo

[Paolo Bonzini]

> m4_dumpdefs can be rewritten to this interface.

Good idea!

> m4_set also uses pushdef stack manipulation, and it may be possible to
> rewrite _m4_set_contents_{1,1c,2} to use this idiom.

Will not do. :-)

> But the lack of a better interface shouldn't stop this patch.

Agreed. :-)

>> The disadvantage is that more macros cannot be copied with m4_copy,
>> including m4_curry and m4_stack_foreach.
> 
> Again, I already proposed a solution for that; maybe it's time to
> implement it?

I'm wary of possible slowdowns.

>> The next patch will use the macros to implement the m4 diversion and
>> expansion stacks in a nicer way, and without possibly quadratic behavior.
> 
> Sweet - I was thinking about that very inefficiency as my next project; it
> looks like you are beating me to it!  There is no maybe about the
> behavior; the current implementation IS quadratic (at least in memory, but
> since it mallocs, probably also in time), because every round of
> m4_expansion_stack_push is copying all previous rounds of text into the
> new definition.

Yes, it just does not show up in practice.

> Do we want to document this in the manual and NEWS, or leave it
> undocumented until we've played with it a bit longer?

Undocumented for now, methinks.

> One more caveat to document - FUNC must not arbitrarily pushdef or popdef
> MACRO, or traversal will be broken.  Except maybe we want to document that
> a single m4_popdef is supported, as a way to prune intermediate stack
> values without affecting the topmost definition.

I'll document that you should not do anything with MACRO, not even rely
on its definition.

> In the context of m4_copy, using m4_tmp was safe, since the user could not
> inject arbitrary macro expansions into the traversal.  But it is now
> conceivable that the user could pass a FUNC that does a nested
> m4_stack_foreach on a different macro.

I see.

> That IS slick.

Happy you liked it!

> The test title is automatically a keyword, so the second listing of
> m4_stack_foreach is redundant.  Let's also add a test of m4_dumpdefs to
> this test.

Ok.  But no m4_dumpdefs, because it writes on stderr and confuses autom4te.

I attach the incremental changes. Same ChangeLog except for the additional

(_m4_dumpdefs_down, _m4_dumpdefs_up): Remove.
(m4_dumpdefs): Rewrite using m4_stack_foreach_lifo.

Paolo

diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index e1728bc..7d61f76 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -529,10 +529,13 @@ m4_define([_m4_bpatsubsts],
 
 # m4_stack_foreach(MACRO, FUNC)
 # m4_stack_foreach_lifo(MACRO, FUNC)
-# -----------------------------
+# ----------------------------------
 # Pass each stacked definition of MACRO to the one-argument macro FUNC.
 # m4_stack_foreach proceeds in FIFO order, while m4_stack_foreach_lifo
-# processes the topmost definitions first.
+# processes the topmost definitions first.  In addition, FUNC should
+# not push or pop definitions of MACRO, and should not expect anything about
+# the active definition of MACRO (it will not be the topmost, and may not
+# be the one passed to FUNC either).
 #
 # The recursive worker _m4_stack_reverse destructively swaps the order of a
 # stack.  We use a temporary stack, and swap directions twice.  Some macros
@@ -542,12 +545,26 @@ m4_define([_m4_stack_reverse],
 [m4_ifdef([$1], [m4_pushdef([$2], _m4_defn([$1]))$3[]_m4_popdef([$1])$0($@)])])
 
 m4_define([m4_stack_foreach],
-[_m4_stack_reverse([$1], [m4_tmp])]dnl
-[_m4_stack_reverse([m4_tmp], [$1], [$2(_m4_defn([m4_tmp]))])])
+[_m4_stack_reverse([$1], [m4_tmp_$1])]dnl
+[_m4_stack_reverse([m4_tmp_$1], [$1], [$2(_m4_defn([m4_tmp_$1]))])])
 
 m4_define([m4_stack_foreach_lifo],
-[_m4_stack_reverse([$1], [m4_tmp], [$2(_m4_defn([m4_tmp]))])]dnl
-[_m4_stack_reverse([m4_tmp], [$1])])
+[_m4_stack_reverse([$1], [m4_tmp_$1], [$2(_m4_defn([m4_tmp_$1]))])]dnl
+[_m4_stack_reverse([m4_tmp_$1], [$1])])
+
+
+# m4_dumpdefs(NAME)
+# -----------------
+# Similar to `m4_dumpdef(NAME)', but if NAME was m4_pushdef'ed, display its
+# value stack (most recent displayed first).
+#
+# This macro cheats, because it relies on the current definition of NAME
+# while the second argument of m4_stack_foreach_lifo is evaluated (which
+# would be undefined according to the API).  If m4_dumpdef is ever rewritten
+# not to use the builtin, revisit this.
+m4_define([m4_dumpdefs],
+[m4_stack_foreach_lifo([$1], [m4_dumpdef([$1])m4_ignore])])
+
 
 # m4_copy(SRC, DST)
 # -----------------
@@ -618,34 +635,6 @@ m4_define([m4_defn],
        [m4_foreach([_m4_macro], address@hidden, 
[$0(_m4_defn([_m4_macro]))])])])
 
 
-# _m4_dumpdefs_up(NAME)
-# ---------------------
-m4_define([_m4_dumpdefs_up],
-[m4_ifdef([$1],
-         [m4_pushdef([_m4_dumpdefs], _m4_defn([$1]))dnl
-m4_dumpdef([$1])dnl
-_m4_popdef([$1])dnl
-_m4_dumpdefs_up([$1])])])
-
-
-# _m4_dumpdefs_down(NAME)
-# -----------------------
-m4_define([_m4_dumpdefs_down],
-[m4_ifdef([_m4_dumpdefs],
-         [m4_pushdef([$1], _m4_defn([_m4_dumpdefs]))dnl
-_m4_popdef([_m4_dumpdefs])dnl
-_m4_dumpdefs_down([$1])])])
-
-
-# m4_dumpdefs(NAME)
-# -----------------
-# Similar to `m4_dumpdef(NAME)', but if NAME was m4_pushdef'ed, display its
-# value stack (most recent displayed first).
-m4_define([m4_dumpdefs],
-[_m4_dumpdefs_up([$1])dnl
-_m4_dumpdefs_down([$1])])
-
-
 # m4_popdef(NAME)
 # ---------------
 # Like the original, except guarantee a warning when using something which is
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index 9cf9522..8ac28d9 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -43,7 +43,7 @@ AT_CHECK_M4SUGAR([-o-],, [$2], [$3])
 
 AT_SETUP([m4@&address@hidden)
 
-AT_KEYWORDS([m4@&address@hidden m4@&address@hidden m4@&address@hidden 
m4@&address@hidden)
+AT_KEYWORDS([m4@&address@hidden m4@&address@hidden m4@&address@hidden 
m4@&address@hidden)
 
 # Test the semantics of macros to walk stacked macro definitions.
 AT_CHECK_M4SUGAR_TEXT([[dnl