Re: argv_ref patch 19: support builtin tokens in more macros

Eric Blake <ebb9 <at> byu.net> writes:

> 
> Second, comparison of macro definitions is now transparent to how those
> macros are implemented.  A user no longer needs to know whether a macro is
> builtin or not.  For example,
> ~ ifelse(defn(`divnum'),defn(`dnl'),yes,no)
> now results in 'no' (those two macros are quite different) rather than yes
> (those two macros are builtins, both got flattened to '', and two empty
> strings are equal).

I should have tested this with longer macro names.  It works fine on branch-
1.6, but on master, if the macro name is long enough and was passed through a 
back-reference, the code mistakenly decided the back-reference was not a valid 
macro name.  Which in turn spectacularly breaks autoconf:

$ M4=~/m4-head/build/tests/m4 autoconf -f
m4:aclocal.m4:14: Warning: m4_ifdef: invalid macro name ignored
aclocal.m4:14: error: m4_defn: undefined macro: m4_PACKAGE_VERSION
aclocal.m4:14: the top level
autom4te: /home/eblake/m4-head/build/tests/m4 failed with exit status: 1

So I guess that means no one has been using m4 1.9a.x with autoconf for the 
past 3 weeks?  (Even worse, that points the finger at me, for not testing 
autoconf 2.62 with m4 1.9a.x?  :)

And in the process of fixing this, I also discovered an automake bug:
http://thread.gmane.org/gmane.comp.sysutils.automake.patches/3141
and an autoconf manual bug:
http://thread.gmane.org/gmane.comp.sysutils.autoconf.patches/5515
In general, you should never expand AC_AUTOCONF_VERSION, but use m4_defn
([AC_AUTOCONF_VERSION]) instead.  I also found another regression in m4's 
gettext support.

From e098cd5590ef999997130522698cbccc9e083930 Mon Sep 17 00:00:00 2001
From: Eric Blake <[email protected]>
Date: Thu, 10 Apr 2008 11:09:03 -0600
Subject: [PATCH] Be namespace clean for M4 version; fixes 2008-04-08 regression.

* configure.ac (version): Rename...
(M4_VERSION): ...to this, since using 'version' broke po.m4.

Signed-off-by: Eric Blake <[email protected]>
---
 ChangeLog    |    6 ++++++
 configure.ac |   10 ++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1ecc606..c857a44 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2008-04-10  Eric Blake  <[email protected]>
+
+	Be namespace clean for M4 version; fixes 2008-04-08 regression.
+	* configure.ac (version): Rename...
+	(M4_VERSION): ...to this, since using 'version' broke po.m4.
+
 2008-04-09  Eric Blake  <[email protected]>
 
 	Mention 1.4.11 release.
diff --git a/configure.ac b/configure.ac
index fdac14d..b4a4515 100644
--- a/configure.ac
+++ b/configure.ac
@@ -23,9 +23,11 @@ AC_PREREQ([2.61a.347])
 ## ------------------------ ##
 ## Autoconf initialization. ##
 ## ------------------------ ##
-m4_define([version], m4_esyscmd([build-aux/git-version-gen .tarball-version]))
-m4_bmatch(m4_defn([version]), [^[0-9]], [], [m4_define([version], [1.9a])])
-AC_INIT([GNU M4], m4_defn([version]), [[email protected]])
+m4_define([M4_VERSION],
+	  m4_esyscmd([build-aux/git-version-gen .tarball-version]))
+m4_bmatch(m4_defn([M4_VERSION]), [^[0-9]], [],
+	  [m4_define([M4_VERSION], [1.9a])])
+AC_INIT([GNU M4], m4_defn([M4_VERSION]), [[email protected]])
 
 AC_CONFIG_SRCDIR([src/m4.h])
 AC_CONFIG_AUX_DIR([build-aux])
@@ -55,7 +57,7 @@ M4_default_preload="M4_DEFAULT_PRELOAD"
 ## Automake Initialization. ##
 ## ------------------------ ##
 AM_INIT_AUTOMAKE([1.10.1 subdir-objects dist-bzip2 dist-lzma]
-m4_bmatch(m4_defn([version]), [-], [gnu], [gnits]))
+m4_bmatch(m4_defn([M4_VERSION]), [-], [gnu], [gnits]))
 
 
 
-- 
1.5.5


From 38f06945a35f382d5f7ab12d8d32184d544ba234 Mon Sep 17 00:00:00 2001
From: Eric Blake <[email protected]>
Date: Thu, 10 Apr 2008 11:51:27 -0600
Subject: [PATCH] Allow back-referenced macro names; fixes 2008-03-13 regression.

* m4/m4module.h (m4_symbol_value_lookup): Change prototype.
* m4/utility.c (m4_symbol_value_lookup): Change signature.
* modules/m4.c (undefine, popdef, ifdef, m4_dump_symbols, defn):
Adjust all callers.
* tests/others.at (ifndef): New test.

Signed-off-by: Eric Blake <[email protected]>
---
 ChangeLog       |    7 +++++++
 m4/m4module.h   |    2 +-
 m4/utility.c    |    8 ++++----
 modules/m4.c    |   26 ++++++++------------------
 tests/others.at |   33 +++++++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c857a44..26906ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2008-04-10  Eric Blake  <[email protected]>
 
+	Allow back-referenced macro names; fixes 2008-03-13 regression.
+	* m4/m4module.h (m4_symbol_value_lookup): Change prototype.
+	* m4/utility.c (m4_symbol_value_lookup): Change signature.
+	* modules/m4.c (undefine, popdef, ifdef, m4_dump_symbols, defn):
+	Adjust all callers.
+	* tests/others.at (ifndef): New test.
+
 	Be namespace clean for M4 version; fixes 2008-04-08 regression.
 	* configure.ac (version): Rename...
 	(M4_VERSION): ...to this, since using 'version' broke po.m4.
diff --git a/m4/m4module.h b/m4/m4module.h
index 357baca..5c1f4e8 100644
--- a/m4/m4module.h
+++ b/m4/m4module.h
@@ -165,7 +165,7 @@ extern bool	m4_bad_argc	   (m4 *, int, const char *, 
size_t, size_t,
 extern bool	m4_numeric_arg	   (m4 *, const char *, const char *, int *);
 extern bool	m4_parse_truth_arg (m4 *, const char *, const char *, bool);
 extern m4_symbol *m4_symbol_value_lookup (m4 *, const char *,
-					  m4_symbol_value *, bool);
+					  m4_macro_args *, size_t, bool);
 
 /* Error handling.  */
 extern void m4_error (m4 *, int, int, const char *, const char *, ...)
diff --git a/m4/utility.c b/m4/utility.c
index 1e17d61..2cd4d18 100644
--- a/m4/utility.c
+++ b/m4/utility.c
@@ -125,12 +125,12 @@ m4_parse_truth_arg (m4 *context, const char *arg, const 
char *me,
    result of the lookup, or NULL.  */
 m4_symbol *
 m4_symbol_value_lookup (m4 *context, const char *caller,
-			m4_symbol_value *value, bool must_exist)
+			m4_macro_args *argv, size_t i, bool must_exist)
 {
   m4_symbol *result = NULL;
-  if (m4_is_symbol_value_text (value))
+  if (m4_is_arg_text (argv, i))
     {
-      const char *name = m4_get_symbol_value_text (value);
+      const char *name = M4ARG (i);
       result = m4_symbol_lookup (M4SYMTAB, name);
       if (must_exist && !result)
 	m4_warn (context, 0, caller, _("undefined macro `%s'"), name);
@@ -153,7 +153,7 @@ m4_verror_at_line (m4 *context, bool warn, int status, int 
errnum,
   char *full = NULL;
   char *safe_macro = NULL;
 
-  /* Sanitize MACRO, sinze we are turning around and using it in a
+  /* Sanitize MACRO, since we are turning around and using it in a
      format string.  The allocation is overly conservative, but
      problematic macro names only occur via indir or changesyntax.  */
   if (macro && strchr (macro, '%'))
diff --git a/modules/m4.c b/modules/m4.c
index 02ac090..d484f4d 100644
--- a/modules/m4.c
+++ b/modules/m4.c
@@ -169,11 +169,8 @@ M4BUILTIN_HANDLER (undefine)
   const char *me = M4ARG (0);
   size_t i;
   for (i = 1; i < argc; i++)
-    {
-      m4_symbol_value *value = m4_arg_symbol (argv, i);
-      if (m4_symbol_value_lookup (context, me, value, true))
-	m4_symbol_delete (M4SYMTAB, m4_get_symbol_value_text (value));
-    }
+    if (m4_symbol_value_lookup (context, me, argv, i, true))
+      m4_symbol_delete (M4SYMTAB, M4ARG (i));
 }
 
 M4BUILTIN_HANDLER (pushdef)
@@ -194,11 +191,8 @@ M4BUILTIN_HANDLER (popdef)
   const char *me = M4ARG (0);
   size_t i;
   for (i = 1; i < argc; i++)
-    {
-      m4_symbol_value *value = m4_arg_symbol (argv, i);
-      if (m4_symbol_value_lookup (context, me, value, true))
-	m4_symbol_popdef (M4SYMTAB, m4_get_symbol_value_text (value));
-    }
+    if (m4_symbol_value_lookup (context, me, argv, i, true))
+      m4_symbol_popdef (M4SYMTAB, M4ARG (i));
 }
 
 
@@ -209,8 +203,7 @@ M4BUILTIN_HANDLER (popdef)
 M4BUILTIN_HANDLER (ifdef)
 {
   m4_push_arg (context, obs, argv,
-	       (m4_symbol_value_lookup (context, M4ARG (0),
-					m4_arg_symbol (argv, 1), false)
+	       (m4_symbol_value_lookup (context, M4ARG (0), argv, 1, false)
 		? 2 : 3));
 }
 
@@ -308,11 +301,9 @@ m4_dump_symbols (m4 *context, m4_dump_symbol_data *data, 
size_t argc,
 
       for (i = 1; i < argc; i++)
 	{
-	  m4_symbol_value *value = m4_arg_symbol (argv, i);
-	  symbol = m4_symbol_value_lookup (context, me, value, complain);
+	  symbol = m4_symbol_value_lookup (context, me, argv, i, complain);
 	  if (symbol)
-	    dump_symbol_CB (NULL, m4_get_symbol_value_text (value), symbol,
-			    data);
+	    dump_symbol_CB (NULL, M4ARG (i), symbol, data);
 	}
     }
 
@@ -365,8 +356,7 @@ M4BUILTIN_HANDLER (defn)
 
   for (i = 1; i < argc; i++)
     {
-      m4_symbol_value *value = m4_arg_symbol (argv, i);
-      m4_symbol *symbol = m4_symbol_value_lookup (context, me, value, true);
+      m4_symbol *symbol = m4_symbol_value_lookup (context, me, argv, i, true);
 
       if (!symbol)
 	;
diff --git a/tests/others.at b/tests/others.at
index fbd692b..22e8b99 100644
--- a/tests/others.at
+++ b/tests/others.at
@@ -273,6 +273,39 @@ Move one disk from source to destination.
 AT_CLEANUP
 
 
+## ------ ##
+## ifndef ##
+## ------ ##
+
+AT_SETUP([ifndef])
+
+dnl This catches a bug added 2008-03-13, fixed 2008-04-10.
+AT_DATA([in.m4],
+[[define(`ifndef', `ifdef(`$1', `$3', `$2')')dnl
+define(`a_really_long_name', `1')dnl
+ifdef(`divnum', `yes', `no')
+ifndef(`divnum', `yes', `no')
+ifdef(`ifndef', `yes', `no')
+ifndef(`ifndef', `yes', `no')
+ifdef(`a_really_long_name', `yes', `no')
+ifndef(`a_really_long_name', `yes', `no')
+ifdef(`no_such', `yes', `no')
+ifndef(`no_such', `yes', `no')
+]])
+
+AT_CHECK_M4([in.m4], [0],
+[[yes
+no
+yes
+no
+yes
+no
+no
+yes
+]])
+
+AT_CLEANUP
+
 
 ## ------- ##
 ## iso8859 ##
-- 
1.5.5