Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

[Paolo Bonzini]

On 31/10/2016 18:01, Eric Blake wrote:
> On 10/31/2016 10:33 AM, Paolo Bonzini wrote:
>>
>>
>> On 31/10/2016 16:28, 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_FUNCS and the first argument is a literal.
>>>> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>>>>
>>>> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
>>>> (_AC_CHECK_FUNCS): Move basic implementation here.
>>>> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
>>>> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
>>>
>>>> -         [$3])dnl])
>>>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
>>>
>>> Why []yes instead of the more typical [yes] ?
>>>
>>> If the user has (unwisely) defined yes as a macro, your version will
>>> compare against their expansion, instead of against the intended literal.
>>
>> Because I've never understood the rules for m4_if, and thought []yes
>> matched what you get from m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]).
> 
> If 'yes' and 'no' are not macros, then these are identical:
> 
> m4_if([$2$3]AS_LITERAL_IF([$1], yes, no), yes, ...)
> m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes[], ...)
> m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes], ...)
> 
> because of the order in which quotes are stripped through successive
> levels of m4 processing.  But if yes or no could be a macro, then only
> the latter form is safe against unintended expansions (yes, including
> the double-quoting within AS_LITERAL_IF, because one layer of quotes
> gets stripped when collecting the args to AS_LITERAL_IF, and another
> gets stripped when concatenating to [$2$3] to determine the final string
> to compare against the literal of the second argument to m4_if).

Good, this is what I wanted.  I'll send v2 where the
only difference will be:

diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index 1043192..ccfb053 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -85,7 +85,7 @@ m4_define([_AH_CHECK_FUNC],
 # `break' to stop the search.
 AC_DEFUN([AC_CHECK_FUNCS],
 [m4_map_args_w([$1], [_AH_CHECK_FUNC(], [)])]dnl
-[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+[m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes],
        [m4_map_args_w([$1], [_$0(], [)])],
        [AS_FOR([AC_func], [ac_func], [$1], [_$0(AC_func, [$2], [$3])])])
 ])# AC_CHECK_FUNCS
diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
index e855025..42f373f 100644
--- a/lib/autoconf/headers.m4
+++ b/lib/autoconf/headers.m4
@@ -182,7 +182,7 @@ 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
-[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+[m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes],
        [m4_map_args_w([$1], [_$0(], [, [], [], [$4])])],
        [AS_FOR([AC_header], [ac_header], [$1], [_$0(AC_header, [$2], [$3], 
[$4])])])
 ])# AC_CHECK_HEADERS


Please commit for me, since I don't have write access to autoconf.git.

Thanks,

Paolo

signature.asc
Description: OpenPGP digital signature