-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 According to Bruno Haible on 1/22/2009 5:31 AM: > Eric,
>> * modules/errno (configure.ac): Require, rather than expand, >> gl_HEADER_ERRNO_H. >> * m4/errno_h.m4 (gl_HEADER_ERRNO_H_BODY): Merge... >> (gl_HEADER_ERRNO_H): ...into this macro, and use AC_DEFUN_ONCE to >> enforce that all clients require it. > > This change is a step backwards. It makes the problem worse. I tend to disagree; reasoning below. At any rate, I don't see any reason to revert the gnulib patches unless/until autoconf does not warn on the cases that those patches resolved. > > So my and your workaround (in gl_HEADER_ERRNO_H and gl_MULTIARCH) was to > write and publish macros for which, when they are both invoked and > required, there is *no* problem. This is a good step because ultimately > it eases maintenance if we don't have to remember "this macro must only > be invoked" and "that macro must only be required". > > Your two patches now reintroduce the problem. But no, I don't want to > think about AC_REQUIRE's bug every time I want to invoke an autoconf macro! My argument is that if Autoconf can enforce rules, such as refusing to compile if you try to expand instead of require an AC_DEFUN_ONCE macro, then it is easy to fix your code if you choose the wrong method. Requiring then expanding for AC_DEFUN is generically ok (although you get redundant expansion, so the output is larger); the problem is only with expanding then requiring. > The question what to do with the warning then is of secondary nature. You can > - make it a non-default warning (like -Wshadow in gcc - not part of - -Wall), or No; the warning catches _actual_ bugs. For example, the gnulib bug with AC_GNU_SOURCE before AC_USE_SYSTEM_EXTENSIONS really did result in out-of-order expansion, as evidenced by the fact that after the patch, the expansion of AC_USE_SYSTEM_EXTENSIONS occurs earlier in the generated configure file. > - fix the warning to apply only to AC_DEFUN_ONCEd macros. After all, I used > AC_DEFUN, not AC_DEFUN_ONCE, to define gl_HEADER_ERRNO_H. Then why do you > emit a warning about it at all? Because the user's bug of expand-before-require affects both AC_DEFUN'd and AC_DEFUN_ONCE'd macros. The only thing that using AC_DEFUN_ONCE instead of AC_DEFUN adds is that you get an automatic warning, even with older autoconf, that a macro was expanded more than once. > - fix the warning to recognize macros which, other than for AC_REQUIRE > invocations, produce only whitespace output, or (better) It is next to impossible to efficiently figure out whether an m4 macro body consists of only whitespace and AC_REQUIRE. > > I wrote: >> - provide a documented way to avoid the warning for a particular macro, >> I much prefer to have a macro which can be both invoked and required, >> even if it requires one more line of code to write it down, than to have >> a macro with restricted use. > > Here's a concrete proposal: Introduce a new macro-defining-macro > AC_DEFUN_IDEMPOTENT, such that: > > - macros defined with AC_DEFUN_IDEMPOTENT are *known* to be expandable > any number of times, hence they may be both invoked and required, I suppose I could implement this for 2.64: m4_define([AC_DEFUN_IDEMPOTENT], [m4_set_add([_m4_idempotent], [$1])AC_DEFUN([$1], [$2])]) then make the warning conditional on whether the macro is a member of the set. But this definition won't port back to 2.63 or earlier, so gnulib would have to do something like this fallback: m4_ifndef([AC_DEFUN_IDEMPOTENT], [m4_define([AC_DEFUN_IDEMPOTENT], m4_defn([AC_DEFUN]))]) Meanwhile, you still have the scenario where a macro declared idempotent may result in duplicated output when using 2.64 (okay, since you declared it was idempotent), but silent out-of-order expansion when using 2.63 or earlier. Which means the onus is thus on the developer using AC_DEFUN_IDEMPOTENT to ensure that their macro consists only of whitespace and AC_REQUIRE, if they don't want to be impacted by expand-before-require bugs. But autoconf can't enforce that a macro declared idempotent was well-written. > > - macros defined with AC_DEFUN_ONCE are *known* to be expandable only > once, hence the recommendation for them is to AC_REQUIRE them, That is already the case, although I can further improve it in 2.64, by issuing a warning if such a macro was not invoked via AC_REQUIRE or expanded at the top level. But again, this improved warning is only available if you develop with autoconf 2.64, so it is still possible that developers using older autoconf will introduce bugs. > > - about macros defined with AC_DEFUN nothing is known. > > So for your warning about required-after-expanded it means: > - the warning should apply always to AC_DEFUN_ONCEd macros, To be clear, you are proposing multiple warnings. In this case, one warning is attached to any direct expansion of a defun_once macro inside a defun macro body, regardless of any other requires that have taken place: `macro' should be AC_REQUIRE'd inside AC_DEFUN (hmm, that gets tricky - there is currently no way to know whether the macro was declared via m4_defun or its synonym AC_DEFUN; with m4_require/AC_REQUIRE the solution is to look at $0, but with direct invocation of an AC_DEFUN_ONCE macro, we don't have context to make the warning message nice) then, since we already warned about direct expansion, any instance of an expand-before-require warning becomes noise for a defun_once macro. > - the warning should never apply to AC_DEFUN_IDEMPOTENTd macros, possible, but puts the onus on the user to use this style of macro definition wisely > - the warning should apply to AC_DEFUNd macros only when the warning > level is higher than the default. The warning should indicate that > either the double expansion is a problem, or the macro should be > changed to be defined with AC_DEFUN_IDEMPOTENT if it is not a problem. Here, the warning should always be issued. If double-expansion was not a problem, then AC_DEFUN_IDEMPOTENT will silence the warning. And if double-expansion IS a problem under 2.64, then you have a case where the code causes silent out-of-order expansion with earlier autoconf, in which case the warning is the right thing to do because it is a true bug in the user's macros. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -----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 iEYEARECAAYFAkl4d2cACgkQ84KuGfSFAYCO8gCfT+ZgbY54Z41hLC24iIxp92vR vFcAoL7mWChnHHoWpP/J0rNLRloZkW/N =4A0o -----END PGP SIGNATURE-----
