Re: AC_REQUIRE fails to ensure ordering of macros

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 1/16/2009 6:08 AM:
> That last link was where I concluded, along with Paolo, that AC_REQUIRE
> has no choice but to expand prerequisites prior to the outermost
> AC_DEFUN'd macro.
> 
>> But the documentation of AC_REQUIRE
>> <http://www.gnu.org/software/autoconf/manual/html_node/Prerequisite-Macros.html>
>> says:
> 
>>   "To be more precise, the required macro is expanded before the outermost
>>    defined macro in the current expansion stack."
> 
> And that documentation stands, although I will be further expanding it to
> give the examples that we have uncovered, including advice on breaking the
> expanded-before-indirectly-required problem that several people have
> encountered.

Here's what I'm committing as a first round of documentation.  If I ever
get my desired warning in place, then I will remove the XFAIL and further
enhance the documentation to add a FAQ node about how to avoid the warning
about expanded-before-required.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkl0jAsACgkQ84KuGfSFAYAL8gCfeY7oNIALH1r7xsza2GFyXyow
LV4AoIWXZYYI2s28SPOON6NJafusAt2/
=9gg8
-----END PGP SIGNATURE-----

From 34e2c8d5484b2e74b7242a19ebca51f9aff82028 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 17 Jan 2009 09:36:02 -0700
Subject: [PATCH] Improve AC_REQUIRE documentation.

* doc/autoconf.texi (Macro Definitions) <AC_DEFUN>: Add @defmac,
and mention interaction with AC_REQUIRE.
(Prerequisite Macros) <AC_REQUIRE>: Give more detail on user
ordering constraint bug, and how to fix it.
* tests/m4sugar.at (m4@&address@hidden: nested): New test.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |    7 +++
 doc/autoconf.texi |   93 ++++++++++++++++++++++++++++++++++++++++++---
 tests/m4sugar.at  |  108 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 200 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 120a70c..7029b84 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2009-01-19  Eric Blake  <address@hidden>
 
+       Improve AC_REQUIRE documentation.
+       * doc/autoconf.texi (Macro Definitions) <AC_DEFUN>: Add @defmac,
+       and mention interaction with AC_REQUIRE.
+       (Prerequisite Macros) <AC_REQUIRE>: Give more detail on user
+       ordering constraint bug, and how to fix it.
+       * tests/m4sugar.at (m4@&address@hidden: nested): New test.
+
        Speed up m4_require.
        * lib/m4sugar/m4sugar.m4 (_m4_divert_dump): Change semantics to
        always be defined, as either empty or a number.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 9a17308..69d5085 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -183,7 +183,8 @@
 templates and an M4 macro package.
 
 Copyright @copyright{} 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000,
-2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Free Software
+Foundation, Inc.
 
 @quotation
 Permission is granted to copy, distribute and/or modify this document
@@ -12800,12 +12801,15 @@ Writing Autoconf Macros
 @node Macro Definitions
 @section Macro Definitions
 
address@hidden AC_DEFUN (@var{name}, @ovar{body})
 @acindex{DEFUN}
 Autoconf macros are defined using the @code{AC_DEFUN} macro, which is
-similar to the M4 builtin @code{m4_define} macro.  In addition to
+similar to the M4 builtin @code{m4_define} macro; this creates a macro
+named @var{name} and with @var{body} as its expansion.  In addition to
 defining a macro, @code{AC_DEFUN} adds to it some code that is used to
-constrain the order in which macros are called (@pxref{Prerequisite
-Macros}).
+constrain the order in which macros are called, while avoiding redundant
+output (@pxref{Prerequisite Macros}).
address@hidden defmac
 
 An Autoconf macro definition looks like this:
 
@@ -12817,6 +12821,16 @@ Macro Definitions
 @samp{$2}, etc.  @xref{Definitions, , How to define new macros, m4.info,
 @acronym{GNU} M4}, for more complete information on writing M4 macros.
 
