[PATCH] Miscellaneous code readability improvements.

[Joel E. Denny]

I pushed this to master and a similar patch to branch-2.5.

>From dfaa48602d4e6d329e3ae9063f44929490928b3d Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Mon, 10 Aug 2009 21:43:07 -0400
Subject: [PATCH] Miscellaneous code readability improvements.

* src/reader.c (reader): Move %define front-end variable
defaults and checking into...
(prepare_percent_define_front_end_variables): ... this new
function.

* src/scan-gram.l (INITIAL): For consistency with string
literals, don't store open quote on character literal.  It's
discarded before returning anyway.
(SC_ESCAPED_CHARACTER): Similarly, don't store close quote.
Make length test more readable, and make the character stored
for an empty literal more obvious while consistent with the
previous behavior.

* src/symtab.c, src/symtab.h: Rename USER_NUMBER_ALIAS to
USER_NUMBER_HAS_STRING_ALIAS throughout.
* src/symtab.c (symbol_make_alias): Remove comment from symtab.c
that is repeated in symtab.h.  Improve argument names to make it
clear which side of the symbol-string alias pair is which.
(symbol_check_alias_consistency): Improve local variable names
for the same purpose.
* src/symtab.h (struct symbol): Make comments about aliases
clearer.
(symbol_make_alias): Improve comments and argument name.
* src/output.c (token_definitions_output): Update for rename to
USER_NUMBER_HAS_STRING_ALIAS and improve comments about aliases.
---
 ChangeLog       |   30 ++++++++++++++++++++
 src/output.c    |    8 +++---
 src/reader.c    |   20 +++++++++-----
 src/scan-gram.l |   32 +++++++++++----------
 src/symtab.c    |   80 ++++++++++++++++++++++++++-----------------------------
 src/symtab.h    |   21 ++++++++------
 6 files changed, 114 insertions(+), 77 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 036120f..2216e7c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,33 @@
+2009-08-10  Joel E. Denny  <address@hidden>
+
+       Miscellaneous code readability improvements.
+
+       * src/reader.c (reader): Move %define front-end variable
+       defaults and checking into...
+       (prepare_percent_define_front_end_variables): ... this new
+       function.
+
+       * src/scan-gram.l (INITIAL): For consistency with string
+       literals, don't store open quote on character literal.  It's
+       discarded before returning anyway.
+       (SC_ESCAPED_CHARACTER): Similarly, don't store close quote.
+       Make length test more readable, and make the character stored
+       for an empty literal more obvious while consistent with the
+       previous behavior.
+
+       * src/symtab.c, src/symtab.h: Rename USER_NUMBER_ALIAS to
+       USER_NUMBER_HAS_STRING_ALIAS throughout.
+       * src/symtab.c (symbol_make_alias): Remove comment from symtab.c
+       that is repeated in symtab.h.  Improve argument names to make it
+       clear which side of the symbol-string alias pair is which.
+       (symbol_check_alias_consistency): Improve local variable names
+       for the same purpose.
+       * src/symtab.h (struct symbol): Make comments about aliases
+       clearer.
+       (symbol_make_alias): Improve comments and argument name.
+       * src/output.c (token_definitions_output): Update for rename to
+       USER_NUMBER_HAS_STRING_ALIAS and improve comments about aliases.
+
 2009-08-08  Alex Rozenman  <address@hidden>
 
        Convert "misleading reference" messages to warnings.
diff --git a/src/output.c b/src/output.c
index c5e9aab..3936aca 100644
--- a/src/output.c
+++ b/src/output.c
@@ -471,10 +471,10 @@ token_definitions_output (FILE *out)
       int number = sym->user_token_number;
       uniqstr id = symbol_id_get (sym);
 
-      /* At this stage, if there are literal aliases, they are part of
-        SYMBOLS, so we should not find symbols which are the aliases
-        here.  */
-      aver (number != USER_NUMBER_ALIAS);
+      /* At this stage, if there are literal string aliases, they are
+         part of SYMBOLS, so we should not find their aliased symbols
+         here.  */
+      aver (number != USER_NUMBER_HAS_STRING_ALIAS);
 
       /* Skip error token and tokens without identifier.  */
       if (sym != errtoken && id)
