Re: [PATCH 2/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_HEADERS

[Paolo Bonzini]

On 31/10/2016 16:32, Eric Blake wrote:
> On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
>> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
>> AS_TR_CPP.  Prefer it if there is nothing in the second and third
>> argument of AC_CHECK_HEADERS and the first argument is a literal.
>> Modify AC_CHECK_HEADERS_ONCE to avoid the variable indirection too.
>>
>> * lib/autoconf/headers.m4 (AC_CHECK_HEADERS): Unroll loop if safe.
>> (_AC_CHECK_HEADERS): Move basic implementation here.
>> (_AC_CHECK_HEADER_ONCE): Expand AC_CHECK_HEADERS here...
>> (_AC_HEADERS_EXPANSION): ... and not here, so remove.
>> (AC_CHECK_INCLUDES_DEFAULT): Remove unnecessary arguments after the first.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  lib/autoconf/headers.m4 | 30 +++++++++++++-----------------
>>  1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
>> index 72262c1..e855025 100644
>> --- a/lib/autoconf/headers.m4
>> +++ b/lib/autoconf/headers.m4
>> @@ -182,22 +182,25 @@ m4_define([AH_CHECK_HEADERS],
>>  # header.  Either ACTION may include `break' to stop the search.
>>  AC_DEFUN([AC_CHECK_HEADERS],
>>  [m4_map_args_w([$1], [_AH_CHECK_HEADER(], [)])]dnl
>> -[AS_FOR([AC_header], [ac_header], [$1],
>> -[AC_CHECK_HEADER(AC_header,
>> -             [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_header)) $2],
>> -             [$3], [$4])dnl])
>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
> 
> Again, why []yes instead of [yes]?
> 
>>  
>>  ## --------------------- ##
>> @@ -261,8 +258,7 @@ ac_includes_default="\
>>  #endif"
>>  ])]dnl
>>  [_AC_CHECK_HEADERS_ONCE(
>> -  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h],
>> -  [], [], [$ac_includes_default])]dnl
>> +  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h])]dnl
> 
> This one, I'm worried about.  $ac_includes_default is a shell variable
> not documented in the manual (so we can probably get away with changing
> it), but doesn't it exist so that projects can redefine the core set of
> default headers to check by default, whereas you are now losing that
> flexibility?

No, the arguments after the first were unused:

AC_DEFUN([_AC_CHECK_HEADERS_ONCE],
  [m4_map_args_w([$1], [_AC_CHECK_HEADER_ONCE(], [)])])


The variable is still used in my patch by _AC_CHECK_HEADER_ONCE:

m4_define([_AC_CHECK_HEADER_ONCE],
[AC_DEFUN([_AC_Header_]m4_translit([[$1]], [./-], [___]),
          [AC_CHECK_HEADERS([$1], [], [], [$ac_includes_default])])
AC_REQUIRE([_AC_Header_]m4_translit([[$1]], [./-], [___]))])


In fact, the whole usage of $ac_includes_default is only there to avoid
recursion between AC_INCLUDES_DEFAULT and AC_CHECK_HEADERS_ONCE: instead
of using AC_INCLUDES_DEFAULT as the fourth argument, we include its
expansion directly.

Now, because AC_CHECK_INCLUDES_DEFAULT is AC_DEFUN_ONCE'd, the mutual
recursion you would not produce an _infinite recursion_; however, you
would still get a bogus warning about m4_requiring a macro that is not
m4_defun'd.

Thanks,

Paolo

signature.asc
Description: OpenPGP digital signature