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

[Eric Blake]

On 11/02/2016 06:41 PM, Paolo Bonzini wrote:
> 
> 
> On 02/11/2016 23:14, Eric Blake wrote:
>> Our use of AC_REQUIRE hoists the check outside of any AS_IF or similar
>> code.  However, while I don't think any well-written configure.ac script
>> will be checking $ac_cv_func_foo prior to calling
>> AC_CHECK_FUNC_ONCE(foo), I _am_ a bit worried that poorly written
>> scripts that do:
>>
>> if condition
>>   AC_CHECK_FUNC_ONCE(foo)
>> fi
>> test $ac_cv_func_foo
> 
> I'm not worried about this.  However, now that I think more about it,
> I'm a bit more worried about something that has a deep chain of
> AC_REQUIREs, and ultimately ends up expanding AC_CHECK_FUNC_ONCE inside
> an "if".

Hmm. Yeah, I've been thinking over this all afternoon.  The old
_AC_FUNCS_EXPANSION expands once, but at the site of the first
AC_CHECK_FUNC_ONCE() [well, hoisted above AS_IF thanks to AC_REQUIRE],
regardless of what other context might be.

> 
>> Potential solution: collect the list of AC_CHECK_FUNC_ONCE functions in
>> an m4 list, and unroll that list where we used to do the AS_FOR, so that
>> we aren't changing the semantics of hoisting all the checks up front
>> early during configure.
> 
> How would you do that?  You would have to expand before the first
> AC_CHECK_FUNC_ONCE, but you do not have a diversion there (you might
> need one per language, in fact).

m4_wrap lets us collect the list as we go, then expand code at the end.
But you're right - I have to expand it to some diversion, and creating a
diversion at the place where AC_CHECK_FUNC_ONCE() is first called isn't
likely to be useful.

> 
> Perhaps you can put the whole code for the tests in a variable (it's
> just a couple lines of shell if you add ac_fn_simple_define) in
> INIT_PREPARE or SHELL_FN, and then eval it in _AC_FUNCS_EXPANSION.

Or use diversions to craft a function header in SHELL_FN+N, body in
SHELL_FN+N+1, and tail in SHELL_FN+N+2, then have the site where
_AC_FUNCS_EXPANSION does its one-time expansion just call the shell
function.  Each body of the shell function would be a one or two line
check for the next C function being probed.  No need for an eval of a
huge shell string.  Oh, and you're right that N would have to be a
per-language offset.

Or maybe we can take a different approach.  Instead of eval, we can use
the knowledge that my proposal for a one-liner function still ensures
that we have a literal function name, literal cache variable name, and
literal witness name.  Instead of making ac_func_list be a list of
singletons 'func1 func2', it can be a list of triplets 'func1 cv1
witness1 func2 cv2 witness2' where our loop for processing them is no
longer a simple AS_FOR.  [Too bad we don't have python's ability to
easily suck off a tuple at a time from a list, but it can be emulated
easily enough].

> 
> That said, what I'm doing now is actually already done by
> AC_CHECK_DECLS_ONCE, so we might just document it as a possible
> incompatibility.

Indeed, that's my fallback approach if I don't come up with an elegant
solution in the next 24 hours.  It's unlikely to affect many real users,
and if it DOES affect someone, it will be a very obscure thing to track
down, but a mention in NEWS (and maybe a slight tweak to the
documentation wording to not promise up-front evaluation) should help if
we end up having to debug a real package that breaks as a result.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature