Re: avoiding obsolete macros like AC_TRY_RUN

[Bruno Haible]

Hi Ralf,

>     * gnulib-tool (func_create_testdir, func_create_megatestdir):
>     Replace obsolete Autoconf macros with modern counterparts.

This part is undoubtably good. I have applied it for you.

For the rest, you would have saved yourself some time by asking whether
a patch of this kind is welcome before performing it on more than 70 files.

I have three problems with your patch.

1) The use of the AC_*_IFELSE macros. Since these macros were introduced,
   I have said in public that there is nothing wrong with the AC_TRY_*
   macros. I use them in plenty of places for > 10 years; they are fine.

   Comparing AC_*_IFELSE with AC_TRY*:
   Advantages of AC_*_IFELSE:
     - More systematic. AC_TRY_LINK takes includes and function-body arguments,
       and AC_TRY_RUN a single argument.
     - Possibility to pass code that contains invocations of autoconf
       macros (rarely used).
   Drawbacks of AC_*_IFELSE:
     - Unmnemonic naming of the macros. Macros and procedure names should follow
       the convention of "verb" or "verb_noun", so that they can better be
       remembered by people who are familiar with English grammar.
     - Danger if a single level of quoting is used instead of double quoting:
       Array declarations and array accesses get silently botched in that
       case. The autoconf manual uses double-quoting in the examples of
       AC_LANG_SOURCE and AC_LANG_PROGRAM, which is good. But some advocates 
like
       you propagate single-quoting.
     - Pitfall: It's easy to forget the invocation of AC_LANG_SOURCE, and then
       autoconf generates code which will fail on many but not all platforms.

   That makes 1.5 advantages for 3.0 drawbacks. A bad tradeoff.

   What are you going to do about the drawbacks in autoconf?
     - Any chance of finding better names for the macros?
     - Any chance of adding a warning to autoconf so that uses of
         AC_*_IFELSE([Program], ...)
       where the Program does not contain an AC_LANG_SOURCE or
       AC_LANG_PROGRAM invocation does generate a warning?

2) Incomplete quotation: You say

>     environ.m4:gt_CHECK_VAR_DECL
>     intl.m4:gt_CHECK_DECL
>     lib-link.m4:AC_LIB_HAVE_LINKFLAGS
>     minmax.m4:gl_MINMAX_IN_HEADER
>     signbit.m4:gl_FLOATTYPE_SIGN_LOCATION
> ...
> For these macros, the patch below removes the double quotation and
> instead uses single quotation (at least for the argument expansion),
> as is used throughout the bulk of Autoconf macros.

   As a general rule, the arguments of AC_LANG_SOURCE or AC_LANG_PROGRAM
   *must* be double-quoted. It is 10 times more frequent to have C programs
   and C statements that contain array declarations/accesses than C programs
   or statements that contain autoconf macro invocations.

   The macros gt_CHECK_VAR_DECL, gt_CHECK_DECL, gl_MINMAX_IN_HEADER,
   gl_FLOATTYPE_SIGN_LOCATION are not public: all possible invocations are
   found in these *.m4 files. Therefore they are correct as is.

   The macro AC_LIB_HAVE_LINKFLAGS is meant to be called with literal
   arguments. Calling it with complex expressions is not supported. In
   particular, one of its arguments is 'testcode', which may contain
   array declarations. If you remove a level of quoting here, you introduce
   a bug.

3) Indentation:

> -          AC_TRY_RUN([
> +          AC_RUN_IFELSE([AC_LANG_SOURCE([[
>  #include <stddef.h>
>  #include <stdio.h>
>  int main ()
>  {
>    return fopen ("conftest.sl/", "w") != NULL;
> -}], [gl_cv_func_fopen_slash=yes], [gl_cv_func_fopen_slash=no],
> +}]])], [gl_cv_func_fopen_slash=yes], [gl_cv_func_fopen_slash=no],

   Lines of code are not *that* expensive that it would warrant
     - opening two macro invocations on the same line, without closing
       the second one on the same line,
     - putting 5 closing brackets/braces/parentheses in a row, like if
       you were programming Forth.
   You can't expect that I approve such an indentation.

Bruno