popdef(undefined), __m4_version__

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'm checking in these two patches on branch-1.6, and a similar doc-only
version of the first patch on the master branch.

The first patch deals with warning when dereferencing undefined macros.
The master branch has already had the behavior of warning on
popdef(undefined)/undefine(undefined) for quite some time, but the manual
claimed they didn't.  Meanwhile, warning on defn(undefined) was recently
added to the branch, and the branch has always warned on
dumpdef(undefined)/indir(undefined).
traceon(undefined)/traceoff(undefined) must not warn (it is legal to turn
tracing on for something not yet defined), neither should m4symbols (it is
useful as a bulk ifdef).

Rationale for adding the warning: it is possible to globally silence the
warning (use -Q, or write a wrapper to explicitly check for defined-ness
before each use; and the patch includes a formula for doing so).  The
warning does not affect exit status (unless you use -E).  Additionally,
the most popular m4 client, m4sugar, has gone to the effort to write the
opposite wrapper to globally produce the warning even when m4 doesn't;
using any wrapper adds some execution overhead, so m4sugar then has to use
a lot of m4_builtin([popdef]) constructs to avoid the wrapper.  So, I also
wrote a patch for autoconf to use m4's native warning instead of a wrapper
for m4_defn/m4_popdef/m4_undefine to see what speed improvement would
result from using fewer wrappers, using 'time M4=... autoconf -f' on
coreutils.  Autoconf patch [a] blindly skips defining the
m4_defn/m4_popdef/m4_undefine wrappers (only safe if the underlying m4
warns), and patch [b] converts all instances of m4_builtin([popdef]) or
m4_builtin([defn]) back to m4_popdef/m4_defn:

~       autoconf 2.62.45-65ff0  +[a]    +[b]    +[a] and [b]
m4-1.4.11               20.171  19.858  20.858  19.357
patched branch-1.6      21.294  20.154  22.202  20.250
argv_ref branch         19.013  17.750  19.219  17.266
master branch           29.966  28.948  32.417  27.904

The differences in base compile times are due somewhat to differing
compiler options, but it is also a bit depressing that the master branch
is 50% slower than 1.6.  Someday I'd like to profile that, and figure out
why the discrepancy.  Using [a] is a large win, but is a behavior
regression for 1.4.x.  Using [b] is a loss if wrappers are in use, but a
slight win if the native builtins are used.

The second patch backports the macro __m4_version__, which has existed on
the master branch for some time now.  The autoconf patch needs a reliable
way to determine whether popdef([undefined]) will warn (for example, it
does not warn for m4-1.4.11), and I found it easiest to use the existence
(but not contents) of __m4_version__ as the key, rather than adding a new
configure time check.  So, the patch I will commit to autoconf is a
variant of [a] where the optimization is made conditional on
__m4_version__ (m4-1.4.x still needs wrappers to warn on undefined
macros), and omitting [b] (so that m4-1.4.x isn't slowed down because it
uses wrappers).  In a few years, when m4 1.6 is more prevalent, we can
then change autoconf to also use [b].

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkiB5nIACgkQ84KuGfSFAYAVDQCfRKhsOOmdh/XlcSvCWa5vTkvC
UzoAn1hwz1Djrf3fIUQQFnd06142tvIj
=mcYv
-----END PGP SIGNATURE-----

>From d8ae88a9374c168218ace6ed6b5e492a9ca35643 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 18 Jul 2008 09:40:55 -0600
Subject: [PATCH] Warn on popping undefined macro.

* src/builtin.c (m4_undefine, m4_popdef): Add warning.
* src/symtab.c (lookup_symbol): Distinguish when deletion occurred.
* doc/m4.texinfo (Undefine, Pushdef): Document the warning.
* NEWS: Document this change, along with a 1.4.x compatibility
definition.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |    9 ++++++++
 NEWS           |   14 ++++++++++--
 doc/m4.texinfo |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/builtin.c  |   12 +++++++---
 src/symtab.c   |    7 ++++-
 5 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d3cb5b4..62ff283 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2008-07-18  Eric Blake  <address@hidden>
+
+       Warn on popping undefined macro.
+       * src/builtin.c (m4_undefine, m4_popdef): Add warning.
+       * src/symtab.c (lookup_symbol): Distinguish when deletion occurred.
+       * doc/m4.texinfo (Undefine, Pushdef): Document the warning.
+       * NEWS: Document this change, along with a 1.4.x compatibility
+       definition.
+
 2008-07-17  Eric Blake  <address@hidden>
 
        Fix missing copyright notices.