diff --git a/src/reader.c b/src/reader.c
index 3f224bf..14624df 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -36,6 +36,7 @@
 #include "scan-gram.h"
 #include "scan-code.h"
 
+static void prepare_percent_define_front_end_variables (void);
 static void check_and_convert_grammar (void);
 
 static symbol_list *grammar = NULL;
@@ -590,8 +591,18 @@ reader (void)
   gram_debug = trace_flag & trace_parse;
   gram_scanner_initialize ();
   gram_parse ();
+  prepare_percent_define_front_end_variables ();
 
-  /* Set front-end %define variable defaults.  */
+  if (! complaint_issued)
+    check_and_convert_grammar ();
+
+  xfclose (gram_in);
+}
+
+static void
+prepare_percent_define_front_end_variables (void)
+{
+  /* Set %define front-end variable defaults.  */
   muscle_percent_define_default ("lr.keep-unreachable-states", "false");
   {
     char *lr_type;
@@ -606,7 +617,7 @@ reader (void)
     free (lr_type);
   }
 
-  /* Check front-end %define variables.  */
+  /* Check %define front-end variables.  */
   {
     static char const * const values[] = {
       "lr.type", "LALR", "IELR", "canonical LR", NULL,
@@ -615,11 +626,6 @@ reader (void)
     };
     muscle_percent_define_check_values (values);
   }
-
-  if (! complaint_issued)
-    check_and_convert_grammar ();
-
-  xfclose (gram_in);
 }
 
 
diff --git a/src/scan-gram.l b/src/scan-gram.l
index de93b95..7a6d7bf 100644
--- a/src/scan-gram.l
+++ b/src/scan-gram.l
@@ -261,7 +261,7 @@ splice       (\\[ \f\t\v]*\n)*
   }
 
   /* Characters.  */
-  "'"        STRING_GROW; token_start = loc->start; BEGIN SC_ESCAPED_CHARACTER;
+  "'"        token_start = loc->start; BEGIN SC_ESCAPED_CHARACTER;
 
   /* Strings. */
   "\""       token_start = loc->start; BEGIN SC_ESCAPED_STRING;
