Re: [PATCH] master: variables: rename namespace as api.namespace.

[Akim Demaille]

>>> "JED" == Joel E Denny <address@hidden> writes:

Hi Joel,

Sorry for the delays.  I sometimes wished we use a ticket system,
sometimes it's not entirely trivial which threads are closed, and
which ones still need changes.  Maybe we should resurrect TODO and
keep it up to date.

 > On Wed, 24 Jun 2009, Akim Demaille wrote:
 >> Discussed in
 >> http://lists.gnu.org/archive/html/bison-patches/2009-04/msg00033.html
 >> 
 >> * data/bison.m4 (b4_percent_define_copy_, b4_percent_define_copy):
 >> New.
 >> (b4_percent_define_use): New.
 >> Use it where applicable.
 >> * data/c++.m4: Replace uses of the variable "namespace" by
 >> "api.namespace".
 >> Default the latter to the former.

 > You've handled this rename differently than I handled the rename of 
 > api.push_pull and lr.keep_unreachable_states to contain dashes instead of 
 > underscores.  I'm not convinced my approach is better, but we ought to 
 > consider whether just one of our approaches is sufficient for all these 
 > renames.

 > In muscle_percent_define_insert, I simply implemented the old variables as 
 > aliases for the new variables.  A disadvantage is that error messages for 
 > the old variables are reported for the new variables instead.  That might 
 > confuse the user a bit, but the names are fairly similar and we usually 
 > report line and column numbers in our error messages.

 > Your implementation is slightly more complex.  Another disadvantage of 
 > yours is that, if both the old and new name is defined, the old one is 
 > reported as unused with no indication of why.

You are right.  I don't know what I chose this path.  Maybe I did not
see your changes, and making the change in the back-end appeared to be
the safest means to get all the possibilities.

Anyway, I have installed the following patch.  It changes the way you
treated the two previous obsolete names, but I think it is simpler and
more general this way.  We could also easily issue warnings for
obsolete names at this point, which would help addressing your point
in the second paragraph: this way the user would be less surprised to
hear about a variable name she did not enter.


>From 171ad99d6421935a278656be6dc7161591835d00 Mon Sep 17 00:00:00 2001
From: Akim Demaille <address@hidden>
Date: Wed, 19 Aug 2009 14:24:15 +0200
Subject: [PATCH] variables: simplify the upgrade of namespace into 
api.namespace.

This patch simplifies "variables: rename namespace as api.namespace",
commit 67501061076ba46355cfd9f9361c7eed861b389c.
Suggested by Joel E. Denny in
http://lists.gnu.org/archive/html/bison-patches/2009-07/msg00006.html

        * src/muscle-tab.c (muscle_percent_variable_update): New.
        (muscle_percent_define_insert): Use it in replacement of the
        previous tr invocation.
        Remove variable_tr, no longer needed.
        * data/bison.m4 (b4_percent_define_copy_, b4_percent_define_copy):
        Remove.
        * data/c++.m4: No longer handle namespace -> api.namespace.
        * tests/input.at (%define backward compatibility): Check that
        namespace is treated as api.namespace.
---
 ChangeLog        |   19 +++++++++++++++++++
 data/bison.m4    |   24 ------------------------
 data/c++.m4      |    9 +--------
 src/muscle-tab.c |   40 ++++++++++++++++++++++++++--------------
 tests/input.at   |   11 +++++++++++
 5 files changed, 57 insertions(+), 46 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9f54a65..903d267 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2009-08-19  Akim Demaille  <address@hidden>
 
+       variables: simplify the upgrade of namespace into api.namespace.
+
+       This patch simplifies "variables: rename namespace as
+       api.namespace", commit 67501061076ba46355cfd9f9361c7eed861b389c.
+       Suggested by Joel E. Denny in
+       http://lists.gnu.org/archive/html/bison-patches/2009-07/msg00006.html
+
+       * src/muscle-tab.c (muscle_percent_variable_update): New.
+       (muscle_percent_define_insert): Use it in replacement of the
+       previous tr invocation.
+       Remove variable_tr, no longer needed.
+       * data/bison.m4 (b4_percent_define_copy_, b4_percent_define_copy):
+       Remove.
+       * data/c++.m4: No longer handle namespace -> api.namespace.
+       * tests/input.at (%define backward compatibility): Check that
+       namespace is treated as api.namespace.
+
+2009-08-19  Akim Demaille  <address@hidden>
+
        doc: %initial-action to initialize yylloc.
        Reported by Bill Allombert.
        * doc/bison.texinfo: Set fill-column to 76.
diff --git a/data/bison.m4 b/data/bison.m4
index 788e8b0..4ca6cee 100644
--- a/data/bison.m4
+++ b/data/bison.m4
@@ -565,30 +565,6 @@ m4_define([b4_percent_define_use],
 [m4_define([b4_percent_define_bison_variables(]$1[)])dnl
 ])
 
