Eric Blake <ebb9 <at> byu.net> writes: > There are a couple > more problems in m4sh itself (for example, _AS_SHELL_SANITIZE expands > _AS_PATH_SEPARATOR_PREPARE then requires it via _AS_PATH_WALK), but I'm still > trying to come up with the cleanest fixes for those.
It turns out that the idiom: m4_defun([a],[A]) m4_defun([b],[m4_require([a])B]) m4_defun([c],[a b]) is rather common (autoconf, automake, and gnulib all triggered it, and gnulib in multiple places), but is also harmless (you can't get out-of-order expansion from an ac_require until you have _nested_ ac_require). So I reworked my patch to recognize this case and avoid treating it as a false positive, which means I no longer have to worry about _AS_SHELL_SANITIZE doing a expand-before-require of _AS_PATH_SEPARATOR_PREPARE. Instead, we can focus directly on the true expand-before-indirect-require cases where there really is out-of-order expansion (gnulib has some of those still, but not autoconf or automake). Here's the current state of my two patches; the second one implements Bruno's idea that any time we issue the expand-before-require message, we can also avoid the out-of-order expansion by merely doing a duplicate expansion. Or, put another way, the canonical example: m4_defun([a],[A]) m4_defun([b],[m4_require([a])B]) m4_defun([c],[m4_require([b])C]) m4_defun([d],[D a c]) now results in a warning (because a was expanded twice, where it earlier autoconf only expanded it once), and the following _corrected_ output: A B D A C >From 525797c1fc204bba2c83928b0e06239ea5c9d3ff Mon Sep 17 00:00:00 2001 From: Eric Blake <[email protected]> Date: Tue, 20 Jan 2009 14:03:59 -0700 Subject: [PATCH] Warn if macro is provided before indirectly required. * lib/m4sugar/m4sugar.m4 (m4_provide): Track the set of all macros provided since last outermost defun. (_m4_defun_pro_outer): Empty the set. (m4_require): Check the set, in order to warn. (_m4_require_call): Distinguish between direct and indirect requires. * tests/m4sugar.at (m4@&t...@_require: nested): Remove xfail, and add test case for direct requires. Signed-off-by: Eric Blake <[email protected]> --- ChangeLog | 12 ++++++++++ lib/m4sugar/m4sugar.m4 | 14 ++++++++--- tests/m4sugar.at | 57 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6023598..c348e6b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,17 @@ 2009-01-20 Eric Blake <[email protected]> + Warn if macro is provided before indirectly required. + * lib/m4sugar/m4sugar.m4 (m4_provide): Track the set of all macros + provided since last outermost defun. + (_m4_defun_pro_outer): Empty the set. + (m4_require): Check the set, in order to warn. + (_m4_require_call): Distinguish between direct and indirect + requires. + * tests/m4sugar.at (m4@&t...@_require: nested): Remove xfail, and add + test case for direct requires. + +2009-01-20 Eric Blake <[email protected]> + Clean up some bugs caught by preliminary dependency validation. * lib/autoconf/headers.m4 (AC_DIR_HEADER): Don't invoke AC_HEADER_DIRENT, since AC_FUNC_CLOSEDIR_VOID requires it. diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4 index 3b28512..04ddc1b 100644 --- a/lib/m4sugar/m4sugar.m4 +++ b/lib/m4sugar/m4sugar.m4 @@ -1772,6 +1772,7 @@ m4_define([_m4_defun_pro], [m4_expansion_stack_push([$1])m4_pushdef([_m4_expanding($1)])]) m4_define([_m4_defun_pro_outer], +[m4_set_delete([_m4_provide])]dnl [m4_pushdef([_m4_divert_dump], m4_divnum)m4_divert_push([GROW])]) # _m4_defun_epi(MACRO-NAME) @@ -1935,8 +1936,11 @@ m4_define([m4_require], [m4_if(_m4_divert_dump, [], [m4_fatal([$0($1): cannot be used outside of an ]dnl m4_if([$0], [m4_require], [[m4_defun]], [[AC_DEFUN]])['d macro])])]dnl -[m4_provide_if([$1], [], - [_m4_require_call([$1], [$2], _m4_divert_dump)])]) +[m4_provide_if([$1], [m4_ifdef([_m4_require], + [m4_set_contains([_m4_provide], [$1], + [m4_warn([syntax], [$0: `$1' was expanded before it was required])])])], + [_m4_require_call([$1], [$2], _m4_divert_dump)m4_set_remove( + [_m4_provide], [$1])])]) # _m4_require_call(NAME-TO-CHECK, [BODY-TO-EXPAND = NAME-TO-CHECK], @@ -1949,12 +1953,13 @@ m4_if([$0], [m4_require], [[m4_defun]], [[AC_DEFUN]]) ['d macro])])]dnl # by avoiding dnl and other overhead on the common path. m4_define([_m4_require_call], [m4_pushdef([_m4_divert_grow], m4_decr(_m4_divert_grow))]dnl +[m4_pushdef([_m4_require])]dnl [m4_divert_push(_m4_divert_grow)]dnl [m4_if([$2], [], [$1], [$2]) m4_provide_if([$1], [], [m4_warn([syntax], [$1 is m4_require'd but not m4_defun'd])])]dnl [_m4_divert_raw($3)_m4_undivert(_m4_divert_grow)]dnl -[m4_divert_pop(_m4_divert_grow)_m4_popdef([_m4_divert_grow])]) +[m4_divert_pop(_m4_divert_grow)_m4_popdef([_m4_divert_grow], [_m4_require])]) # _m4_divert_grow @@ -1976,7 +1981,8 @@ m4_define([m4_expand_once], # m4_provide(MACRO-NAME) # ---------------------- m4_define([m4_provide], -[m4_define([m4_provide($1)])]) +[m4_ifdef([m4_provide($1)], [], +[m4_set_add([_m4_provide], [$1], [m4_define([m4_provide($1)])])])]) # m4_provide_if(MACRO-NAME, IF-PROVIDED, IF-NOT-PROVIDED) diff --git a/tests/m4sugar.at b/tests/m4sugar.at index e822ffb..a6739cf 100644 --- a/tests/m4sugar.at +++ b/tests/m4sugar.at @@ -507,7 +507,7 @@ d post ]]) -dnl Direct invocation, top level +dnl Direct invocation, nested requires, top level AT_CHECK_M4SUGAR_TEXT([[dnl m4_defun([a], [[a]])dnl m4_defun([b], [[b]m4_require([a])])dnl @@ -528,10 +528,10 @@ 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. +dnl Direct invocation, nested requires, nested defun. This is an example +dnl of expansion before requirement, such that b occurs before its +dnl prerequisite a. This indicates a bug in the macros (but not in +dnl autoconf), so we should be emitting a warning. AT_CHECK_M4SUGAR_TEXT([[dnl m4_defun([a], [[a]])dnl m4_defun([b], [[b]m4_require([a])])dnl @@ -552,9 +552,50 @@ c a c post -]], [stderr]) -AT_XFAIL_IF([:]) -AT_CHECK([test -s stderr]) +]], +[[script.4s:14: warning: m4@&t...@_require: `a' was expanded before it was required +script.4s:5: b is expanded from... +script.4s:6: c is expanded from... +script.4s:7: outer is expanded from... +script.4s:14: the top level +]]) + +dnl Direct invocation, expand-before-require but no nested require. As this +dnl is common in real life, but does not result in out-of-order expansion, +dnl we silently permit this. +AT_CHECK_M4SUGAR_TEXT([[dnl +m4_defun([a], [[a]])dnl +m4_defun([b], [[b]m4_require([a])])dnl +m4_defun([c], [[c]])dnl +m4_defun([d], [[d]m4_require([c])])dnl +pre1 +a +b +a +b +post1 +m4_defun([outer], +[pre2 +c +d +c +d +post2])dnl +outer +]], +[[pre1 +a +b +a +b +post1 +pre2 +c +d +c +d +post2 +]]) AT_CLEANUP -- 1.6.0.4 >From 08fbde77efa94c161aa91325fabeef50474e2bc9 Mon Sep 17 00:00:00 2001 From: Eric Blake <[email protected]> Date: Tue, 20 Jan 2009 14:22:41 -0700 Subject: [PATCH] Fix out-of-order expansion with expand-before-require. * lib/m4sugar/m4sugar.m4 (m4_require): Redundantly expand a required macro when issuing expand-before-require warning. * tests/m4sugar.at (m4@&t...@_require: nested): Adjust test. Suggested by Bruno Haible. Signed-off-by: Eric Blake <[email protected]> --- ChangeLog | 6 ++++++ lib/m4sugar/m4sugar.m4 | 8 ++++---- tests/m4sugar.at | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index c348e6b..0d9549e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-01-20 Eric Blake <[email protected]> + Fix out-of-order expansion with expand-before-require. + * lib/m4sugar/m4sugar.m4 (m4_require): Redundantly expand a + required macro when issuing expand-before-require warning. + * tests/m4sugar.at (m4@&t...@_require: nested): Adjust test. + Suggested by Bruno Haible. + Warn if macro is provided before indirectly required. * lib/m4sugar/m4sugar.m4 (m4_provide): Track the set of all macros provided since last outermost defun. diff --git a/lib/m4sugar/m4sugar.m4 b/lib/m4sugar/m4sugar.m4 index 04ddc1b..57b4e57 100644 --- a/lib/m4sugar/m4sugar.m4 +++ b/lib/m4sugar/m4sugar.m4 @@ -1937,10 +1937,10 @@ m4_define([m4_require], [m4_fatal([$0($1): cannot be used outside of an ]dnl m4_if([$0], [m4_require], [[m4_defun]], [[AC_DEFUN]])['d macro])])]dnl [m4_provide_if([$1], [m4_ifdef([_m4_require], - [m4_set_contains([_m4_provide], [$1], - [m4_warn([syntax], [$0: `$1' was expanded before it was required])])])], - [_m4_require_call([$1], [$2], _m4_divert_dump)m4_set_remove( - [_m4_provide], [$1])])]) + [m4_set_contains([_m4_provide], [$1], [m4_warn([syntax], + [$0: `$1' was expanded before it was required])_m4_require_call], + [m4_ignore])], [m4_ignore])], [_m4_require_call])([$1], [$2], + _m4_divert_dump)m4_set_remove([_m4_provide], [$1])]) # _m4_require_call(NAME-TO-CHECK, [BODY-TO-EXPAND = NAME-TO-CHECK], diff --git a/tests/m4sugar.at b/tests/m4sugar.at index a6739cf..613a3c9 100644 --- a/tests/m4sugar.at +++ b/tests/m4sugar.at @@ -545,7 +545,8 @@ c post])dnl outer ]], -[[b +[[a +b pre a c -- 1.6.0.4