+Most macros fall in one of two general categories.  The first category
+includes macros which take arguments, in order to generate output
+parameterized by those arguments.  Macros in this category are designed
+to be directly expanded, often multiple times, and should not be used as
+the argument to @code{AC_REQUIRE}.  The other category includes macros
+which are shorthand for a fixed block of text, and therefore do not take
+arguments.  For this category of macros, directly expanding the macro
+multiple times results in redundant output, so it is more common to use
+the macro as the argument to @code{AC_REQUIRE}.
+
 Be sure to properly quote both the @var{macro-body} @emph{and} the
 @var{macro-name} to avoid any problems if the macro happens to have
 been previously defined.
@@ -13030,7 +13044,8 @@ Prerequisite Macros
 that it has been called.
 
 @code{AC_REQUIRE} must be used inside a macro defined by @code{AC_DEFUN}; it
-must not be called from the top level.
+must not be called from the top level.  Also, it does not make sense to
+require a macro that takes parameters.
 @end defmac
 
 @code{AC_REQUIRE} is often misunderstood.  It really implements
@@ -13103,9 +13118,75 @@ Prerequisite Macros
 @end group
 @end example
 
+However, this implementation can lead to another class of problems.
+Since dependencies are hoisted prior to the outermost @code{AC_DEFUN}
+body, but only if they have not been previously seen, it is still
+possible to encounter out-of-order expansion.  Given this example:
+
address@hidden
+AC_DEFUN([A], [[in A]])
+AC_DEFUN([B], [AC_REQUIRE([A])[in B]])
+AC_DEFUN([C], [AC_REQUIRE([B])[in C]])
+AC_DEFUN([OUTER], [[in OUTER]
+A
+C])
+OUTER
address@hidden example
+
address@hidden
+observe that the expansion of @code{B} occurred prior to the expansion
+of @code{OUTER}, but because the requirement for @code{B} was not
+encountered until after @code{A} had already been directly expanded,
+there was no reason to hoist @code{A}.  Or, put another way, the fact
+that @code{B} was hoisted but not @code{A} means that @code{B} occurs
+before its prerequisite:
+
address@hidden
+in B
+in OUTER
+in A
+in C
address@hidden example
+
+The bug is not in Autoconf, but in the macro definitions.  If you ever
+pass a particular macro name to @code{AC_REQUIRE}, then you are implying
+that the macro only needs to be expanded once.  But to enforce this, all
+uses of that macro should be through @code{AC_REQUIRE}; directly
+expanding the macro defeats the point of using @code{AC_REQUIRE} to
+eliminate redundant expansion.  In the example, this rule of thumb was
+violated because @code{B} requires @code{A} while @code{OUTER} directly
+expands it.  One way of fixing the bug is to factor @code{A} into two
+macros, the portion designed for direct and repeated use (here, named
address@hidden), and the portion designed for one-shot output and used only
+inside @code{AC_REQUIRE} (here, named @code{A_PREREQ}).  Then, by fixing
+all clients to use the correct macro according to their needs:
+
address@hidden
+AC_DEFUN([A], [AC_REQUIRE([A_PREREQ])[in A]])
+AC_DEFUN([A_PREREQ], [[in A_PREREQ]])
+AC_DEFUN([B], [AC_REQUIRE([A_PREREQ])[in B]])
+AC_DEFUN([C], [AC_REQUIRE([B])[in C]])
+AC_DEFUN([OUTER], [[in OUTER]
+A
+C])
+OUTER
address@hidden example
+
address@hidden
+the resulting output will then obey all dependency rules:
+
address@hidden
+in A_PREREQ
+in B
+in OUTER
+in A
+in C
address@hidden example
+
 The helper macros @code{AS_IF} and @code{AS_CASE} may be used to
 enforce expansion of required macros outside of shell conditional
-constructs.  You are furthermore encouraged to put all @code{AC_REQUIRE} calls
+constructs.  You are furthermore encouraged, although not required, to
+put all @code{AC_REQUIRE} calls
 at the beginning of a macro.  You can use @code{dnl} to avoid the empty
 lines they leave.
 
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index 0d90ca2..e822ffb 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -2,8 +2,8 @@
 
 AT_BANNER([M4sugar.])
 
