Re: outdated m4sugar

[Joel E. Denny]

On Sat, 12 Jul 2008, Joel E. Denny wrote:

> > | Patching Bison so that it works with either LIFO or FIFO m4wrap is
> > | straight-forward.  I just removed the code in m4_wrap in m4_init and
> > | rewrote Bison to invoke it after processing the skeletons.  Only one use
> > | of m4wrap remains, and so order no longer matters.  Bison's make check

> I've updated my m4sugar.m4 patch to rename whichever of m4symbols or 
> symbols is defined, so this should work in all cases.

> > | I also had to make Bison pass -g to m4 just in case POSIXLY_CORRECT is
> > | set.  This does not work with m4 1.4.6, but Bison could just unset
> > | POSIXLY_CORRECT when invoking m4 if we need to support m4 1.4.6 for some
> > | reason.

The following patch, not yet pushed, implements the above as well as the 
-dV fix you mentioned.  Let me know if you see any problems or if you 
think there's a better solution.

Thanks again for looking out for us, Eric.

>From c8eab8147b16375626c2e76569510fb75d80b591 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Tue, 15 Jul 2008 03:33:47 -0400
Subject: [PATCH] Ensure forward-compatibility with GNU M4.

Reported by Eric Blake at
<http://lists.gnu.org/archive/html/bug-bison/2008-07/msg00000.html>.
* data/m4sugar/m4sugar.m4 (m4_symbols): Rename from either symbols or
m4symbols, depending on which is already defined.
(m4_wrap): Eliminate by moving wrapped code, which dealt with cleaning
up the diversion stack, to...
* data/cleanup.m4: ... this new file.  Only one m4_wrap use remains in
the skeletons (in bison.m4 to check %define and %code usage in the
skeletons), so m4_wrap's future switch from LIFO to FIFO order will be
irrelevant.
* src/output.c (output_skeleton): For the m4 invocation, pass
../data/cleanup.m4, pass -dV first because it may become
position-independent, and unset POSIXLY_CORRECT so Bison's skeletons
have access to GNU M4 extensions.  Add comments explaining these
issues in more detail.
---
 ChangeLog               |   18 ++++++++++++
 data/cleanup.m4         |    4 +++
 data/m4sugar/m4sugar.m4 |   10 ++-----
 src/output.c            |   69 +++++++++++++++++++++++++++++++++++-----------
 4 files changed, 77 insertions(+), 24 deletions(-)
 create mode 100644 data/cleanup.m4

diff --git a/ChangeLog b/ChangeLog
index d450038..32910ec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2008-07-15  Joel E. Denny  <address@hidden>
+
+       Ensure forward-compatibility with GNU M4.  Reported by Eric Blake at
+       <http://lists.gnu.org/archive/html/bug-bison/2008-07/msg00000.html>.
+       * data/m4sugar/m4sugar.m4 (m4_symbols): Rename from either symbols or
+       m4symbols, depending on which is already defined.
+       (m4_wrap): Eliminate by moving wrapped code, which dealt with cleaning
+       up the diversion stack, to...
+       * data/cleanup.m4: ... this new file.  Only one m4_wrap use remains in
+       the skeletons (in bison.m4 to check %define and %code usage in the
+       skeletons), so m4_wrap's future switch from LIFO to FIFO order will be
+       irrelevant.
+       * src/output.c (output_skeleton): For the m4 invocation, pass
+       ../data/cleanup.m4, pass -dV first because it may become
+       position-independent, and unset POSIXLY_CORRECT so Bison's skeletons
+       have access to GNU M4 extensions.  Add comments explaining these
+       issues in more detail.
+
 2008-07-14  Joel E. Denny  <address@hidden>
 
        Add .gitignore everywhere based on .cvsignore.
diff --git a/data/cleanup.m4 b/data/cleanup.m4
new file mode 100644
index 0000000..9afb158
--- /dev/null
+++ b/data/cleanup.m4
@@ -0,0 +1,4 @@
+# Check the divert push/pop perfect balance.
+m4_divert_pop([KILL])
+m4_ifdef([_m4_divert_diversion],
+         [m4_fatal([$0: unbalanced m4_divert_push:]_m4_divert_n_stack)])
diff --git a/data/m4sugar/m4sugar.m4 b/data/m4sugar/m4sugar.m4
index f3a0901..f939b5d 100644
--- a/data/m4sugar/m4sugar.m4
+++ b/data/m4sugar/m4sugar.m4
@@ -146,7 +146,8 @@ m4_rename([regexp], [m4_bregexp])
 m4_rename_m4([shift])
 m4_undefine([sinclude])
 m4_rename_m4([substr])
-m4_rename_m4([symbols])
+m4_ifdef([symbols], [m4_rename_m4([symbols])])
+m4_ifdef([m4symbols], [m4_rename([m4symbols], [m4_symbols])])
 m4_rename_m4([syscmd])
 m4_rename_m4([sysval])
 m4_rename_m4([traceoff])
