Re: [PATCH 10/12] use a shell function for AC_COMPUTE_INT

[Eric Blake]

Paolo Bonzini <bonzini <at> gnu.org> writes:

> 
> The remaining three patches achieve the final 5-10% reduction.
> 
> 2008-10-27  Paolo Bonzini  <bonzini <at> gnu.org>
> 
>       * lib/autoconf/general.m4 (_AC_COMPUTE_INT_COMPILE,
>       _AC_COMPUTE_INT_RUN): Add IF-SUCCESS argument.
>       (_AC_COMPUTE_INT_BODY): New.
>       (AC_COMPUTE_INT): Define and use a shell function.

This one will be a bit more fun.  Let's defer applying this until another round 
of discussion and/or a rebase with these nits addressed.

> @@ -2896,32 +2897,60 @@ while test "x$ac_lo" != "x$ac_hi"; do
>    _AC_COMPILE_IFELSE([AC_LANG_BOOL_COMPILE_TRY([$3], [($1) <= $ac_mid])],
>                    [ac_hi=$ac_mid], [ac_lo=`expr '(' $ac_mid ')' + 1`])

Hmm.  Maybe I should speed up my work on AS_VAR_ARITH (borrowing from 
autotest's at_func_arith), since this is a place that can use it for fewer 
forks.  But that can be a separate patch.

>  done
> +AS_VAR_PUSHDEF([ac_Result], [$2])dnl

This is a helper macro.  We can guarantee that since this is only called inside 
_AC_COMPUTE_INT_BODY, it will be a valid indirect macro name that doesn't need 
any further transliteration (AS_VAR_PUSHDEF is primarily useful when you need 
to turn an abitrary string, such as "struct foo.bar", into a valid shell 
name "struct_foo_bar", but here, we know that an m4 argument of "$[]3" already 
contains a valid shell variable name).

>  case $ac_lo in

I've started adding ' @%:@((' after the 'in', as a strategic shell comment to 
balance m4 (); we'd get that for free if we used AS_CASE.  I'm not sure whether 
using AS_CASE is worth it though - on one hand, it adds more macro expansions 
than open-coding, on the other hand, it is no longer on a hot path since the 
shell function body is only emitted once per language.

> -?*) $2=$ac_lo;;
> -'') $4 ;;
> +?*) AS_VAR_SET([ac_Result], [$ac_lo]); $4 ;;

Given my above assertion that the m4 argument $2 always contains a valid 
indirect shell name (whereas it used to be a valid direct shell name), this 
could be written:

?*) AS_VAR_SET([$2], [$ac_lo]); $4 ;;

> +'') $5 ;;
>  esac[]dnl
> +AS_VAR_POPDEF([ac_Result])dnl
>  ])# _AC_COMPUTE_INT_COMPILE
> 
>  
> -# _AC_COMPUTE_INT_RUN(EXPRESSION, VARIABLE, PROLOGUE, [IF-FAILS])
> -# ---------------------------------------------------------------
> +# _AC_COMPUTE_INT_RUN(EXPRESSION, VARIABLE, PROLOGUE, [IF-SUCCESS],
> +#                     [IF-FAILURE])
> +# -----------------------------------------------------------------
>  # Store the evaluation of the integer EXPRESSION in VARIABLE.
>  m4_define([_AC_COMPUTE_INT_RUN],
> -[_AC_RUN_IFELSE([AC_LANG_INT_SAVE([$3], [$1])],
> -             [$2=`cat conftest.val`], [$4])])
> +[AS_VAR_PUSHDEF([ac_Result], [$2])dnl

Likewise, for using $2 as-is.

> +_AC_RUN_IFELSE([AC_LANG_INT_SAVE([$3], [$1])],
> +            [AS_VAR_SET([ac_Result], [`cat conftest.val`]); $4], [$5])

Is this an instance where read(1) would be more efficient?  Separate patch, 
though.

> +rm -f conftest.val
> +AS_VAR_POPDEF([ac_Result])dnl
> +])
> 
>  
> +# _AC_COMPUTE_INT_BODY
> +# --------------------
> +# Shell function body for AC_COMPUTE_INT.
> +m4_define([_AC_COMPUTE_INT_BODY],
> +[  AS_LINENO_PUSH([$[]1])
> +  if test "$cross_compiling" = yes; then
> +    _AC_COMPUTE_INT_COMPILE([$[]2], [$[]3], [$[]4],
> +                         [ac_retval=0], [ac_retval=1])
> +  else
> +    _AC_COMPUTE_INT_RUN([$[]2], [$[]3], [$[]4],
> +                     [ac_retval=0], [ac_retval=1])
> +  fi
> +  rm -f conftest.val
> +  AS_LINENO_POP
> +  return $ac_retval
> +])dnl

We don't really need a dnl after the m4_define (I've nearly done it several 
times on patches 1-6, though, since it is a code move from somewhere where the 
dnl made sense).  Using # _AC_COMPUTE_INT_BODY looks a bit nicer.

> +
>  # AC_COMPUTE_INT(VARIABLE, EXPRESSION, PROLOGUE, [IF-FAILS])
>  # ----------------------------------------------------------
>  AC_DEFUN([AC_COMPUTE_INT],

This could use some comments as to the function of this macro.

> -[AC_LANG_COMPILER_REQUIRE()dnl
> -if test "$cross_compiling" = yes; then
> -  _AC_COMPUTE_INT_COMPILE([$2], [$1], [$3], [$4])
> -else
> -  _AC_COMPUTE_INT_RUN([$2], [$1], [$3], [$4])
> -fi
> -rm -f conftest.val[]dnl
> -])# _AC_COMPUTE_INT
> +[AC_LANG_COMPILER_REQUIRE()]dnl
> +[AC_REQUIRE_SHELL_FN([ac_func_]_AC_LANG_ABBREV[_compute_int],
> +  [AS_FUNCTION_DESCRIBE([ac_func_]_AC_LANG_ABBREV[_compute_int],
> +    [LINENO EXPR VAR INCLUDES],
> +    [Tries to find the compile-time value of EXPR in a program that includes
> +     INCLUDES, setting VAR accordingly.  Returns whether the value could
> +     be computed])],
> +    [_$0_BODY])]dnl
> +[AS_IF([ac_func_[]_AC_LANG_ABBREV[]_compute_int "$LINENO" "$2" "$1" ]dnl
> +       ["AS_ESCAPE([$3], ["])"],

[""] to keep font-lock balance.

-- 
Eric Blake