diff --git a/NEWS b/NEWS
index 1ed9489..0519be1 100644
--- a/NEWS
+++ b/NEWS
@@ -49,9 +49,17 @@ Foundation, Inc.
    earlier in the command line, so that the client will not break in the
    face of an upgraded m4 and a POSIXLY_CORRECT execution environment.
 
-** The `defn' builtin now warns when operating on an undefined macro name.
-   To simulate 1.4.x behavior, use:
-     pushdef(`defn', `ifdef(`$1', `builtin(`defn', `$1')')')
+** The `defn', `popdef', and `undefine' builtins now warn when operating
+   on an undefined macro name, similar to what was already done in the
+   `indir' and `dumpdef' builtins.  To simulate 1.4.x behavior, use:
+     pushdef(`defn', `ifelse(`$#', `0', ``defn'',
+                             `ifdef(`$1', `builtin(`defn', `$1')')')')
+     pushdef(`popdef', `ifelse(`$#', `0', ``popdef'', `_$0($@)')')
+     define(`_popdef', `ifdef(`$1', `builtin(`popdef', `$1')')ifelse(`$#',
+                `1', `', `$0(shift($@))')')
+     pushdef(`undefine', `ifelse(`$#', `0', ``undefine'', `_$0($@)')')
+     define(`_undefine', `ifdef(`$1', `builtin(`undefine', `$1')')ifelse(`$#',
+                `1', `', `$0(shift($@))')')
 
 ** Enhance the `indir' builtin to trace indirect macros, where the trace
    is requested via `traceon' or the command-line option `-t'.  Previously,
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index cad75ca..ab6ef00 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -2078,7 +2078,8 @@ A macro definition can be removed with @code{undefine}:
 
 @deffn Builtin undefine (@address@hidden)
 For each argument, remove the macro @var{name}.  The macro names must
-necessarily be quoted, since they will be expanded otherwise.
+necessarily be quoted, since they will be expanded otherwise.  If an
+argument is not a defined macro, a warning is issued.
 
 The expansion of @code{undefine} is void.
 The macro @code{undefine} is recognized only with parameters.
@@ -2113,8 +2114,32 @@ f(`bye')
 @result{}f(bye)
 @end example
 
-It is not an error for @var{name} to have no macro definition.  In that
-case, @code{undefine} does nothing.
+As of M4 1.6, @code{undefine} will warn if @var{name} is not a macro.
+The following example shows how to recover earlier behavior that
+silently ignores undefined names, using concepts that will be explained
+later in the manual.  Or, you could use the command line option
address@hidden (@option{--quiet}, @pxref{Operation modes, , Invoking m4}).
+
address@hidden
+define(`a', `1')
address@hidden
+undefine
address@hidden
+undefine(`a', `a')
address@hidden:stdin:3: Warning: undefine: undefined macro `a'
address@hidden
+define(`a', `1')
address@hidden
+pushdef(`undefine', `ifelse(`$#', `0', ``undefine'', `_$0($@@)')')
address@hidden
+define(`_undefine', `ifdef(`$1', `builtin(`undefine',
+  `$1')')ifelse(`$#', `1', `', `$0(shift($@@))')')
address@hidden
+undefine
address@hidden
+undefine(`a', `a')
address@hidden
address@hidden example
 
 @node Defn
 @section Renaming macros
@@ -2364,7 +2389,8 @@ exactly like @code{define}.
 
 If a macro has several definitions (of which only one is accessible),
 the topmost definition can be removed with @code{popdef}.  If there is
-no previous definition, @code{popdef} behaves like @code{undefine}.
+no previous definition, @code{popdef} behaves like @code{undefine}, and
+if there is no definition at all, @code{popdef} issues a warning.
 
 The expansion of both @code{pushdef} and @code{popdef} is void.
 The macros @code{pushdef} and @code{popdef} are recognized only with
@@ -2437,6 +2463,33 @@ revealing the former definition.
 It is possible to temporarily redefine a builtin with @code{pushdef}
 and @code{defn}.
 
+As of M4 1.6, @code{popdef} will warn if @var{name} is not a macro.
+The following example shows how to recover earlier behavior that
+silently ignores undefined names, using concepts that will be explained
+later in the manual.  Or, you could use the command line option
address@hidden (@option{--quiet}, @pxref{Operation modes, , Invoking m4}).
+
address@hidden
+define(`a', `1')
address@hidden
+popdef
address@hidden
+popdef(`a', `a')
address@hidden:stdin:3: Warning: popdef: undefined macro `a'
address@hidden
+define(`a', `1')
address@hidden
+pushdef(`popdef', `ifelse(`$#', `0', ``popdef'', `_$0($@@)')')
address@hidden
+define(`_popdef', `ifdef(`$1', `builtin(`popdef',
+  `$1')')ifelse(`$#', `1', `', `$0(shift($@@))')')
address@hidden
+popdef
address@hidden
+popdef(`a', `a')
address@hidden
address@hidden example
+
 @node Indir
 @section Indirect call of macros
 
diff --git a/src/builtin.c b/src/builtin.c
index aa108d4..c3e4a7e 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -717,8 +717,10 @@ m4_undefine (struct obstack *obs, int argc, 
macro_arguments *argv)
   for (i = 1; i < argc; i++)
     if (arg_type (argv, i) != TOKEN_TEXT)
       m4_warn (0, me, _("invalid macro name ignored"));
-    else
-      lookup_symbol (ARG (i), ARG_LEN (i), SYMBOL_DELETE);
+    else if (!lookup_symbol (ARG (i), ARG_LEN (i), SYMBOL_DELETE))
+      m4_warn (0, me, _("undefined macro %s"),
+              quotearg_style_mem (locale_quoting_style, ARG (i),
+                                  ARG_LEN (i)));
 }
 
 static void
@@ -737,8 +739,10 @@ m4_popdef (struct obstack *obs, int argc, macro_arguments 
*argv)
   for (i = 1; i < argc; i++)
     if (arg_type (argv, i) != TOKEN_TEXT)
       m4_warn (0, me, _("invalid macro name ignored"));
-    else
-      lookup_symbol (ARG (i), ARG_LEN (i), SYMBOL_POPDEF);
+    else if (!lookup_symbol (ARG (i), ARG_LEN (i), SYMBOL_POPDEF))
+      m4_warn (0, me, _("undefined macro %s"),
+              quotearg_style_mem (locale_quoting_style, ARG (i),
+                                  ARG_LEN (i)));
 }
 
 /*---------------------.
diff --git a/src/symtab.c b/src/symtab.c
index d4da200..7e76cfc 100644
--- a/src/symtab.c
+++ b/src/symtab.c
@@ -155,7 +155,9 @@ free_symbol (symbol *sym)
 | can either just do a lookup, do a lookup and insert if not         |
 | present, do an insertion even if the name is already in the list,  |
 | delete the first occurrence of the name on the list, or delete all |
-| occurrences of the name on the list.                               |
+| occurrences of the name on the list.  The return value when        |
+| requesting deletion is non-NULL if deletion occurred, but must not |
+| be dereferenced.                                                   |
 `-------------------------------------------------------------------*/
 
 symbol *
@@ -268,6 +270,7 @@ lookup_symbol (const char *name, size_t len, symbol_lookup 
mode)
        return NULL;
       {
        bool traced = false;
+        symbol *result = sym;
        if (SYMBOL_NEXT (sym) != NULL
            && SYMBOL_SHADOWED (SYMBOL_NEXT (sym))
            && mode == SYMBOL_POPDEF)
@@ -301,8 +304,8 @@ lookup_symbol (const char *name, size_t len, symbol_lookup 
mode)
            SYMBOL_NEXT (sym) = *spp;
            (*spp) = sym;
          }
+        return result;
       }
-      return NULL;
 
     default:
       assert (!"symbol_lookup");
-- 
1.5.6


>From 37a774c7298a6d8e05e5678a78957ac4c4e71e46 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 18 Jul 2008 11:14:16 -0600
Subject: [PATCH] Add __m4_version__.

* src/builtin.c (predefined_tab): Add new macro to table.
* doc/m4.texinfo (Platform macros): Document it.
* NEWS: Likewise.
* checks/check-them (examples): Support @value{VERSION} in test.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |    6 ++++++
 NEWS              |    6 ++++++
 checks/check-them |    5 ++++-
 doc/m4.texinfo    |   22 ++++++++++++++++++++--
 src/builtin.c     |    1 +
 5 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 62ff283..6b55a4e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2008-07-18  Eric Blake  <address@hidden>
 
+       Add __m4_version__.
+       * src/builtin.c (predefined_tab): Add new macro to table.
+       * doc/m4.texinfo (Platform macros): Document it.
+       * NEWS: Likewise.
+       * checks/check-them (examples): Support @value{VERSION} in test.
+
        Warn on popping undefined macro.
        * src/builtin.c (m4_undefine, m4_popdef): Add warning.
        * src/symtab.c (lookup_symbol): Distinguish when deletion occurred.
diff --git a/NEWS b/NEWS
index 0519be1..47e0af4 100644
--- a/NEWS
+++ b/NEWS
@@ -49,6 +49,12 @@ Foundation, Inc.
    earlier in the command line, so that the client will not break in the
    face of an upgraded m4 and a POSIXLY_CORRECT execution environment.
 
+** A new predefined text macro, `__m4_version__', expands to the unquoted
+   version number of M4, if GNU extensions are enabled.  While you should
+   generally favor feature tests over version number checks, this macro can
+   be used, via `defn', to determine whether the version of m4 processing
+   your file is adequate.
+
 ** The `defn', `popdef', and `undefine' builtins now warn when operating
    on an undefined macro name, similar to what was already done in the
    `indir' and `dumpdef' builtins.  To simulate 1.4.x behavior, use:
diff --git a/checks/check-them b/checks/check-them
index bad88f8..8ddef58 100755
--- a/checks/check-them
+++ b/checks/check-them
@@ -69,6 +69,9 @@ else
   strip_needed=:
 fi
 
+# Find out what version the executable claims to be
+m4version=`sed -n 's/.* //p;q' $out`
+
 # Find out if diff supports useful options.
 if diff -u /dev/null /dev/null 2>/dev/null ; then
   diffopts="-u"
@@ -108,7 +111,7 @@ do
   xoutfile=`sed -n 's/^dnl @ expected output: //p' "$file"`
   if test -z "$xoutfile" ; then
     sed -e '/^dnl @result{}/!d' -e 's///' -e "s|examples/|$examples/|" \
-      "$file" > $xout
+      -e "s|@value{VERSION}|$m4version|" "$file" > $xout
   else
     cp "$examples/$xoutfile" $xout
   fi
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index ab6ef00..79ea41d 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -6347,7 +6347,8 @@ exit value if this is not the case.
 Sometimes it is desirable for an input file to know which platform
 @code{m4} is running on.  @acronym{GNU} @code{m4} provides several
 macros that are predefined to expand to the empty string; checking for
-their existence will confirm platform details.
+their existence will confirm platform details.  It also provides a macro
+for learning about @code{m4} itself.
 
 @deffn {Optional builtin} __gnu__
 @deffnx {Optional builtin} __os2__
@@ -6362,10 +6363,15 @@ string.  For now, these macros silently ignore all 
arguments, but in a
 future release of M4, they might warn if arguments are present.
 @end deffn
 
address@hidden builtin __m4_version__
+Expands to an unquoted string containing the release version number of
+the running @acronym{GNU} @code{m4} executable.
address@hidden deffn
+
 When @acronym{GNU} extensions are in effect (that is, when you did not
 use the @option{-G} option, @pxref{Limits control, , Invoking m4}),
 @acronym{GNU} @code{m4} will define the macro @address@hidden to
-expand to the empty string.
+expand to the empty string, and provide @address@hidden
 
 @example
 $ @kbd{m4}
@@ -6388,6 +6394,18 @@ Extensions are ifdef(`__gnu__', `active', `inactive')
 @result{}Extensions are inactive
 @end example
 
+Since the version string is unquoted and can potentially contain macro
+names (for example, a beta release could be numbered @samp{1.9b}), the
address@hidden macro should generally be used via @code{defn}
+rather than directly invoked (@pxref{Defn}).  In general, feature tests
+are more reliable than version number checks, so exercise caution when
+using this macro.
+
address@hidden
+defn(`__m4_version__')
address@hidden@value{VERSION}
address@hidden example
+
 On UNIX systems, @acronym{GNU} @code{m4} will define @address@hidden
 by default, or @code{unix} when the @option{-G} option is specified.
 
diff --git a/src/builtin.c b/src/builtin.c
index c3e4a7e..0ea814a 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -163,6 +163,7 @@ static predefined const predefined_tab[] =
 # warning Platform macro not provided
 #endif
   { NULL,      "__gnu__",      "" },
+  { NULL,      "__m4_version__", VERSION },
 
   { NULL,      NULL,           NULL },
 };
-- 
1.5.6