Re: [PATCH] add m4_stack_foreach and m4_stack_foreach_lifo

[Eric Blake]

[dropping m4-discuss; and gmane's web interface to reply to
cross-posted threads is lousy].

Paolo Bonzini <bonzini <at> gnu.org> writes:

> > 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. 

Fair enough; I wrote m4_set, so I'm stuck maintaining it. ;)
I can do that in a followup patch.

> > 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.

Reasonable.

> > 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.

Ouch.  So it does (if you aren't using m4.git's master branch).  I'm
rather used to the following idiom for quick m4sugar tests:

m4 -Ilib m4sugar/m4sugar.m4 -

in part because autom4te caches output until the end, and because
I'm not worrying about allowed/forbidden tokens when testing m4sugar
algorithms.  In that setup, m4_dumpdef/m4_dumpdefs actually works.
But you are right; with m4 1.4.x, m4_dumpdef outputs to --debugfile,
rather than to stderr.  Note that beta m4 1.4o, and the current m4.git
master branch, always send dumpdef output to stderr, regardless of
the debug file.  I'm debating as to whether m4 1.6 should also always
send dumpdef to stderr, as that would certainly be more useful for
autoconf's needs.

At any rate, we _have_ to override m4_dumpdef to use m4_errprint, if
we don't want autom4te to crash like it did (at least override it for
1.4.x, using the __m4_version__ check also in use for
_m4_defn/m4_foreach/...).
So I will go ahead and write a patch for that.

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

OK to apply after the following changes:

> 
>  # m4_stack_foreach(MACRO, FUNC)
>  # m4_stack_foreach_lifo(MACRO, FUNC)
> -# -----------------------------
> +# ----------------------------------

[putting my picky maintainer hat on]
Can we please maintain lexicographic sorting within the sections of
m4sugar?  For that matter, m4_stack_* provides a looping primitive;
so I think it fits better in section 8, on m4 loops; so can you place it
after m4_map_args_pair and before section 9, rather than right here?
Yes, that means that m4_copy and m4_dumpdefs are defined prior to
_m4_stack_reverse; however, that is not a problem since they aren't
used in the meantime in creating other m4sugar macro definitions.

>  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]))])])

Let's use the name [m4_tmp-$1] rather than [m4_tmp_$1]; that way,
it is not a valid macro name, and is that much less likely to trip up a
poorly written FUNC (and ensures that we are using m4_defn/m4_indir
when we are supposed to, rather than macro invocation).
(_m4_set_contents_1 used _m4_set_($1) as the invalid macro name,
but [-$1] is shorter than [($1)], and slightly easier to use).

> +# m4_dumpdefs(NAME)
> +# -----------------
> +# Similar to `m4_dumpdef(NAME)', but if NAME was m4_pushdef'ed, display
> its
> +# value stack (most recent displayed first).

Why are you relocating this macro?  It can safely stay where
it was, in sorted order.

> --- a/tests/m4sugar.at
> +++ b/tests/m4sugar.at
> @@ -43,7 +43,7 @@ AT_CHECK_M4SUGAR([-o-],, [$2], [$3])
> 
>  AT_SETUP([m4@&t <at> _stack_foreach])
> 
> -AT_KEYWORDS([m4@&t <at> _stack_foreach m4@&t <at> _stack_foreach_lifo
> m4@&t <at> _copy m4@&t <at> _n])
> +AT_KEYWORDS([m4@&t <at> _dumpdefs m4@&t <at> _stack_foreach_lifo m4@&t
> <at> _copy m4@&t <at> _n])

No need to call out m4_dumpdefs after all; I'll add a (separate)
m4_dumpdef[s] test once I rewrite it to not break autom4te.

Can you also add this testcase before committing, so that we
stress nested stack listings:

m4_pushdef([xyz], [123])dnl
m4_pushdef([xyz], [456])dnl
m4_define([doit], [[$1](m4_shift(m4_stack_foreach([xyz], [,m4_echo])))
])
m4_stack_foreach([abc], [doit])

and verify the results:

abc(123,456)
def(123,456)
ghi(123,456)

(If you don't add this, then I will do it in the same followup as
when I revisit the m4_set stack traversals.)

-- 
Eric Blake

-- 
View this message in context: 
http://www.nabble.com/-PATCH--add-m4_stack_foreach-and-m4_stack_foreach_lifo-tp20206169p20211588.html
Sent from the Gnu - Autoconf - Patches mailing list archive at Nabble.com.