Re: fix last weekend's regressions in AC_DEFINE

[Eric Blake]

Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:

> * Eric Blake wrote on Mon, Oct 01, 2007 at 06:12:18PM CEST:
> > Sorry for the breakage; and at least the AC_F77_WRAPPERS caught it.
> 
> Thanks for your work on this, and for the git help in the other thread.
> 
> > +2007-10-01  Eric Blake  <ebb9 <at> byu.net>
> > +
> > +       Fix regression in AC_DEFINE([macro(with_arg)]).
> > +       * lib/autoconf/general.m4 (AC_DEFINE_TRACE): Don't chop off close
> > +       quotes with a careless m4_substr.

Now that I'm looking at it again, both AC_DEFINE_TRACE and _AC_DEFINE_Q are 
chopping off the arguments for a macro name; and I only touched one of the two 
places.  I'm installing this patch below to consolidate this into a single 
operation; and yes, I noticed another (slight) speedup on my testcase of 
coreutils.

2007-10-10  Eric Blake  <address@hidden>

        Another AC_DEFINE speedup.
        * lib/autoconf/general.m4 (AC_DEFINE_TRACE): Move parameter
        elision...
        (_AC_DEFINE_Q): ...here, and only do it once.
        * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Avoid overquoting.
        * lib/m4sugar/m4sh.m4 (AS_LITERAL_IF): Fix m4_defn overquoting
        introduced 2007-10-05.


And in the process, I _finally_ understood enough of Ralf's question to 
hopefully answer it:

> > +
> 
> FWIW, the ChangeLog entry is a bit misleading, as
>   AC_DEFINE([MACRO([with], [args])])
> 
> is broken with respect to autoheader anyway.  One needs to use
>   AC_DEFINE(MACRO([with], [args]))
> 
> instead, otherwise the template will not be right.  IOW, with
>   AC_INIT
>   AC_CONFIG_HEADERS([config.h])
>   AC_DEFUN([FOO], [$1])
>   AC_DEFINE([FOO([BAR])], 1, [text])
>   AC_OUTPUT
> 
> config.h.in contains FOO rather than BAR.  I assume there is other code
> out there besides _AC_F77_WRAPPERS that uses the lesser quotation BTW.

Here's the current state of things.

  AC_DEFINE(macro([arg]))

invokes the m4 macro, with argument [arg], and uses the expansion of the macro 
as the desired preprocessor macro name.  This was done in a number of places in 
autoconf code (git grep 'AC_DEFINE.*AS_TR'); the notable exception was the line 
I fixed below in functions.m4 (and I had to fix it, because otherwise test 175 
AC_CHECK_FUNCS and others would break).  If the m4 macro expands to a literal, 
that literal is traced (the most common use case of AS_TR_CPP([HAVE_$ac_func]) 
has the property that since HAVE_$ac_func is not literal, the macro expansion 
is not literal either, and nothing is traced by the AC_DEFINE; here, autoheader 
relies on subsidiary macros like _AH_CHECK_FUNCS to learn the true list of 
functions to trace).  Since the name is already expanded, exposing it to 
another round of expansions is generally harmless (unless the string contains 
[], but as that is not valid in preprocessor variable names, it hasn't been a 
problem in practice).

  AC_DEFINE([macro(arg)])
  AC_DEFINE([macro([arg])])

tries to declare '#define macro(arg)' in config.h, then strips (arg) to check 
whether 'macro' is a literal, in which case it is traced.  Here, you get the 
result that autoheader traces 'macro', which is ok _provided_ that 'macro' is 
not also an m4 macro (and in the first case, that 'arg' is also not an m4 
macro).  My patch does not touch this situation.  (The problem boils down to 
the fact that _AC_DEFINE_Q calls AC_DEFINE_TRACE with VARIABLE quoted, but 
outpus VARIABLE in the here-doc unquoted).  It would be possible to make this 
consistent; I haven't tried yet, but I think quoting VARIABLE in the here-doc 
will break other things, so the only proper fix would be computing the trace of 
the expansion of VARIABLE (and in the majority of the cases, where 'macro' is 
not an m4 macro, there will be no difference in the outcome).  This also 
explains why the line in functions.m4 was broken - autoheader was trying to 
trace the macro name AS_TR_CPP, rather than HAVE_$ac_func.

  AC_DEFINE([[macro(arg)]])

