autoconf-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] use m4_require to implement AS_REQUIRE


From: Eric Blake
Subject: Re: [PATCH] use m4_require to implement AS_REQUIRE
Date: Mon, 13 Oct 2008 06:34:00 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/20080914 Thunderbird/2.0.0.17 Mnenhy/0.7.5.666

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

According to Paolo Bonzini on 10/12/2008 10:20 AM:
> Hi, I know I should be writing testcases for AS_ME_PREPARE. :-) But
> while I was looking at adding shell functions to Autoconf proper, I
> decided that the current basic implementation of AS_REQUIRE is not
> scalable.

True on both points.  ;-)

> I chose not to add a third argument to m4_require, as I did for
> AS_REQUIRE, because m4_require is on an expensive path.  Should
> m4_divert_require be documented?

We can leave it undocumented for a release, until we're happy with it.
For that matter, a lot of m4sugar diversion magic is (intentionally)
underdocumented, since we should generally be providing sane wrapper
macros that take care of the diversions under the hood, rather than
forcing the user to directly think about diversions.

> 
> I have not run the entire testsuite (I will before committing) but I
> have run the m4sugar and m4sh testsuites, as well as tests related to
> the shell functions I already added.  While testing, I noticed the effect
> of the speedups introduced after 2.61; on GNU Smalltalk I have:
> 
>    ver      usertime   size
>    2.61     18.154s    1037578
>    2.63     13.192s    1055693
> 
>    +funcs1  13.907s    886574
>    +funcs2  11.467s    780238

I take it these are speedups in 'autoconf' time, and not in 'configure' time?

> In the meanwhile, ok to apply after proper testing?

I haven't looked closely at contents yet; please give me a bit more time
to properly review before applying.  But here's a preliminary nits review:


>  # BODY-TO-EXPAND is some initialization which must be expanded in the
> -# given diversion when expanded (required or not).  This is very
> -# different from m4_require.  For instance:
> +# given diversion when expanded (required or not).  The expansion
> +# goes in the named diversion or an earlier one.

Hmmm.  M4SH-INIT is at diversion 6.  If this is identical to m4_require,
then it doesn't take very many nested AS_REQUIRES before we have dropped
to diversion 0, or even worse to diversion -1.  Even a shorter chain would
get confusing if we start impinging on HEADER_COPYRIGHT.  Do we need to
see the named diversions in m4sh renumbered to leave some reserved
diversion numbers, to guarantee that AS_REQUIRE prerequisites appear
cleanly after M4SH-SANITIZE.

> +m4_defun([AS_REQUIRE],
> +[m4_define([_m4_divert_desired], m4_default_quoted([$3], [M4SH-INIT]))dnl
> +m4_if(m4_eval(_m4_divert(_m4_divert_dump) <= 
> _m4_divert(_m4_divert_desired)), 1,

Or maybe this eval is trying to guarantee just that?  Again, I'm typing
this without a thorough review; it is just first impressions.

> @@ -1698,7 +1713,7 @@ m4_provide_if([$1],
>             [],
>             [m4_warn([syntax],
>                      [$1 is m4_require'd but not m4_defun'd])])]],
> -      [[m4_divert(_m4_defn([_m4_divert_dump]))]],
> +      [[m4_divert($3)]],

Underquoted?

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

iEYEARECAAYFAkjzQDgACgkQ84KuGfSFAYC+wQCeJBXOYvXkA6eIpWmXo/W9BV1M
hssAnj6K12sk6yXbRoSpWiIfgBkw+dfg
=tIjE
-----END PGP SIGNATURE-----




reply via email to

[Prev in Thread] Current Thread [Next in Thread]