@@ -1764,11 +1765,6 @@ m4_define([m4_init],
 # for sake of simplicity.
 m4_pattern_forbid([^_?m4_])
 m4_pattern_forbid([^dnl$])
-
-# Check the divert push/pop perfect balance.
-m4_wrap([m4_ifdef([_m4_divert_diversion],
-          [m4_fatal([$0: unbalanced m4_divert_push:]_m4_divert_n_stack)])[]])
-
 m4_divert_push([KILL])
-m4_wrap([m4_divert_pop([KILL])[]])
+# Bison pops KILL in ../close.m4.
 ])
diff --git a/src/output.c b/src/output.c
index 043298b..8cfb93a 100644
--- a/src/output.c
+++ b/src/output.c
@@ -473,18 +473,17 @@ output_skeleton (void)
   FILE *in;
   FILE *out;
   int filter_fd[2];
-  char const *argv[7];
+  char const *argv[8];
   pid_t pid;
 
-  /* Compute the names of the package data dir and skeleton file.
-     Test whether m4sugar.m4 is readable, to check for proper
-     installation.  A faulty installation can cause deadlock, so a
-     cheap sanity check is worthwhile.  */
+  /* Compute the names of the package data dir and skeleton files.  */
   char const m4sugar[] = "m4sugar/m4sugar.m4";
   char const m4bison[] = "bison.m4";
+  char const m4cleanup[] = "cleanup.m4";
   char *full_m4sugar;
   char *full_m4bison;
   char *full_skeleton;
+  char *full_m4cleanup;
   char const *p;
   char const *m4 = (p = getenv ("M4")) ? p : M4;
   char const *pkgdatadir = compute_pkgdatadir ();
@@ -501,31 +500,67 @@ output_skeleton (void)
   full_m4sugar = xstrdup (full_skeleton);
   strcpy (full_skeleton + pkgdatadirlen + 1, m4bison);
   full_m4bison = xstrdup (full_skeleton);
+  strcpy (full_skeleton + pkgdatadirlen + 1, m4cleanup);
+  full_m4cleanup = xstrdup (full_skeleton);
   if (strchr (skeleton, '/'))
     strcpy (full_skeleton, skeleton);
   else
     strcpy (full_skeleton + pkgdatadirlen + 1, skeleton);
+
+  /* Test whether m4sugar.m4 is readable, to check for proper
+     installation.  A faulty installation can cause deadlock, so a
+     cheap sanity check is worthwhile.  */
   xfclose (xfopen (full_m4sugar, "r"));
 
   /* Create an m4 subprocess connected to us via two pipes.  */
 
   if (trace_flag & trace_tools)
-    fprintf (stderr, "running: %s %s - %s %s\n",
-            m4, full_m4sugar, full_m4bison, full_skeleton);
-
-  argv[0] = m4;
-  argv[1] = full_m4sugar;
-  argv[2] = "-";
-  argv[3] = full_m4bison;
-  argv[4] = full_skeleton;
-  argv[5] = trace_flag & trace_m4 ? "-dV" : NULL;
-  argv[6] = NULL;
-
+    fprintf (stderr, "running: %s %s - %s %s %s\n",
+             m4, full_m4sugar, full_m4bison, full_skeleton, full_m4cleanup);
+
+  /* Some future version of GNU M4 (most likely 1.6) may treat the -dV in a
+     position-dependent manner.  Keep it as the first argument so that all
+     files are traced.
+     
+     See the thread starting at
+     <http://lists.gnu.org/archive/html/bug-bison/2008-07/msg00000.html>
+     for details.  */
+  {
+    int i = 0;
+    argv[i++] = m4;
+    if (trace_flag & trace_m4)
+      argv[i++] = "-dV";
+    argv[i++] = full_m4sugar;
+    argv[i++] = "-";
+    argv[i++] = full_m4bison;
+    argv[i++] = full_skeleton;
+    argv[i++] = full_m4cleanup;
+    argv[i++] = NULL;
+  }
+  /* When POSIXLY_CORRECT is set, some future versions of GNU M4 (most likely
+     2.0) may drop some of the GNU extensions that Bison's skeletons depend
+     upon.  So that the next release of Bison is forward compatible with those
+     future versions of GNU M4, we unset POSIXLY_CORRECT here.
+
+     FIXME: A user might set POSIXLY_CORRECT to affect processes run from
+     macros like m4_sycmd in a custom skeleton.  For now, Bison makes no
+     promises about the behavior of custom skeletons, so this scenario not a
+     concern.  However, we eventually want to eliminate this shortcoming.  The
+     next release of GNU M4 (1.4.12 or 1.6) will accept the -g command-line
+     option, which in a later release will indicate that POSIXLY_CORRECT should
+     be ignored.  Once the GNU M4 that accepts -g is pervasive, Bison should
+     use -g instead of unsetting POSIXLY_CORRECT.
+     
+     See the thread starting at
+     <http://lists.gnu.org/archive/html/bug-bison/2008-07/msg00000.html>
+     for details.  */
+  unsetenv ("POSIXLY_CORRECT");
   init_subpipe ();
   pid = create_subpipe (argv, filter_fd);
-  free (full_m4bison);
   free (full_m4sugar);
+  free (full_m4bison);
   free (full_skeleton);
+  free (full_m4cleanup);
 
   out = fdopen (filter_fd[0], "w");
   if (! out)
-- 
1.5.4.3