more fun with m4_expand

[Eric Blake]

I was about to make AS_ESCAPE always use m4_expand, until I ran into an issue: 
currently, we pass AC_INCLUDES_DEFAULT through AS_ESCAPE, which in turn calls 
m4_divert_text.  If you call m4_expand on that, instead of performing the macro 
expansion after the escapes are added, then the text that should have been 
diverted instead ends up as part of the argument collection being done by 
m4_expand.  Not good.

So here's a 2-patch series that first identifies this situation (to make it 
easier to spot if it occurs in the wild), and second adds support for 
m4_divert_text inside m4_expand.  This still doesn't quite fix things for 
AC_INCLUDES_DEFAULT (I have to figure out how to factor out the AC_REQUIRE, 
since that changes diversions without using m4_divert_text), but it is 
progress, and makes one more thing that m4_expand can handle but m4_quote 
cannot.

I couldn't figure out a clean way to make:

m4_expand([m4_divert_text([1], [m4_divert_text([2], [ 2)]) 1)] 0)])

print exactly one instance of [ 2)] (different variations gave 0 or 2 instances 
of it, because m4_expand parses the inner m4_divert_text multiple times when 
trying to figure out how many `(' to prepend), but it seems fishy for diverted 
text to recursively want to divert more text, when you can just call 
m4_divert_text twice at the outer level, so I'm not too worried.

I'm going to sit on this patch a few more days before applying, unless I get a 
review.

>From 08dcf8a7622815dcedd9c03a353aee897085ba01 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 24 Nov 2008 13:52:01 -0700
Subject: [PATCH] Add safety check for m4_expand vs. diversions.

* lib/m4sugar/m4sugar.m4 (m4_expand): Make more robust against
diverted text.
* doc/autoconf.texi (Evaluation Macros) <m4_expand>: Document new
safety check.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |    8 ++++++++
 doc/autoconf.texi      |   28 ++++++++++++++++++++++++++--
 lib/m4sugar/m4sugar.m4 |   23 +++++++++++++++++++----
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c6c7109..97812a7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2008-11-24  Eric Blake  <address@hidden>
 
+       Add safety check for m4_expand vs. diversions.
+       * lib/m4sugar/m4sugar.m4 (m4_expand): Make more robust against
+       diverted text.
+       * doc/autoconf.texi (Evaluation Macros) <m4_expand>: Document new
+       safety check.
+
+2008-11-24  Eric Blake  <address@hidden>
+
        Fix typo in AS_MESSAGE_LOG_FD patch.
        * lib/m4sugar/m4sh.m4 (AS_ERROR): Check correct condition.
 
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 957b8be..c198e2e 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -10725,10 +10725,31 @@ Diversion support
 M4sugar makes heavy use of diversions, because it is often the case that
 text that must appear early in the output is not discovered until late
 in the input.  Additionally, some of the topological sorting algorithms
-used in resolving macro dependencies use diversions.  Therefore, most
+used in resolving macro dependencies use diversions.  However, most
 macros should not need to change diversions directly, but rather rely on
 higher-level M4sugar macros to manage diversions transparently.
 
+In the rare case that it is necessary to write a macro that explicitly
+outputs text to a different diversion, it is important to be aware of an
+M4 limitation regarding diversions: text only goes to a diversion if it
+is not part of argument collection.  Therefore, any macro that changes
+the current diversion cannot be used as an unquoted argument to another
+macro, but must be expanded at the top level.  The macro
address@hidden will diagnose any attempt to change diversions, since
+it is generally useful only as an argument to another macro.  The
+following example shows what happens when diversion manipulation is
+attempted within macro arguments; the text @code{unwanted} is output,
+even though it was processed while the current diversion was
address@hidden, because it was part of the argument collection of
address@hidden:
+
address@hidden
+m4_echo([good
+]m4_divert_push([KILL])unwanted[]m4_divert_pop([KILL]))
address@hidden
address@hidden
address@hidden example
+
 To make diversion management easier, M4sugar uses the concept of named
 diversions.  Rather than using diversion numbers directly, it is nicer
 to associate a name with each diversion; the diversion number associated
@@ -11295,7 +11316,10 @@ Evaluation Macros
 @var{arg}.  Additionally, @code{m4_expand} is able to expand text that
 would involve an unterminated comment, whereas expanding that same text
 as the argument to @code{m4_quote} runs into difficulty in finding the
-end of the argument.
+end of the argument.  Since manipulating diversions during argument
+collection is inherently unsafe, @code{m4_expand} issues an error if
address@hidden attempts to change the current diversion (@pxref{Diversion
+support}).
 
 @example
 m4_define([active], [ACT, IVE])dnl
diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index fd443ca..3901c19 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -832,8 +832,11 @@ m4_define([m4_echo], address@hidden)
 # this), and should not contain the unlikely delimiters -=<{( or
 # )}>=-.  It is possible to have unbalanced quoted `(' or `)', as well
 # as unbalanced unquoted `)'.  m4_expand can handle unterminated
-# comments or dnl on the final line, at the expense of speed, while
-# _m4_expand is faster but must be given a terminated expansion.
+# comments or dnl on the final line, at the expense of speed; it also
+# aids in detecting attempts to incorrectly change the current
+# diversion inside ARG.  Meanwhile, _m4_expand is faster but must be
+# given a terminated expansion, and has no safety checks for
+# mis-diverted text.
 #
 # Exploit that extra unquoted () will group unquoted commas and the
 # following whitespace.  m4_bpatsubst can't handle newlines inside $1,
@@ -848,8 +851,11 @@ m4_define([m4_echo], address@hidden)
 # this time with one more `(' in the second argument and in the
 # open-quote delimiter.  We must also ignore the slop from the
 # previous try.  The final macro is thus half line-noise, half art.
-m4_define([m4_expand], [m4_chomp(_$0([$1
-]))])
+m4_define([m4_expand],
+[m4_pushdef([m4_divert], _m4_defn([_m4_divert_unsafe]))]dnl
+[m4_pushdef([m4_divert_push], _m4_defn([_m4_divert_unsafe]))]dnl
+[m4_chomp(_$0([$1
+]))_m4_popdef([m4_divert], [m4_divert_push])])
 
 m4_define([_m4_expand], [$0_([$1], [(], -=<{($1)}>=-, [}>=-])])
 
@@ -1380,6 +1386,15 @@ m4_define([m4_divert_once],
 [m4_expand_once([m4_divert_text([$1], [$2])])])
 
 
+# _m4_divert_unsafe
+# -----------------
+# Issue a warning that the attempt to change the current diversion is
+# unsafe, because this macro is being expanded during argument
+# collection of m4_expand.
+m4_define([_m4_divert_unsafe],
+[m4_fatal([$0: cannot change diversion to `$1' inside m4_expand])])
+
+
 # m4_undivert(DIVERSION-NAME...)
 # ------------------------------
 # Undivert DIVERSION-NAME.  Unlike the M4 version, this requires at
-- 
1.6.0.4


>From b13f347e421d89c23a4d2eeb57198bb3fadcef7b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 24 Nov 2008 15:08:25 -0700
Subject: [PATCH] Support m4_divert_text inside m4_expand.

* lib/m4sugar/m4sugar.m4 (_m4_divert_text, _m4_divert_undefer):
New macros.
(m4_expand): Handle m4_divert_text by deferring it to the next
top-level diversion change.
(m4_divert, m4_divert_push): Perform any deferred diversions.
* tests/m4sugar.at (m4@&address@hidden): Enhance test.
* doc/autoconf.texi (Evaluation Macros) <m4_expand>
(Diversion support) <m4_divert_text>: Document new limited support
for diversion inside m4_expand.
* NEWS: Document the change.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |   12 +++++++++++
 NEWS                   |   10 ++++----
 doc/autoconf.texi      |    9 ++++++-
 lib/m4sugar/m4sugar.m4 |   51 ++++++++++++++++++++++++++++++++++++++++-------
 tests/m4sugar.at       |    4 +++
 5 files changed, 71 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 97812a7..42ea686 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2008-11-24  Eric Blake  <address@hidden>
 
+       Support m4_divert_text inside m4_expand.
+       * lib/m4sugar/m4sugar.m4 (_m4_divert_text, _m4_divert_undefer):
+       New macros.
+       (m4_expand): Handle m4_divert_text by deferring it to the next
+       top-level diversion change.
+       (m4_divert, m4_divert_push): Perform any deferred diversions.
+       * tests/m4sugar.at (m4@&address@hidden): Enhance test.
+       * doc/autoconf.texi (Evaluation Macros) <m4_expand>
+       (Diversion support) <m4_divert_text>: Document new limited support
+       for diversion inside m4_expand.
+       * NEWS: Document the change.
+
        Add safety check for m4_expand vs. diversions.
        * lib/m4sugar/m4sugar.m4 (m4_expand): Make more robust against
        diverted text.
diff --git a/NEWS b/NEWS
index 3edc6bc..f5f1856 100644
--- a/NEWS
+++ b/NEWS
@@ -30,11 +30,11 @@ GNU Autoconf NEWS - User visible changes.
    m4_copy  m4_dumpdefs  m4_rename
 
 ** The m4sugar macro m4_expand has been taught to handle unterminated
-   comments and shell case statements.  As a result, it is used
-   internally in more places, such as AC_DEFINE and AT_CHECK.  Most
-   uses of AC_DEFINE and AT_CHECK should not behave any differently;
-   however, it may be necessary to add double-quoting around
-   unbalanced `(' where single-quoting used to be sufficient.
+   comments, shell case statements, and m4_divert_text.  As a result,
+   it is used internally in more places, such as AC_DEFINE and
+   AT_CHECK.  Most uses of AC_DEFINE and AT_CHECK should not behave
+   any differently; however, it may be necessary to add double-quoting
+   around unbalanced `(' where single-quoting used to be sufficient.
 
 ** The following documented m4sh macros are new:
    AS_LINENO_PREPARE  AS_ME_PREPARE  AS_SET_STATUS  AS_VAR_APPEND
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index c198e2e..aab94b3 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -10835,11 +10835,16 @@ Diversion support
 @defmac m4_divert_text (@var{diversion}, @ovar{content})
 @msindex{divert_text}
 Output @var{content} and a newline into @var{diversion}, without
-affecting the current diversion.  Shorthand for:
+affecting the current diversion.  This is conceptually shorthand for:
 @example
 m4_divert_push(address@hidden)@var{content}
 m4_divert_pop(address@hidden)dnl
 @end example
+
address@hidden
+except that @code{m4_divert_text} can be safely used inside
address@hidden while @code{m4_divert_push} cannot.  @var{content}
+should not try to recursively use @code{m4_divert_text}.
 @end defmac
 
 @defmac m4_init
@@ -11319,7 +11324,7 @@ Evaluation Macros
 end of the argument.  Since manipulating diversions during argument
 collection is inherently unsafe, @code{m4_expand} issues an error if
 @var{arg} attempts to change the current diversion (@pxref{Diversion
-support}).
+support}) via any means other than @code{m4_divert_text}.
 
 @example
 m4_define([active], [ACT, IVE])dnl
diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4
index 3901c19..3566534 100644
--- a/lib/m4sugar/m4sugar.m4
+++ b/lib/m4sugar/m4sugar.m4
@@ -844,6 +844,10 @@ m4_define([m4_echo], address@hidden)
 # temporary quotes to remove the delimiters that conveniently included
 # the unquoted () that were added prior to the changequote.
 #
+# m4_divert_text is supported by deferring the diversion until after a
+# successful m4_expand, while keeping in mind that the side effect of
+# deferral may happen more than once if there are unbalanced `)'.
+#
 # Thanks to shell case statements, too many people are prone to pass
 # underquoted `)', so we try to detect that by passing a marker as a
 # fourth argument; if the marker is not present, then we assume that
@@ -854,8 +858,13 @@ m4_define([m4_echo], address@hidden)
 m4_define([m4_expand],
 [m4_pushdef([m4_divert], _m4_defn([_m4_divert_unsafe]))]dnl
 [m4_pushdef([m4_divert_push], _m4_defn([_m4_divert_unsafe]))]dnl
-[m4_chomp(_$0([$1
-]))_m4_popdef([m4_divert], [m4_divert_push])])
+[m4_pushdef([m4_divert_text], _m4_defn([_m4_divert_text]))]dnl
+[m4_pushdef([_m4_divert_text_defer])]dnl
+[m4_chomp(_$0([m4_define([_m4_divert_text_defer])$1
+]))m4_define([_m4_divert_defer],
+  _m4_defn([_m4_divert_defer])_m4_defn([_m4_divert_text_defer]))]dnl
+[_m4_popdef([m4_divert], [m4_divert_push], [m4_divert_text],
+  [_m4_divert_text_defer])])
 
 m4_define([_m4_expand], [$0_([$1], [(], -=<{($1)}>=-, [}>=-])])
 
@@ -1338,16 +1347,26 @@ m4_define([m4_divert_stack_push],
 # -------------------------
 # Change the diversion stream to DIVERSION-NAME.
 m4_define([m4_divert],
+[_m4_divert_undefer()]dnl
 [m4_popdef([_m4_divert_stack])]dnl
 [m4_define([_m4_divert_diversion], [$1])]dnl
 [m4_divert_stack_push([$0], [$1])]dnl
 [m4_builtin([divert], _m4_divert([$1]))])
 
 
+# m4_divert_once(DIVERSION-NAME, CONTENT)
+# ---------------------------------------
+# Output CONTENT into DIVERSION-NAME once, if not already there.
+# An end of line is appended for free to CONTENT.
+m4_define([m4_divert_once],
+[m4_expand_once([m4_divert_text([$1], [$2])])])
+
+
 # m4_divert_push(DIVERSION-NAME)
 # ------------------------------
 # Change the diversion stream to DIVERSION-NAME, while stacking old values.
 m4_define([m4_divert_push],
+[_m4_divert_undefer()]dnl
 [m4_divert_stack_push([$0], [$1])]dnl
 [m4_pushdef([_m4_divert_diversion], [$1])]dnl
 [m4_builtin([divert], _m4_divert([$1]))])
@@ -1373,17 +1392,33 @@ m4_define([m4_divert_pop],
 # ---------------------------------------
 # Output CONTENT into DIVERSION-NAME (which may be a number actually).
 # An end of line is appended for free to CONTENT.
+#
+# We also supply an alternate definition in use during m4_expand,
+# which defers the diversion changes until the next top-level macro
+# that wants to change the diversion.  _m4_divert_text collects into
+# _m4_divert_text_defer, which m4_expand then moves into
+# _m4_divert_defer (necessary, since m4_expand may evaluate the
+# m4_divert_text more than once while handling shell case statements),
+# then _m4_divert_undefer is called at strategic points to empty
+# _m4_divert_defer.
 m4_define([m4_divert_text],
 [m4_divert_push([$1])$2
 m4_divert_pop([$1])])
 
+m4_define([_m4_divert_text],
+[m4_define([_m4_divert_text_defer], _m4_defn([_m4_divert_text_defer])]dnl
+[[m4_builtin([divert], ]_m4_divert([$1])[)]m4_dquote(_m4_expand([$2
+])))])
 
-# m4_divert_once(DIVERSION-NAME, CONTENT)
-# ---------------------------------------
-# Output CONTENT into DIVERSION-NAME once, if not already there.
-# An end of line is appended for free to CONTENT.
-m4_define([m4_divert_once],
-[m4_expand_once([m4_divert_text([$1], [$2])])])
+
+# _m4_divert_undefer
+# ------------------
+# Process any pending deferred m4_divert_text directives, prior to
+# another top-level diversion change.
+m4_define([_m4_divert_undefer],
+[m4_unquote(_m4_defn([_m4_divert_defer]))m4_define([_m4_divert_defer])])
+
+m4_define([_m4_divert_defer])
 
 
 # _m4_divert_unsafe
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index 4bdc06c..a324816 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -707,6 +707,8 @@ m4_expand([a
 dnl])
 m4_expand([a
 -dnl])
+dnl fun with diversions
+m4_expand([m4_divert_text([1], [1)])0)])
 ]],
 [[#active
 ACTIVE
@@ -727,6 +729,8 @@ ACTIVE # active
 a
 a
 -
+0)
+1)
 ]])
 
 AT_CLEANUP
-- 
1.6.0.4