-# Copyright (C) 2000, 2001, 2002, 2005, 2006, 2007, 2008 Free Software
-# Foundation, Inc.
+# Copyright (C) 2000, 2001, 2002, 2005, 2006, 2007, 2008, 2009 Free
+# Software Foundation, Inc.
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -379,6 +379,7 @@ AT_CLEANUP
 ## --------------------------- ##
 
 AT_SETUP([m4@&address@hidden: error message])
+AT_KEYWORDS([m4@&address@hidden)
 
 AT_DATA_M4SUGAR([script.4s],
 [[m4_defun([foo], [FOO])
@@ -398,6 +399,7 @@ AT_CLEANUP
 ## ----------------------------------- ##
 
 AT_SETUP([m4@&address@hidden: circular dependencies])
+AT_KEYWORDS([m4@&address@hidden)
 
 AT_DATA_M4SUGAR([script.4s],
 [[m4_defun([foo], [m4_require([bar])])
@@ -427,6 +429,7 @@ AT_CLEANUP
 ## ---------------------- ##
 
 AT_SETUP([m4@&address@hidden: one-shot initialization])
+AT_KEYWORDS([m4@&address@hidden)
 AT_KEYWORDS([m4@&address@hidden m4@&address@hidden)
 
 AT_CHECK_M4SUGAR_TEXT([[
@@ -455,6 +458,107 @@ hello, again
 AT_CLEANUP
 
 
+## -------------------- ##
+## m4_require: nested.  ##
+## -------------------- ##
+
+AT_SETUP([m4@&address@hidden: nested])
+AT_KEYWORDS([m4@&address@hidden m4@&address@hidden)
+
+dnl From the m4sugar.m4 discourse: Require chains, top level
+AT_CHECK_M4SUGAR_TEXT([[dnl
+m4_defun([a], [[a]])dnl aka TEST2a
+m4_defun([b], [[b]m4_require([a])])dnl aka TEST3
+m4_defun([c], [[c]m4_require([b])])dnl aka TEST2b
+m4_defun([d], [[d]m4_require([a])m4_require([c])])dnl aka TEST1
+pre
+d
+d
+post
+]],
+[[pre
+a
+b
+c
+d
+d
+post
+]])
+
+dnl From the m4sugar.m4 discourse: Require chains, nested
+AT_CHECK_M4SUGAR_TEXT([[dnl
+m4_defun([a], [[a]])dnl aka TEST2a
+m4_defun([b], [[b]m4_require([a])])dnl aka TEST3
+m4_defun([c], [[c]m4_require([b])])dnl aka TEST2b
+m4_defun([d], [[d]m4_require([a])m4_require([c])])dnl aka TEST1
+m4_defun([wrap],
+[pre
+d
+d
+post])dnl
+wrap
+]],
+[[a
+b
+c
+pre
+d
+d
+post
+]])
+
+dnl Direct invocation, top level
+AT_CHECK_M4SUGAR_TEXT([[dnl
+m4_defun([a], [[a]])dnl
+m4_defun([b], [[b]m4_require([a])])dnl
+m4_defun([c], [[c]m4_require([b])])dnl
+pre
+a
+c
+a
+c
+post
+]],
+[[pre
+a
+b
+c
+a
+c
+post
+]])
+
+dnl Direct invocation, nested.  This is an example of expansion before
+dnl requirement, such that b occurs before its prerequisite a.  This
+dnl indicates a bug in the macros (but not in autoconf), so we should
+dnl be emitting a warning.
+AT_CHECK_M4SUGAR_TEXT([[dnl
+m4_defun([a], [[a]])dnl
+m4_defun([b], [[b]m4_require([a])])dnl
+m4_defun([c], [[c]m4_require([b])])dnl
+m4_defun([outer],
+[pre
+a
+c
+a
+c
+post])dnl
+outer
+]],
+[[b
+pre
+a
+c
+a
+c
+post
+]], [stderr])
+AT_XFAIL_IF([:])
+AT_CHECK([test -s stderr])
+
+AT_CLEANUP
+
+
 ## --------- ##
 ## m4_cond.  ##
 ## --------- ##
-- 
1.6.0.4