-
-# b4_percent_define_copy_(SOURCE, DESTINATION, [|_loc|_syncline])
-# ---------------------------------------------------------------
-# Copy the value/loc/syncline from the variable SOURCE to the DESTINATION.
-# Ignore undefined values.  "Use" the SOURCE so that there are no
-# complaints about unused variables.
-m4_define([b4_percent_define_copy_],
-[m4_ifdef([b4_percent_define$3(]$1[)],
-          [m4_define([b4_percent_define$3(]$2[)],
-                     m4_defn([b4_percent_define$3(]$1[)]))])dnl
-])
-
-
-# b4_percent_define_copy(SOURCE, DESTINATION)
-# -------------------------------------------
-# Define the variable DESTINATION as a copy of SOURCE.
-m4_define([b4_percent_define_copy],
-[b4_percent_define_use([$1])dnl
-b4_percent_define_copy_([$1], [$2], [])dnl
-b4_percent_define_copy_([$1], [$2], [_loc])dnl
-b4_percent_define_copy_([$1], [$2], [_syncline])dnl
-])
-
-
 # b4_percent_define_get(VARIABLE)
 # -------------------------------
 # Mimic muscle_percent_define_get in ../src/muscle_tab.h exactly.  That is, if
diff --git a/data/c++.m4 b/data/c++.m4
index 1bde73e..946d9b6 100644
--- a/data/c++.m4
+++ b/data/c++.m4
@@ -27,14 +27,7 @@ m4_include(b4_pkgdatadir/[c.m4])
 b4_percent_define_default([[parser_class_name]], [[parser]])
 b4_percent_define_default([[location_type]], [[location]])
 b4_percent_define_default([[filename_type]], [[std::string]])
-
-# api.namespace defaults to namespace, and then to b4_prefix.
-b4_percent_define_ifdef([api.namespace],
-   [],
-   [b4_percent_define_ifdef([namespace],
-       [b4_percent_define_copy([namespace], [api.namespace])],
-       [b4_percent_define_default([api.namespace],
-                                  m4_defn([b4_prefix]))])])
+b4_percent_define_default([[api.namespace]], m4_defn([b4_prefix]))
 
 b4_percent_define_default([[global_tokens_and_yystype]], [[false]])
 b4_percent_define_default([[define_location_comparison]],
diff --git a/src/muscle-tab.c b/src/muscle-tab.c
index c78e3f8..bfb7803 100644
--- a/src/muscle-tab.c
+++ b/src/muscle-tab.c
@@ -389,6 +389,30 @@ muscle_user_name_list_grow (char const *key, char const 
*user_name,
   muscle_grow (key, "]]", "");
 }
 
+/** If the \a variable name is obsolete, return the name to use,
+ * otherwise \a variable. */
+static
+char const *
+muscle_percent_variable_update (char const *variable)
+{
+  typedef struct
+  {
+    const char *obsolete;
+    const char *updated;
+  } conversion_type;
+  const conversion_type conversion[] =
+    {
+      { "api.push_pull", "api.push-pull", },
+      { "lr.keep_unreachable_states", "lr.keep-unreachable-states", },
+      { "namespace", "api.namespace", },
+    };
+  int i;
+  for (i = 0; i < sizeof conversion / sizeof *conversion; ++i)
+    if (!strcmp (conversion[i].obsolete, variable))
+      return conversion[i].updated;
+  return variable;
+}
+
 #define MUSCLE_USER_NAME_CONVERT(NAME, PREFIX, USER_NAME, SUFFIX)    \
 do {                                                                 \
   char *tmp;                                                         \
@@ -406,20 +430,13 @@ muscle_percent_define_insert (char const *variable, 
location variable_loc,
                               char const *value,
                               muscle_percent_define_how how)
 {
-  char *variable_tr = NULL;
   char const *name;
   char const *loc_name;
   char const *syncline_name;
   char const *how_name;
 
   /* Permit certain names with underscores for backward compatibility.  */
-  if (0 == strcmp (variable, "api.push_pull")
-      || 0 == strcmp (variable, "lr.keep_unreachable_states"))
-    {
-      variable_tr = strdup (variable);
-      tr (variable_tr, '_', '-');
-      variable = variable_tr;
-    }
+  variable = muscle_percent_variable_update (variable);
 
   MUSCLE_USER_NAME_CONVERT (name, "percent_define(", variable, ")");
   MUSCLE_USER_NAME_CONVERT (loc_name, "percent_define_loc(", variable, ")");
@@ -434,10 +451,7 @@ muscle_percent_define_insert (char const *variable, 
location variable_loc,
       muscle_percent_define_how how_old =
         atoi (muscle_find_const (how_name));
       if (how_old == MUSCLE_PERCENT_DEFINE_F)
-        {
-          free (variable_tr);
-          return;
-        }
+        return;
       complain_at (variable_loc, _("%s `%s' redefined"),
                    "%define variable", variable);
       complain_at (muscle_percent_define_get_loc (variable),
@@ -452,8 +466,6 @@ muscle_percent_define_insert (char const *variable, 
location variable_loc,
   muscle_user_name_list_grow ("percent_define_user_variables", variable,
                               variable_loc);
   MUSCLE_INSERT_INT (how_name, how);
-
-  free (variable_tr);
 }
 
 /* This is used for backward compatibility, e.g., "%define api.pure"
diff --git a/tests/input.at b/tests/input.at
index 146d581..810e760 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -1056,6 +1056,17 @@ AT_BISON_CHECK([[input.y]], [1], [],
 ]])
 
 AT_DATA([[input.y]],
+[[%define namespace "foo"
+%define api.namespace "foo"
+%%
+start: ;
+]])
+AT_BISON_CHECK([[input.y]], [1], [],
+[[input.y:2.9-21: %define variable `api.namespace' redefined
+input.y:1.9-17: previous definition
+]])
+
+AT_DATA([[input.y]],
 [[%define foo_bar "baz"
 %%
 start: ;
-- 
1.6.4