@@ -493,16 +493,18 @@ splice     (\\[ \f\t\v]*\n)*
 <SC_ESCAPED_CHARACTER>
 {
   "'"|"\n" {
-    STRING_GROW;
     STRING_FINISH;
     loc->start = token_start;
-    val->character = last_string[1];
+    val->character = last_string[0];
     {
       /* FIXME: Eventually, make these errors.  */
-      size_t length = strlen (last_string);
-      if (length < 3)
-        warn_at (*loc, _("empty character literal"));
-      else if (length > 3)
+      if (last_string[0] == '\0')
+        {
+          warn_at (*loc, _("empty character literal"));
+          /* '\0' seems dangerous even if we are about to complain.  */
+          val->character = '\'';
+        }
+      else if (last_string[1] != '\0')
         warn_at (*loc, _("extra characters in character literal"));
     }
     if (yytext[0] == '\n')
@@ -514,17 +516,17 @@ splice     (\\[ \f\t\v]*\n)*
   <<EOF>> {
     STRING_FINISH;
     loc->start = token_start;
+    val->character = last_string[0];
     {
-      size_t length = strlen (last_string);
       /* FIXME: Eventually, make these errors.  */
-      if (length < 2)
-        warn_at (*loc, _("empty character literal"));
-      else if (length > 2)
+      if (last_string[0] == '\0')
+        {
+          warn_at (*loc, _("empty character literal"));
+          /* '\0' seems dangerous even if we are about to complain.  */
+          val->character = '\'';
+        }
+      else if (last_string[1] != '\0')
         warn_at (*loc, _("extra characters in character literal"));
-      if (length > 1)
-        val->character = last_string[1];
-      else
-        val->character = last_string[0];
     }
     unexpected_eof (token_start, "'");
     STRING_FREE;
diff --git a/src/symtab.c b/src/symtab.c
index 727ac2c..f702054 100644
--- a/src/symtab.c
+++ b/src/symtab.c
@@ -164,7 +164,7 @@ is_identifier (uniqstr s)
 uniqstr
 symbol_id_get (symbol const *sym)
 {
-  aver (sym->user_token_number != USER_NUMBER_ALIAS);
+  aver (sym->user_token_number != USER_NUMBER_HAS_STRING_ALIAS);
   if (sym->alias)
     sym = sym->alias;
   return is_identifier (sym->tag) ? sym->tag : 0;
@@ -402,7 +402,7 @@ symbol_user_token_number_set (symbol *sym, int 
user_token_number, location loc)
 {
   int *user_token_numberp;
 
-  if (sym->user_token_number != USER_NUMBER_ALIAS)
+  if (sym->user_token_number != USER_NUMBER_HAS_STRING_ALIAS)
     user_token_numberp = &sym->user_token_number;
   else
     user_token_numberp = &sym->alias->user_token_number;
@@ -451,29 +451,24 @@ symbol_check_defined_processor (void *sym, void *null 
ATTRIBUTE_UNUSED)
 }
 
 
-/*------------------------------------------------------------------.
-| Declare the new symbol SYM.  Make it an alias of SYMVAL, and type |
-| SYMVAL with SYM's type.                                           |
-`------------------------------------------------------------------*/
-
 void
-symbol_make_alias (symbol *sym, symbol *symval, location loc)
+symbol_make_alias (symbol *sym, symbol *str, location loc)
 {
-  if (symval->alias)
+  if (str->alias)
     warn_at (loc, _("symbol `%s' used more than once as a literal string"),
-            symval->tag);
+            str->tag);
   else if (sym->alias)
     warn_at (loc, _("symbol `%s' given more than one literal string"),
             sym->tag);
   else
     {
-      symval->class = token_sym;
-      symval->user_token_number = sym->user_token_number;
-      sym->user_token_number = USER_NUMBER_ALIAS;
-      symval->alias = sym;
-      sym->alias = symval;
-      symval->number = sym->number;
-      symbol_type_set (symval, sym->type_name, loc);
+      str->class = token_sym;
+      str->user_token_number = sym->user_token_number;
+      sym->user_token_number = USER_NUMBER_HAS_STRING_ALIAS;
+      str->alias = sym;
+      sym->alias = str;
+      str->number = sym->number;
+      symbol_type_set (str, sym->type_name, loc);
     }
 }
 
@@ -486,46 +481,47 @@ symbol_make_alias (symbol *sym, symbol *symval, location 
loc)
 static inline void
 symbol_check_alias_consistency (symbol *this)
 {
-  symbol *alias = this;
-  symbol *orig  = this->alias;
+  symbol *sym = this;
+  symbol *str  = this->alias;
 
-  /* Check only those that _are_ the aliases.  */
-  if (!(this->alias && this->user_token_number == USER_NUMBER_ALIAS))
+  /* Check only the symbol in the symbol-string pair.  */
+  if (!(this->alias
+        && this->user_token_number == USER_NUMBER_HAS_STRING_ALIAS))
     return;
 
-  if (orig->type_name != alias->type_name)
+  if (str->type_name != sym->type_name)
     {
-      if (orig->type_name)
-       symbol_type_set (alias, orig->type_name, orig->type_location);
+      if (str->type_name)
+       symbol_type_set (sym, str->type_name, str->type_location);
       else
-       symbol_type_set (orig, alias->type_name, alias->type_location);
+       symbol_type_set (str, sym->type_name, sym->type_location);
     }
 
 
-  if (orig->destructor.code || alias->destructor.code)
+  if (str->destructor.code || sym->destructor.code)
     {
-      if (orig->destructor.code)
-       symbol_destructor_set (alias, &orig->destructor);
+      if (str->destructor.code)
+       symbol_destructor_set (sym, &str->destructor);
       else
-       symbol_destructor_set (orig, &alias->destructor);
+       symbol_destructor_set (str, &sym->destructor);
     }
 
-  if (orig->printer.code || alias->printer.code)
+  if (str->printer.code || sym->printer.code)
     {
-      if (orig->printer.code)
-       symbol_printer_set (alias, &orig->printer);
+      if (str->printer.code)
+       symbol_printer_set (sym, &str->printer);
       else
-       symbol_printer_set (orig, &alias->printer);
+       symbol_printer_set (str, &sym->printer);
     }
 
-  if (alias->prec || orig->prec)
+  if (sym->prec || str->prec)
     {
-      if (orig->prec)
-       symbol_precedence_set (alias, orig->prec, orig->assoc,
-                              orig->prec_location);
+      if (str->prec)
+       symbol_precedence_set (sym, str->prec, str->assoc,
+                              str->prec_location);
       else
-       symbol_precedence_set (orig, alias->prec, alias->assoc,
-                              alias->prec_location);
+       symbol_precedence_set (str, sym->prec, sym->assoc,
+                              sym->prec_location);
     }
 }
 
@@ -565,8 +561,8 @@ symbol_pack (symbol *this)
              this->number = this->alias->number;
            }
        }
-      /* Do not do processing below for USER_NUMBER_ALIASes.  */
-      if (this->user_token_number == USER_NUMBER_ALIAS)
+      /* Do not do processing below for USER_NUMBER_HAS_STRING_ALIASes.  */
+      if (this->user_token_number == USER_NUMBER_HAS_STRING_ALIAS)
        return true;
     }
   else /* this->class == token_sym */
