Re: autoconf 2.63b: wrong macro expansion order

[Eric Blake]

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

[adding autoconf-patches]

According to Eric Blake on 4/6/2009 6:46 AM:
> According to Andreas Schwab on 4/5/2009 9:22 AM:
>> >From the POV of the macro writer the 2.63 behaviour does not look like a
>> bug.  Basically the following two snippets:
> 
>>     AC_DEFUN([FOO],[AC_COMPILE_IFELSE([])])
>>     foobar=
>>     AC_PROG_CC
>>     FOO
> 
>> and
> 
>>     AC_DEFUN([FOO],[AC_COMPILE_IFELSE([])])
>>     AC_DEFUN([BAR],[
>>     foobar=
>>     AC_PROG_CC
>>     FOO])
>>     BAR
> 
>> are expected to be completely equivalent.
> 
> Unfortunately, that expectation is wrong, because of the existence of
> AC_REQUIRE.  At this point, I think the only thing I can do is add some
> documentation, in three places:

Or even in four.  Any comments on this proposed patch?  Especially for the
'Required Before Expanded' node - we want to make sure it reads easily
enough to point other users to that node when they encounter problems.  I
stand by my ground that we really did fix an eight-year-old bug, but I am
also aware that we haven't heard the last from packages that happened to
be successfully exploiting the bug and which will need help in fixing
their code to be more robust.

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

iEYEARECAAYFAknfsSgACgkQ84KuGfSFAYA9kQCbB2oSi0bCDJZw3fJxhhakp8/M
BJAAn3p6wZtdx8ztAzlS/3vqtTit+6ZP
=qppL
-----END PGP SIGNATURE-----

>From 9949c50de4aee76c9b8863057ac080a44ff66f70 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 10 Apr 2009 14:43:33 -0600
Subject: [PATCH] Improve documentation related to expanded-before-required.

* doc/autoconf.texi (Expanded Before Required): Add a case study.
(Running the Compiler) <AC_COMPILE_IFELSE>: Remind users that
running a compile test will AC_REQUIRE the compiler check.
(Macro Definitions) <AC_DEFUN>: Contrast AC_DEFUN and m4_define.
(C Compiler) <AC_PROG_CC>: Mention the fact that only first
invocation of this macro checks for $EXEEXT, and that many other
macros use it via AC_REQUIRE.
Reported by Andreas Schwab.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |   12 +++++
 doc/autoconf.texi |  124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0e18a41..23f55b8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2009-04-10  Eric Blake  <address@hidden>

+       Improve documentation related to expanded-before-required.
+       * doc/autoconf.texi (Expanded Before Required): Add a case study.
+       (Running the Compiler) <AC_COMPILE_IFELSE>: Remind users that
+       running a compile test will AC_REQUIRE the compiler check.
+       (Macro Definitions) <AC_DEFUN>: Contrast AC_DEFUN and m4_define.
+       (C Compiler) <AC_PROG_CC>: Mention the fact that only first
+       invocation of this macro checks for $EXEEXT, and that many other
+       macros use it via AC_REQUIRE.
+       Reported by Andreas Schwab.
+
+2009-04-10  Eric Blake  <address@hidden>
+
        Test parallel handling of syntax error.
        * tests/autotest.at (parallel syntax error): New test.
        Suggested by Ralf Wildenhues.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 32855a9..9848595 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -6737,6 +6737,15 @@ C Compiler
 it to @option{-g -O2} for the @acronym{GNU} C compiler (@option{-O2} on systems
 where @acronym{GCC} does not accept @option{-g}), or @option{-g} for
 other compilers.
+
+Many Autoconf macros use a compiler, and thus call
address@hidden([AC_PROG_CC])} to ensure that the compiler has been
+determined before the body of the outermost @code{AC_DEFUN} macro.
+Although @code{AC_PROG_CC} is safe to directly expand multiple times, it
+performs certain checks (such as the proper value of @env{EXEEXT}) only
+on the first invocation.  Therefore, care must be used when invoking
+this macro from within another macro rather than at the top level
+(@pxref{Expanded Before Required}).
 @end defmac

 @anchor{AC_PROG_CC_C_O}
@@ -8371,6 +8380,12 @@ Running the Compiler
 @code{AC_MSG_FAILURE}.  This macro does not try to link; use
 @code{AC_LINK_IFELSE} if you need to do that (@pxref{Running the
 Linker}).
+
+This macro uses @code{AC_REQUIRE} for the compiler associated with the
+current language, which means that if the compiler has not yet been
+determined, the compiler determination will be made prior to the body of
+the outermust @code{AC_DEFUN} macro that triggered this macro to
+expand (@pxref{Expanded Before Required}).
 @end defmac

 @ovindex ERL
@@ -12945,6 +12960,24 @@ Macro Definitions
 @code{dnl} is more useful to get rid of the newlines following macros
 that produce no output, such as @code{AC_REQUIRE}.

