Eric Blake <ebb9 <at> byu.net> writes: > Here's my first draft of a patch. Unfortunately, there is probably existing > code that was expecting the old buggy behavior.
And here's my proposed patch that fixes those, by introducing a new macro that states when we want the old behavior (AS_IF, AS_CACHE_VAL, and maybe a small handful of other places) instead of the new. As a bonus, wrapping text in AC_REQUIRE_HOIST issues a warning on code that was previously triggering the buggy AC_REQUIRE behaivior. From: Eric Blake <[email protected]> Date: Tue, 30 Dec 2008 17:18:42 -0700 Subject: [PATCH] Introduce m4_require_hoist/AC_REQUIRE_HOIST. * lib/m4sugar/m4sugar.m4 (m4_require_hoist): New macro. (m4_require, m4_provide): Use it to restore some measure of backwards compatibility, when needed, but with safety checking. * lib/autoconf/general.m4 (AC_REQUIRE_HOIST): New macro. (AC_CACHE_VAL, AC_CACHE_CHECK): Perform prequisite hoisting. * lib/m4sugar/m4sh.m4 (AS_CASE, AS_FOR, AS_IF): Likewise. * doc/autoconf.texi (Prerequisite Macros) <AC_REQUIRE_HOIST>: Document new macro. * doc/autoconf.texi (Suggested Ordering) <AC_PROVIDE>: Document existing macro. * NEWS: Document this. Signed-off-by: Eric Blake <[email protected]> --- ChangeLog | 13 +++++++++ NEWS | 11 +++++++- doc/autoconf.texi | 70 +++++++++++++++++++++++++++++++++++++++++++++++ lib/autoconf/general.m4 | 31 +++++++++++++------- lib/m4sugar/m4sh.m4 | 23 +++++++++++---- lib/m4sugar/m4sugar.m4 | 25 +++++++++++++++-- 6 files changed, 152 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index e5f3872..e114407 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2008-12-30 Eric Blake <[email protected]> + Introduce m4_require_hoist/AC_REQUIRE_HOIST. + * lib/m4sugar/m4sugar.m4 (m4_require_hoist): New macro. + (m4_require, m4_provide): Use it to restore some measure of + backwards compatibility, when needed, but with safety checking. + * lib/autoconf/general.m4 (AC_REQUIRE_HOIST): New macro. + (AC_CACHE_VAL, AC_CACHE_CHECK): Perform prequisite hoisting. + * lib/m4sugar/m4sh.m4 (AS_CASE, AS_FOR, AS_IF): Likewise. + * doc/autoconf.texi (Prerequisite Macros) <AC_REQUIRE_HOIST>: + Document new macro. + * doc/autoconf.texi (Suggested Ordering) <AC_PROVIDE>: Document + existing macro. + * NEWS: Document this. + Fix output location of m4_defun->m4_defun->m4_require. This patch fails the testsuite, due to changed hoisting behavior. * lib/m4sugar/m4sugar.m4 (Defining macros): Update comments to diff --git a/NEWS b/NEWS index 99da48e..50cbd67 100644 --- a/NEWS +++ b/NEWS @@ -36,7 +36,10 @@ GNU Autoconf NEWS - User visible changes. This gives `A B PRE C POST' in older versions, and `PRE A B C POST' in this version; but since A and PRE don't have any dependency relation, both outputs should be equally correct (if they do have a - dependency relation, then you need an additional AC_REQUIRE). + dependency relation, then you should add AC_REQUIRE([a]) as part of + PRE, or wrap the body of outer with the new AC_REQUIRE_HOIST to get + the old behavior without the risk of depedencies issued out of + order). ** Autotest testsuites accept an option --jobs[=N] for parallel testing. @@ -53,6 +56,12 @@ GNU Autoconf NEWS - User visible changes. ** Autoreconf added aclocal to the set of programs affected by the `autoreconf -I dir' option. +** The following autoconf macros are documented now: + AC_PROVIDE + +** The following autoconf macros are new: + AC_REQUIRE_HOIST + ** The following documented m4sugar macros are new: m4_chomp m4_curry m4_default_quoted m4_esyscmd_s m4_map_args m4_map_args_pair m4_map_args_sep m4_map_args_w m4_set_map diff --git a/doc/autoconf.texi b/doc/autoconf.texi index d619578..dfc7242 100644 --- a/doc/autoconf.texi +++ b/doc/autoconf.texi @@ -13109,6 +13109,63 @@ Prerequisite Macros at the beginning of a macro. You can use @code{dnl} to avoid the empty lines they leave. +...@defmac AC_REQUIRE_HOIST (@var{text}) +...@acindex{require_hoist} +Expands @var{text} as normal, except that the current macro behaves as +if it called @code{AC_REQUIRE} on the same set of macros required by any +macros defined by @code{AC_DEFUN} and directly invoked (rather than +required) during the expansion of @var{text}. Issue a warning if, +during the course of @var{text}, a macro is first expanded and later +required, as that is a likely indicator that dependencies could not be +honored and the output is out of order. + +This macro was implemented in Autoconf 2.64, as part of fixing a bug in +...@code{ac_require}. All prior versions behave as if this macro were +always in effect, except that there is no warning when the +...@code{ac_require} bug outputs required macros in the wrong order. To +remove the warning, either avoid hoisting required macros, or add +...@code{ac_require([...@var{macro}])} at a point earlier in @var{text} than +where @var{macro} is directly expanded. + +There are very few places where this macro is actually needed. One use +is in the implementation of @code{AS_IF}, where it is desirable to hoist +the prerequisite macro expansions to occur before the overall if +statement, rather than hidden inside the branch of the conditional that +contains the macro call that needed the prerequisite. +...@end defmac + +Here is an example of how @code{AC_REQUIRE_HOIST} affects output. With +Autoconf 2.64 and newer, @code{MACRO1} and @code{MACRO2} have different +output order; with older Autoconf, the use of a preparation sequence +allows both invocations to match @code{MACRO2}. + +...@example +dnl Use this sequence to be portable to earlier autoconf versions. +m4_ifndef([AC_REQUIRE_HOIST], + [m4_define([AC_REQUIRE_HOIST], [$1])])dnl +AC_DEFUN([A1], [[in A1]])dnl +AC_DEFUN([B1], [[in B1]AC_REQUIRE([A1])])dnl +AC_DEFUN([MACRO1], [PRE +B1 +POST])dnl +MACRO1 +...@result{}pre +...@result{}a1 +...@result{}b1 +...@result{}post + +AC_DEFUN([A2], [[in A2]])dnl +AC_DEFUN([B2], [[in B2]AC_REQUIRE([A2])])dnl +AC_DEFUN([MACRO2], [AC_REQUIRE_HOIST([PRE +B2 +POST])])dnl +MACRO2 +...@result{}a1 +...@result{}pre +...@result{}b1 +...@result{}post +...@end example + @node Suggested Ordering @subsection Suggested Ordering @cindex Macros, ordering @@ -13149,6 +13206,19 @@ Suggested Ordering that it has been called. @end defmac +...@defmac AC_PROVIDE (@var{this-macro-name}) +...@acindex{provide} +Declares that @var{this-macro-name} has been provided, which affects the +behavior of @code{AC_REQUIRE} and @code{AC_BEFORE} when checking for +...@var{this-macro-name}. Normally, this is not needed, since +...@code{ac_defun} automatically provides the current macro. But sometimes +it is desirable to have at most one out of a set of multiple +initialization options executed; by giving the set a unique name, and +making sure each initialization option provides that name, then it is +possible to check that the single name has been provided rather than +iterating through all the options. +...@end defmac + @node One-Shot Macros @subsection One-Shot Macros @cindex One-shot macros diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4 index bcf8720..65db81b 100644 --- a/lib/autoconf/general.m4 +++ b/lib/autoconf/general.m4 @@ -171,16 +171,18 @@ m4_copy([m4_divert_pop], [AC_DIVERT_POP]) # AC_DEFUN(NAME, EXPANSION) # AC_DEFUN_ONCE(NAME, EXPANSION) # AC_BEFORE(THIS-MACRO-NAME, CALLED-MACRO-NAME) -# AC_REQUIRE(STRING) +# AC_REQUIRE(MACRO-NAME) +# AC_REQUIRE_HOIST(TEXT) # AC_PROVIDE(MACRO-NAME) # AC_PROVIDE_IFELSE(MACRO-NAME, IF-PROVIDED, IF-NOT-PROVIDED) # ----------------------------------------------------------- -m4_copy([m4_defun], [AC_DEFUN]) -m4_copy([m4_defun_once], [AC_DEFUN_ONCE]) -m4_copy([m4_before], [AC_BEFORE]) -m4_copy([m4_require], [AC_REQUIRE]) -m4_copy([m4_provide], [AC_PROVIDE]) -m4_copy([m4_provide_if], [AC_PROVIDE_IFELSE]) +m4_copy([m4_defun], [AC_DEFUN]) +m4_copy([m4_defun_once], [AC_DEFUN_ONCE]) +m4_copy([m4_before], [AC_BEFORE]) +m4_copy([m4_require], [AC_REQUIRE]) +m4_copy([m4_require_hoist],[AC_REQUIRE_HOIST]) +m4_copy([m4_provide], [AC_PROVIDE]) +m4_copy([m4_provide_if], [AC_PROVIDE_IFELSE]) # AC_OBSOLETE(THIS-MACRO-NAME, [SUGGESTION]) @@ -1991,8 +1993,11 @@ rm -f confcache[]dnl # AC_CACHE_VAL(CACHE-ID, COMMANDS-TO-SET-IT) # ------------------------------------------ # The name of shell var CACHE-ID must contain `_cv_' in order to get saved. -# Should be dnl'ed. Try to catch common mistakes. +# Should be dnl'ed. Try to catch common mistakes. Additionally, hoist +# any AC_REQUIRE to occur before the cache check, and issue an error if +# this is not possible due to a missing AC_REQUIRE. m4_defun([AC_CACHE_VAL], +[m4_require_hoist( [AS_LITERAL_IF([$1], [m4_if(m4_index(m4_quote($1), [_cv_]), [-1], [AC_DIAGNOSE([syntax], [$0($1, ...): suspicious cache-id, must contain _cv_ to be cached])])])dnl @@ -2007,20 +2012,24 @@ m4_if(m4_index([$2], [AC_SUBST]), [-1], [], AS_VAR_SET_IF([$1], [_AS_ECHO_N([(cached) ])], [$2]) -]) +])]) # AC_CACHE_CHECK(MESSAGE, CACHE-ID, COMMANDS) # ------------------------------------------- +# Wrap a cache check with MESSAGE and the result of the check. +# Additionally, hoist any AC_REQUIRE to occur before this macro, or issue +# an error if it is not possible. +# # Do not call this macro with a dnl right behind. m4_defun([AC_CACHE_CHECK], -[AC_MSG_CHECKING([$1]) +[m4_require_hoist([AC_MSG_CHECKING([$1]) AC_CACHE_VAL([$2], [$3])dnl AS_LITERAL_IF([$2], [AC_MSG_RESULT([$$2])], [AS_VAR_COPY([ac_res], [$2]) AC_MSG_RESULT([$ac_res])])dnl -]) +])]) # _AC_CACHE_CHECK_INT(MESSAGE, CACHE-ID, EXPRESSION, # [PROLOGUE = DEFAULT-INCLUDES], [IF-FAILS]) diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4 index 358aa81..b042fb3 100644 --- a/lib/m4sugar/m4sh.m4 +++ b/lib/m4sugar/m4sh.m4 @@ -523,6 +523,10 @@ _AS_UNSET_PREPARE # realize the impacts of using insufficient m4 quoting. This macro # always provides a default case, to work around a Solaris /bin/sh # bug regarding the exit status when no case matches. +# +# Additionally, hoist any required macros occuring in the expansion of +# the arguments to occur prior to the AS_IF expansion, or issue an +# error alerting the user of a missing AC_REQUIRE. m4_define([_AS_CASE], [ [...@%:@(] $1[)] m4_default([$2], [:]) ;;]) @@ -531,9 +535,9 @@ m4_define([_AS_CASE_DEFAULT], *[)] m4_default([$1], [:]) ;;]) m4_defun([AS_CASE], -[case $1 in[]m4_map_args_pair([_$0], [_$0_DEFAULT], +[m4_require_hoist([case $1 in[]m4_map_args_pair([_$0], [_$0_DEFAULT], m4_shift($...@m4_if(m4_eval([$# & 1]), [1], [,]))) -esac])# AS_CASE +esac])])# AS_CASE # _AS_EXIT_PREPARE @@ -581,6 +585,10 @@ m4_defun([AS_EXIT], # that element, thus providing a direct value rather than a shell # variable indirection. # +# Additionally, hoist any required macros occuring in the expansion of +# the arguments to occur prior to the AS_IF expansion, or issue an +# error alerting the user of a missing AC_REQUIRE. +# # Only use the optimization if LIST can be used without additional # shell quoting in either a literal or double-quoted context (that is, # we give up on default IFS chars, parameter expansion, command @@ -589,10 +597,10 @@ m4_defun([AS_EXIT], m4_defun([AS_FOR], [m4_pushdef([$1], m4_if([$3], [], [[$$2]], m4_translit([$3], ]dnl m4_dquote(_m4_defn([m4_cr_symbols2]))[[%+=:,./-]), [], [[$3]], [[$$2]]))]dnl -[for $2[]m4_ifval([$3], [ in $3]) +[m4_require_hoist([for $2[]m4_ifval([$3], [ in $3]) do m4_default([$4], [:]) -done[]_m4_popdef([$1])]) +done[]_m4_popdef([$1])])]) # AS_IF(TEST1, [IF-TRUE1 = :]...[IF-FALSE = :]) @@ -608,6 +616,9 @@ done[]_m4_popdef([$1])]) # | fi # with simplifications if IF-TRUE1 and/or IF-FALSE is empty. # +# Additionally, hoist any required macros occuring in the expansion of +# the arguments to occur prior to the AS_IF expansion, or issue an +# error alerting the user of a missing AC_REQUIRE. m4_define([_AS_IF], [elif $1; then m4_default([$2], [:]) @@ -618,10 +629,10 @@ m4_define([_AS_IF_ELSE], $1])]) m4_defun([AS_IF], -[if $1; then +[m4_require_hoist([if $1; then m4_default([$2], [:]) m4_map_args_pair([_$0], [_$0_ELSE], m4_shift2($@))]dnl -[fi[]])# AS_IF +[fi[]])])# AS_IF # AS_SET_STATUS(STATUS) diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4 index 825001c..d30c4ca 100644 --- a/lib/m4sugar/m4sugar.m4 +++ b/lib/m4sugar/m4sugar.m4 @@ -2104,8 +2104,10 @@ m4_define([m4_require], [m4_ifdef([_m4_divert_grow], [], [m4_fatal([$0($1): cannot be used outside of an ]dnl m4_bmatch([$0], [^AC_], [[AC_DEFUN]], [[m4_defun]])['d macro])])]dnl -[m4_provide_if([$1], [], - [_m4_require_call([$1], [$2], _m4_divert_grow)])]) +[m4_provide_if([$1], [m4_set_contains([_m4_hoist_provide], [$1], + [m4_warn([m4_require_hoist: $1 was provided before it was required])])], + [_m4_require_call([$1], [$2], m4_ifdef([_m4_hoisting], [_m4_hoisting], + [_m4_divert_grow]))m4_set_remove([_m4_hoist_provide], [$1])])]) # _m4_require_call(NAME-TO-CHECK, [BODY-TO-EXPAND = NAME-TO-CHECK], DIVERSION) @@ -2124,6 +2126,22 @@ m4_provide_if([$1], [], [m4_warn([syntax], [m4_divert_pop($3)]) +# m4_require_hoist(TEXT) +# ---------------------- +# Expand TEXT, except that any m4_require done during TEXT is hoisted +# to also be a requirement of the current defun'd macro. In order to +# prevent subtle bugs, an error is issued if a previously unprovided +# macro name is first provided and then required within TEXT. +m4_define([m4_require_hoist], +[m4_ifdef([_m4_divert_grow], [], + [m4_fatal([$0: cannot be used outside of an ]dnl +m4_if([$0], [m4_require_hoist], [[m4_defun]], [[AC_DEFUN]])['d macro])])]dnl +[m4_pushdef([_m4_hoisting], m4_ifdef([_m4_hoisting], [_m4_hoisting], + [_m4_divert_grow]))$1[]]dnl +[_m4_popdef([_m4_hoisting])m4_ifdef([_m4_hoisting], [], + [m4_set_delete([_m4_hoist_provide])])]) + + # m4_expand_once(TEXT, [WITNESS = TEXT]) # -------------------------------------- # If TEXT has never been expanded, expand it *here*. Use WITNESS as @@ -2138,7 +2156,8 @@ m4_define([m4_expand_once], # ---------------------- # Declare that MACRO-NAME has been provided. m4_define([m4_provide], -[m4_define([m4_provide($1)])]) +[m4_provide_if([$1], [], [m4_ifdef([_m4_hoisting], + [m4_set_add([_m4_hoist_provide], [$1])])m4_define([m4_provide($1)])])]) # m4_provide_if(MACRO-NAME, IF-PROVIDED, IF-NOT-PROVIDED) -- 1.6.0.4