The quoting rule of thumb says that this _should_ result in the 
literal '#define macro(arg)' as the config.h line, regardless of 
whether 'macro' is also an m4 macro.  But right now, it fails, again because 
the tracing is computed over the quoted VARIABLE, and [ is not a valid 
identifier character.

So, given the above, should I work on a followup patch that makes _AC_DEFINE_Q 
trace the expansion of VARIABLE, rather than the quoted VARIABLE?

---

From: Eric Blake <address@hidden>
Date: Wed, 10 Oct 2007 14:29:22 -0600
Subject: [PATCH] Another AC_DEFINE speedup.

* lib/autoconf/general.m4 (AC_DEFINE_TRACE): Move parameter
elision...
(_AC_DEFINE_Q): ...here, and only do it once.
* lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Avoid overquoting.
* lib/m4sugar/m4sh.m4 (AS_LITERAL_IF): Fix m4_defn overquoting
introduced 2007-10-05.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                 |    8 ++++++++
 lib/autoconf/functions.m4 |    4 ++--
 lib/autoconf/general.m4   |   21 ++++++++++++---------
 lib/m4sugar/m4sh.m4       |    3 ++-
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4e31b80..ea4f4bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2007-10-10  Eric Blake  <address@hidden>
 
+       Another AC_DEFINE speedup.
+       * lib/autoconf/general.m4 (AC_DEFINE_TRACE): Move parameter
+       elision...
+       (_AC_DEFINE_Q): ...here, and only do it once.
+       * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Avoid overquoting.
+       * lib/m4sugar/m4sh.m4 (AS_LITERAL_IF): Fix m4_defn overquoting
+       introduced 2007-10-05.
+
        Whitespace cleanup.
        * lib/autoconf/general.m4: Use consistent indentation.
        * configure: Regenerate.
diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index a992325..95f7b73 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -1,6 +1,6 @@
 # This file is part of Autoconf.                       -*- Autoconf -*-
 # Checking for functions.
-# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006 Free Software
+# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007 Free Software
 # Foundation, Inc.
 #
 # This program is free software: you can redistribute it and/or modify
@@ -88,7 +88,7 @@ AC_DEFUN([AC_CHECK_FUNCS],
 for ac_func in $1
 do
 AC_CHECK_FUNC($ac_func,
-             [AC_DEFINE_UNQUOTED([AS_TR_CPP([HAVE_$ac_func])]) $2],
+             [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_$ac_func])) $2],
              [$3])dnl
 done
 ])
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 896362d..384eb42 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -2015,14 +2015,9 @@ AS_IDENTIFIER_IF([$1], [],
 # AC_DEFINE_TRACE(CPP-SYMBOL)
 # ---------------------------
 # This macro is a wrapper around AC_DEFINE_TRACE_LITERAL which filters
-# out non literal symbols.
-#
-# m4_index is roughly 5 to 8 times faster than m4_bpatsubst, so only
-# use the regex when necessary.
+# out non literal symbols.  CPP-SYMBOL must not include any parameters.
 m4_define([AC_DEFINE_TRACE],
-[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL(
-  m4_if(m4_index([$1], [(]), [-1], [[$1]],
-       [m4_bpatsubst([[$1]], [(.*)])]))])])
+[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL([$1])])])
 
 
 # AC_DEFINE(VARIABLE, [VALUE], [DESCRIPTION])
@@ -2041,9 +2036,17 @@ m4_define([AC_DEFINE_UNQUOTED], [_AC_DEFINE_Q([], $@)])
 
 # _AC_DEFINE_Q(QUOTE, VARIABLE, [VALUE], [DESCRIPTION])
 # -----------------------------------------------------
+# Internal function that performs common elements of AC_DEFINE{,_UNQUOTED}.
+#
+# m4_index is roughly 5 to 8 times faster than m4_bpatsubst, so only
+# use the regex when necessary.  AC_name is defined with over-quotation,
+# so that we can avoid m4_defn.
 m4_define([_AC_DEFINE_Q],
-[AC_DEFINE_TRACE([$2])dnl
-m4_ifval([$4], [AH_TEMPLATE(m4_bpatsubst([[$2]], [(.*)]), [$4])])dnl
+[m4_pushdef([AC_name], m4_if(m4_index([$2], [(]), [-1], [[[$2]]],
+                            [m4_bpatsubst([[[$2]]], [(.*)])]))dnl
+AC_DEFINE_TRACE(AC_name)dnl
+m4_ifval([$4], [AH_TEMPLATE(AC_name, [$4])])dnl
+m4_popdef([AC_name])dnl
 cat >>confdefs.h <<$1_ACEOF
 address@hidden:@define] $2 m4_if($#, 2, 1, [$3])
 _ACEOF
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 15a4ed1..157b844 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -1272,7 +1272,8 @@ m4_dquote(m4_dquote(m4_defn([m4_cr_symbols1])))[[))], 
[0], [$2], [$3])])
 m4_define([AS_LITERAL_IF],
 [m4_cond([m4_eval(m4_index(m4_quote($1), address@hidden|@]) == -1)], [0], [$3],
         [m4_index(m4_translit(m4_quote($1),
-                              [[]`,#]]m4_dquote(m4_defn([m4_cr_symbols2]))[,
+                              [[]`,#]]]dnl
+m4_dquote(m4_dquote(m4_defn([m4_cr_symbols2])))[[,
                               [$$$]),
                   [$])], [-1], [$2],
         [$3])])
-- 
1.5.3.2