Re: Emacs rewrite in a maintainable language

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

On 10/12/2015 08:17 AM, Oleh Krehel wrote:
> Quoting the Emacs sources:
>
>      # define MIN(a,b) ((a) < (b) ? (a) : (b))
>      #define streq(X, Y) (*(X) == *(Y) && strcmp ((X) + 1, (Y) + 1) == 0)
>      #define SET_FLAG(F, FLAG)	((F) |= (FLAG))
>      #define HAS_FLAG(F, FLAG)	(((F) & (FLAG)) != 0)
>      #define xnew(n, Type)      ((Type *) xmalloc ((n) * sizeof (Type)))
>      #define xrnew(op, n, Type) ((op) = (Type *) xrealloc (op, (n) * sizeof (Type)))
>      #define switch_line_buffers() (curndx = 1 - curndx)
>      #define curlinepos (lbs[curndx].linepos)
>      #define BVAR(buf, field) ((buf)->field ## _)
>
> Some of these can be replaced with plain type checked C++

Only BVAR belongs to the Emacs core. The other macros belong to 
auxiliary executables that presumably would be unaffected by any rewrite 
of the core. Anyway, for fun I just now replaced streq, SET_FLAG, and 
HAS_FLAG with functions, in the attached patch. More work needs to be 
done in this area but one step at a time....