+Public third-party macros need to use @code{AC_DEFUN}, and not
address@hidden, in order to be found by @command{aclocal}
+(@pxref{Extending aclocal,,, automake, @acronym{GNU} Automake}).
+Additionally, if it is ever determined that a macro should be made
+obsolete, it is easy to convert from @code{AC_DEFUN} to @code{AU_DEFUN}
+in order to have @command{autoupdate} assist the user in choosing a
+better alternative, but there is no corresponding way to make
address@hidden issue an upgrade notice (@pxref{AU_DEFUN}).
+
+There is another subtle, but important, difference between using
address@hidden and @code{AC_DEFUN}: only the former is unaffected by
address@hidden  When writing a file, it is always safe to replace a
+block of text with a @code{m4_define} macro that will expand to the same
+text.  But replacing a block of text with an @code{AC_DEFUN} macro with
+the same content does not necessarily give the same results, because it
+changes the location where any embedded but unsatisfied
address@hidden invocations within the block will be expanded.  For an
+example of this, see @ref{Expanded Before Required}.

 @node Macro Names
 @section Macro Names
@@ -23031,6 +23064,97 @@ Expanded Before Required
 environment, then the macro should always be directly expanded instead
 of required.

+For another case study, consider this example trimmed down from an
+actual package.  Originally, the package contained shell code and
+multiple macro invocations at the top level of @file{configure.ac}:
+
address@hidden
+AC_DEFUN([FOO], [AC_COMPILE_IFELSE(address@hidden)])
+foobar=
+AC_PROG_CC
+FOO
address@hidden example
+
address@hidden
+but that was getting complex, so the author wanted to offload some of
+the text into a new macro in another file included via
address@hidden  The na@"ive approach merely wraps the text in a new
+macro:
+
address@hidden
+AC_DEFUN([FOO], [AC_COMPILE_IFELSE(address@hidden)])
+AC_DEFUN([BAR], [
+foobar=
+AC_PROG_CC
+FOO
+])
+BAR
address@hidden example
+
address@hidden
+With older versions of Autoconf, the setting of @samp{foobar=} occurs
+before the single compiler check, as the author intended.  But with
+Autoconf 2.64, this issues the ``expanded before it was required''
+warning for @code{AC_PROG_CC}, and outputs two copies of the compiler
+check, one before @samp{foobar=}, and one after.  To understand why this
+is happening, remember that the use of @code{AC_COMPILE_IFELSE} includes
+a call to @code{AC_REQUIRE([AC_PROG_CC])} under the hood.  According to
+the documented semantics of @code{AC_REQUIRE}, this means that
address@hidden @emph{must} occur before the body of the outermost
address@hidden, which in this case is @code{BAR}, thus preceeding the
+use of @samp{foobar=}.  The older versions of Autoconf were broken with
+regards to the rules of @code{AC_REQUIRE}, which explains why the code
+changed from one over to two copies of @code{AC_PROG_CC} when upgrading
+autoconf.  In other words, the author was unknowingly relying on a bug
+exploit to get the desired results, and that exploit broke once the bug
+was fixed.
+
+So, what recourse does the author have, to restore their intended
+semantics of setting @samp{foobar=} prior to a single compiler check,
+regardless of whether Autoconf 2.63 or 2.64 is used?  One idea is to
+remember that only @code{AC_DEFUN} is impacted by @code{AC_REQUIRE};
+there is always the possibility of using the lower-level
address@hidden:
+
address@hidden
+AC_DEFUN([FOO], [AC_COMPILE_IFELSE(address@hidden)])
+m4_define([BAR], [
+foobar=
+AC_PROG_CC
+FOO
+])
+BAR
address@hidden example
+
address@hidden
+This works great if everything is in the same file.  However, it does
+not help in the case where the author wants to have @command{aclocal}
+find the definition of @code{BAR} from its own file, since
address@hidden requires the use of @code{AC_DEFUN}.  In this case, a
+better fix is to recognize that if @code{BAR} also uses
address@hidden, then there will no longer be direct expansion prior
+to a subsequent require.  Then, by creating yet another helper macro,
+the author can once again guarantee a single invocation of
address@hidden, which will still occur after @code{foobar=}.  The
+author can also use @code{AC_BEFORE} to make sure no other macro
+appearing before @code{BAR} has triggered an unwanted expansion of
address@hidden
+
address@hidden
+AC_DEFUN([FOO], [AC_COMPILE_IFELSE(address@hidden)])
+AC_DEFUN([BEFORE_CC], [
+foobar=
+])
+AC_DEFUN([BAR], [
+AC_BEFORE([$0], [AC_PROG_CC])dnl
+AC_REQUIRE([BEFORE_CC])dnl
+AC_REQUIRE([AC_PROG_CC])dnl
+FOO
+])
+BAR
address@hidden example
+
+
 @c ===================================================== History of Autoconf.

 @node History
-- 
1.6.1.2