@@ -613,7 +609,7 @@ symbol_translation (symbol *this)
 {
   /* Non-terminal? */
   if (this->class == token_sym
-      && this->user_token_number != USER_NUMBER_ALIAS)
+      && this->user_token_number != USER_NUMBER_HAS_STRING_ALIAS)
     {
       /* A token which translation has already been set? */
       if (token_translations[this->user_token_number] != undeftoken->number)
diff --git a/src/symtab.h b/src/symtab.h
index 50dd8cf..b093af3 100644
--- a/src/symtab.h
+++ b/src/symtab.h
@@ -85,9 +85,9 @@ struct symbol
   assoc assoc;
   int user_token_number;
 
-  /* Points to the other in the identifier-symbol pair for an alias.
-     Special value USER_NUMBER_ALIAS in the identifier half of the
-     identifier-symbol pair for an alias.  */
+  /* Points to the other in the symbol-string pair for an alias.
+     Special value USER_NUMBER_HAS_STRING_ALIAS in the symbol half of the
+     symbol-string pair for an alias.  */
   symbol *alias;
   symbol_class class;
   bool declared;
@@ -96,11 +96,11 @@ struct symbol
 /** Undefined user number.  */
 #define USER_NUMBER_UNDEFINED -1
 
-/* `symbol->user_token_number == USER_NUMBER_ALIAS' means this symbol
-   *has* (not is) a string literal alias.  For instance, `%token foo
+/* `symbol->user_token_number == USER_NUMBER_HAS_STRING_ALIAS' means
+   this symbol has a literal string alias.  For instance, `%token foo
    "foo"' has `"foo"' numbered regularly, and `foo' numbered as
-   USER_NUMBER_ALIAS.  */
-#define USER_NUMBER_ALIAS -9991
+   USER_NUMBER_HAS_STRING_ALIAS.  */
+#define USER_NUMBER_HAS_STRING_ALIAS -9991
 
 /* Undefined internal token number.  */
 #define NUMBER_UNDEFINED (-1)
@@ -133,8 +133,11 @@ bool symbol_is_dummy (const symbol *sym);
  ** a string). */
 uniqstr symbol_id_get (symbol const *sym);
 
-/** Declare the new symbol \c sym.  Make it an alias of \c symval.  */
-void symbol_make_alias (symbol *sym, symbol *symval, location loc);
+/**
+ * Make \c str the literal string alias of \c sym.  Copy token number,
+ * symbol number, and type from \c sym to \c str.
+ */
+void symbol_make_alias (symbol *sym, symbol *str, location loc);
 
 /** Set the \c type_name associated with \c sym.
 
-- 
1.5.4.3