[-- Attachment #2: 0001-Unmacroize-ebrowse.c-and-etags.c-a-bit.patch --]
[-- Type: text/x-patch, Size: 8095 bytes --]

From ddeec8ae168382bc16658f6836489ad26d7cf755 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Oct 2015 15:30:18 -0700
Subject: [PATCH] Unmacroize ebrowse.c and etags.c a bit

* lib-src/ebrowse.c (READ_CHUNK_SIZE): Now an enum constant.
(streq, filename_eq, set_flag, has_flag): Now inline functions.
(set_flag): First arg is now an address, not an lvalue.
All callers changed.
(filename_eq, set_flag, has_flag):
Rename from FILENAME_EQ, SET_FLAG, HAS_FLAG.
All callers changed.
* lib-src/etags.c (streq, strcaseeq, strneq, strncaseeq):
Now inline functions.  Remove asserts that are unnecessary these
days (and in some cases were too-generous anyway).
---
 lib-src/ebrowse.c | 84 ++++++++++++++++++++++++++++++++-----------------------
 lib-src/etags.c   | 27 +++++++++++++++---
 2 files changed, 72 insertions(+), 39 deletions(-)

diff --git a/lib-src/ebrowse.c b/lib-src/ebrowse.c
index 09edc7d..f2093ae 100644
--- a/lib-src/ebrowse.c
+++ b/lib-src/ebrowse.c
@@ -32,27 +32,32 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #define SEEK_END 2
 #endif
 
-/* Conditionalize function prototypes.  */
-
-/* Value is non-zero if strings X and Y compare equal.  */
-
-#define streq(X, Y) (*(X) == *(Y) && strcmp ((X) + 1, (Y) + 1) == 0)
-
 #include <min-max.h>
 
 /* Files are read in chunks of this number of bytes.  */
 
-#define READ_CHUNK_SIZE (100 * 1024)
+enum { READ_CHUNK_SIZE = 100 * 1024 };
 
-#if defined (__MSDOS__)
-#define FILENAME_EQ(X,Y)    (strcasecmp (X,Y) == 0)
-#else
-#if defined (WINDOWSNT)
-#define FILENAME_EQ(X,Y)    (stricmp (X,Y) == 0)
+/* Value is true if strings X and Y compare equal.  */
+
+static bool
+streq (char const *x, char const *y)
+{
+  return strcmp (x, y) == 0;
+}
+
+static bool
+filename_eq (char const *x, char const *y)
+{
+#ifdef __MSDOS__
+  return strcasecmp (x, y) == 0;
+#elif defined WINDOWSNT
+  return stricmp (x, y) == 0;
 #else
-#define FILENAME_EQ(X,Y)    (streq (X,Y))
-#endif
+  return streq (x, y);
 #endif
+}
+
 /* The default output file name.  */
 
 #define DEFAULT_OUTFILE "BROWSE"
@@ -217,10 +222,19 @@ enum visibility
 #define F_EXTERNC	256	/* Is declared extern "C".  */
 #define F_DEFINE	512	/* Is a #define.  */
 
-/* Two macros to set and test a bit in an int.  */
+/* Set and test a bit in an int.  */
 
-#define SET_FLAG(F, FLAG)	((F) |= (FLAG))
-#define HAS_FLAG(F, FLAG)	(((F) & (FLAG)) != 0)
+static void
+set_flag (int *f, int flag)
+{
+  *f |= flag;
+}
+
+static bool
+has_flag (int f, int flag)
+{
+  return (f & flag) != 0;
+}
 
 /* Structure describing a class member.  */
 
@@ -682,7 +696,7 @@ add_member_decl (struct sym *cls, char *name, char *regexp, int pos, unsigned in
     m = add_member (cls, name, var, sc, hash);
 
   /* Have we seen a new filename?  If so record that.  */
-  if (!cls->filename || !FILENAME_EQ (cls->filename, filename))
+  if (!cls->filename || !filename_eq (cls->filename, filename))
     m->filename = filename;
 
   m->regexp = regexp;
@@ -745,7 +759,7 @@ add_member_defn (struct sym *cls, char *name, char *regexp, int pos, unsigned in
   if (!cls->sfilename)
     cls->sfilename = filename;
 
-  if (!FILENAME_EQ (cls->sfilename, filename))
+  if (!filename_eq (cls->sfilename, filename))
     m->def_filename = filename;
 
   m->def_regexp = regexp;
@@ -830,7 +844,7 @@ add_global_decl (char *name, char *regexp, int pos, unsigned int hash, int var,
   if (!found)
     {
       if (!global_symbols->filename
-	  || !FILENAME_EQ (global_symbols->filename, filename))
+	  || !filename_eq (global_symbols->filename, filename))
 	m->filename = filename;
 
       m->regexp = regexp;
@@ -931,11 +945,11 @@ mark_virtual (struct sym *r)
   for (p = r->subs; p; p = p->next)
     {
       for (m = r->fns; m; m = m->next)
-        if (HAS_FLAG (m->flags, F_VIRTUAL))
+        if (has_flag (m->flags, F_VIRTUAL))
           {
             for (m2 = p->sym->fns; m2; m2 = m2->next)
               if (m->param_hash == m2->param_hash && streq (m->name, m2->name))
-                SET_FLAG (m2->flags, F_VIRTUAL);
+                set_flag (&m2->flags, F_VIRTUAL);
           }
 
       mark_virtual (p->sym);
@@ -1159,7 +1173,7 @@ sym_scope_1 (struct sym *p)
   strcpy (scope_buffer + scope_buffer_len, p->name);
   scope_buffer_len += len;
 
-  if (HAS_FLAG (p->flags, F_TEMPLATE))
+  if (has_flag (p->flags, F_TEMPLATE))
     {
       ensure_scope_buffer_room (3);
       strcpy (scope_buffer + scope_buffer_len, "<>");
@@ -2435,7 +2449,7 @@ parm_list (int *flags)
         {
           /* We can overload the same function on `const' */
           hash = (hash << 1) ^ CONST;
-          SET_FLAG (*flags, F_CONST);
+          set_flag (flags, F_CONST);
           MATCH ();
         }
 
@@ -2443,7 +2457,7 @@ parm_list (int *flags)
         {
           MATCH ();
           SKIP_MATCHING_IF ('(');
-          SET_FLAG (*flags, F_THROW);
+          set_flag (flags, F_THROW);
         }
 
       if (LOOKING_AT ('='))
@@ -2452,7 +2466,7 @@ parm_list (int *flags)
           if (LOOKING_AT (CINT) && yyival == 0)
             {
               MATCH ();
-              SET_FLAG (*flags, F_PURE);
+              set_flag (flags, F_PURE);
             }
         }
     }
@@ -2505,25 +2519,25 @@ member (struct sym *cls, int vis)
           /* A function or class may follow.  */
         case TEMPLATE:
           MATCH ();
-          SET_FLAG (flags, F_TEMPLATE);
+          set_flag (&flags, F_TEMPLATE);
           /* Skip over template argument list */
           SKIP_MATCHING_IF ('<');
           break;
 
         case EXPLICIT:
-          SET_FLAG (flags, F_EXPLICIT);
+          set_flag (&flags, F_EXPLICIT);
           goto typeseen;
 
         case MUTABLE:
-          SET_FLAG (flags, F_MUTABLE);
+          set_flag (&flags, F_MUTABLE);
           goto typeseen;
 
         case T_INLINE:
-          SET_FLAG (flags, F_INLINE);
+          set_flag (&flags, F_INLINE);
           goto typeseen;
 
         case VIRTUAL:
-          SET_FLAG (flags, F_VIRTUAL);
+          set_flag (&flags, F_VIRTUAL);
           goto typeseen;
 
         case '[':
@@ -2761,7 +2775,7 @@ parse_classname (void)
       if (LOOKING_AT ('<'))
         {
           skip_matching ();
-          SET_FLAG (last_class->flags, F_TEMPLATE);
+          set_flag (&last_class->flags, F_TEMPLATE);
         }
 
       if (!LOOKING_AT (DCOLON))
@@ -3189,7 +3203,7 @@ declaration (int flags)
           break;
 
         case T_INLINE:
-          SET_FLAG (flags, F_INLINE);
+          set_flag (&flags, F_INLINE);
           MATCH ();
           break;
 
@@ -3335,14 +3349,14 @@ globals (int start_flags)
                   MATCH_IF ('}');
                 }
               else
-                SET_FLAG (flags, F_EXTERNC);
+                set_flag (&flags, F_EXTERNC);
             }
           break;
 
         case TEMPLATE:
           MATCH ();
           SKIP_MATCHING_IF ('<');
-          SET_FLAG (flags, F_TEMPLATE);
+          set_flag (&flags, F_TEMPLATE);
           break;
 
         case CLASS: case STRUCT: case UNION:
diff --git a/lib-src/etags.c b/lib-src/etags.c
index 791722d..8b980d3 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -150,10 +150,29 @@ char pot_etags_version[] = "@(#) pot revision number is 17.38.1.4";
 # define CTAGS false
 #endif
 
-#define streq(s,t)	(assert ((s)!=NULL || (t)!=NULL), !strcmp (s, t))
-#define strcaseeq(s,t)	(assert ((s)!=NULL && (t)!=NULL), !c_strcasecmp (s, t))
-#define strneq(s,t,n)	(assert ((s)!=NULL || (t)!=NULL), !strncmp (s, t, n))
-#define strncaseeq(s,t,n) (assert ((s)!=NULL && (t)!=NULL), !c_strncasecmp (s, t, n))
+static bool
+streq (char const *s, char const *t)
+{
+  return strcmp (s, t) == 0;
+}
+
+static bool
+strcaseeq (char const *s, char const *t)
+{
+  return c_strcasecmp (s, t) == 0;
+}
+
+static bool
+strneq (char const *s, char const *t, size_t n)
+{
+  return strncmp (s, t, n) == 0;
+}
+
+static bool
+strncaseeq (char const *s, char const *t, size_t n)
+{
+  return c_strncasecmp (s, t, n) == 0;
+}
 
 /* C is not in a name.  */
 static bool
-- 
2.1.0