Re: Fix an 8-year-old AC_REQUIRE bug

[Eric Blake]

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

According to Paolo Bonzini on 12/31/2008 2:10 AM:
>> 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.
> 
> IMHO, this can wait for 2.65... we changed enough m4sugar internals in
> 2.64, and all of them were in a backwards-compatible way.  If we stick
> to that policy, understanding regressions will be much easier.

I've polished up the patch so that it now passes the autoconf testsuite;
you can preview it here (this branch is subject to rebasing, though):
$ git fetch git://repo.or.cz/autoconf/ericb.git m4-require
http://repo.or.cz/w/autoconf/ericb.git?a=shortlog;h=refs/heads/m4-require

But I am starting to agree with you that this particular version of the
patch is too invasive, at least for 2.64.  It is flushing out real issues;
for example, when used on coreutils, I noticed that the macro
AC_PROG_GCC_TRADITIONAL invokes:
if ...
  AC_EGREP_CPP
fi
which, under the patch's new behavior, means that the check for egrep is
no longer hoisted outside of the if unless I rewrite
AC_PROG_GCC_TRADITIONAL to use AS_IF instead of raw if, or if I change it
to use my new AC_REQUIRE_HOIST.  But that just means that _lots_ of other
uses in the wild have come to depend on this functionality, namely, that
m4_require'd text is hoisted to the front of the _outermost_ defun'd
macro.  Or put another way, it's a feature, not a bug, and Bruno's
original post is an example of a mis-use of the feature.

So, I'm now starting to look at a different approach, using knowledge
learned from the previous attempt.  If nothing else, I hope to at least be
able to make autoconf warn when, during the course of a single outermost
defun'd macro, another macro is first provided and later required.  That
will be safe for 2.64, with no change in current ordering but adding a way
to identify the problems.  With regards to my simplified testcase:

m4_defun([a],[[a]])
m4_defun([b],[[b]m4_require([a])])
m4_defun([c],[[c]m4_require([b])])
m4_defun([outer], [pre c post])

this would mean that autoconf issues a warning that [a] was provided
before it was required, and that the user should add m4_require([a]) as
part of the prefix of [outer], converting 'b pre a c post' to 'a b pre c
post' (changing things to emit 'pre a b c post' is what my current patch
does, but it is too risky because of shell conditionals that were written
without AS_IF).

But perhaps I can do one better; if I can detect problems, perhaps I can
find a way to _also_ hoist them.  Maybe something along the following lines:

make the m4_provide mechanism checkpointable, where macros provided after
the last checkpoint can be rolled back to an unprovided state (good in its
own right, since m4_expand already wants to be able to do rollbacks when
parsing unbalanced ')')

rewrite _m4_defun_pro/epi to establish a checkpoint before every outermost
invocation of a defun'd macro, and if any provide-before-require is found,
then discard the entire GROW diversion, unprovide all symbols detected
during the expansion, then re-expand a hoisted m4_require followed by the
original defun'd body

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

iEYEARECAAYFAklbdwcACgkQ84KuGfSFAYBJhQCfWCctN/2YX8JcMD5G/YVGs2wM
zMkAoMqKzFkdMganUQmz+zRUmG/3E/EV
=ALKA
-----END PGP SIGNATURE-----