Re: {master} symbols: improve symbol aliasing

[Akim Demaille]

Hi Valentin,

Le 30 juil. 2013 à 17:37, Valentin Tolmer <address@hidden> a écrit :

> From: nitnelave <address@hidden>
> 
> Rather than having duplicate info in the symbol and the alias that has to be
> resolved later on, both the symbol and the alias have a common pointer to a
> separate structure containing this info.
> 
> * src/symtab.h: New structure sym_content
> * src/symtab.c (sym_content_new, sym_content_free, symbol_free): New
> * src/AnnotationList.c, src/conflicts.c, src/gram.c, src/gram.h,
> src/graphviz.c, src/ielr.c, src/output.c, src/parse-gram.y, src/print.c
> src/print-xml.c, src/print_graph.c, src/reader.c, src/reduce.c,
> src/state.h, src/symlist.c, src/symtab.c, src/symtab.h, src/tables.c (misc):
> adaptation of the new structures
> * tests/input.at: corrections

As you notified me privately, this patch does not work properly
on some grammars (that turn out to be mine :) that declare aliases
*after* having declared properties.

I install the following patch to fix this issue.

commit 8044fda634bafbe9cc52aaacdeee1c10b55f0ac3
Author: Akim Demaille <address@hidden>
Date:   Tue Dec 10 09:24:20 2013 +0100

    symbols: properly fuse the properties of two symbol aliases
    
    This completes and fixes a7280757105b2909f6a58fdd1c582de8e278319a.
    Reported by Valentin Tolmer.
    
    Before it Bison used to put the properties of the symbols
    (associativity, printer, etc.) in the 'symbol' structure.  An
    identifier-named token (FOO) and its string-named alias ("foo")
    duplicated these properties, and symbol_check_alias_consistency()
    checked that both had compatible properties and fused them, at the end
    of the parsing of the grammar.
    
    The commit a7280757105b2909f6a58fdd1c582de8e278319a introduces a
    sym_content structure that keeps all these properties, and ensures
    that both aliases point to the same sym_content (instead of
    duplicating).  However, it removed symbol_check_alias_consistency,
    which resulted in the non-fusion of *existing* properties:
    
      %token FOO "foo"
      %left FOO %left "foo"
    
    was properly diagnosed as a redeclaration, but
    
      %left FOO %left "foo"
      %token FOO "foo"
    
    was not, as the properties of FOO and "foo" were not checked before
    fusion.  It certainly also means that
    
      %left "foo"
      %token FOO "foo"
    
    did not transfer properly the associativity to FOO.
    
    The fix is simple: reintroduce symbol_check_alias_consistency (under a
    better name, symbol_merge_properties) and call it where appropriate.
    
    Also, that commit made USER_NUMBER_HAS_STRING_ALIAS useless, but left
    it.
    
    * src/symtab.h (USER_NUMBER_HAS_STRING_ALIAS): Remove, unused.
    Adjust dependencies.
    * src/symtab.c (symbol_merge_properties): New, based on the former
    symbol_check_alias_consistency.
    * tests/input.at: Re-enable tests that we now pass.

diff --git a/src/symtab.c b/src/symtab.c
index 5a8b616..8a2690e 100644
--- a/src/symtab.c
+++ b/src/symtab.c
@@ -269,7 +269,6 @@ is_identifier (uniqstr s)
 uniqstr
 symbol_id_get (symbol const *sym)
 {
-  aver (sym->content->user_token_number != USER_NUMBER_HAS_STRING_ALIAS);
   if (sym->alias)
     sym = sym->alias;
   return is_identifier (sym->tag) ? sym->tag : 0;
@@ -559,6 +558,45 @@ semantic_type_check_defined_processor (void *sem_type,
 }
 
 
+/*-------------------------------------------------------------------.
+| Merge the properties (precedence, associativity, etc.) of SYM, and |
+| its string-named alias STR; check consistency.                     |
+`-------------------------------------------------------------------*/
+
+static void
+symbol_merge_properties (symbol *sym, symbol *str)
+{
+  if (str->content->type_name != sym->content->type_name)
+    {
+      if (str->content->type_name)
+        symbol_type_set (sym,
+                         str->content->type_name, str->content->type_location);
+      else
+        symbol_type_set (str,
+                         sym->content->type_name, sym->content->type_location);
+    }
+
+
+  {
+    int i;
+    for (i = 0; i < CODE_PROPS_SIZE; ++i)
+      if (str->content->props[i].code)
+        symbol_code_props_set (sym, i, &str->content->props[i]);
+      else if (sym->content->props[i].code)
+        symbol_code_props_set (str, i, &sym->content->props[i]);
+  }
+
+  if (sym->content->prec || str->content->prec)
+    {
+      if (str->content->prec)
+        symbol_precedence_set (sym, str->content->prec, str->content->assoc,
+                               str->content->prec_location);
+      else
+        symbol_precedence_set (str, sym->content->prec, sym->content->assoc,
+                               sym->content->prec_location);
+    }
+}
+
 void
 symbol_make_alias (symbol *sym, symbol *str, location loc)
 {
@@ -570,6 +608,7 @@ symbol_make_alias (symbol *sym, symbol *str, location loc)
               _("symbol %s given more than one literal string"), sym->tag);
   else
     {
+      symbol_merge_properties (sym, str);
       sym_content_free (str->content);
       str->content = sym->content;
       str->content->symbol = str;
@@ -591,8 +630,6 @@ symbol_pack (symbol *this)
   aver (this->content->number != NUMBER_UNDEFINED);
   if (this->content->class == nterm_sym)
     this->content->number += ntokens;
-  else if (this->content->user_token_number == USER_NUMBER_HAS_STRING_ALIAS)
-    return true;
 
   symbols[this->content->number] = this->content->symbol;
   return true;
diff --git a/src/symtab.h b/src/symtab.h
index 05d1233..e9984f9 100644
--- a/src/symtab.h
+++ b/src/symtab.h
@@ -137,12 +137,6 @@ struct sym_content
 /** Undefined user number.  */
 # define USER_NUMBER_UNDEFINED -1
 
-/* '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_HAS_STRING_ALIAS.  */
-# define USER_NUMBER_HAS_STRING_ALIAS -9991
-
 /* Undefined internal token number.  */
 # define NUMBER_UNDEFINED (-1)
 
diff --git a/tests/input.at b/tests/input.at
index 205342b..28a0a03 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -721,49 +721,49 @@ input.y:2.1-5:     previous declaration
 # This time, declare the alias after its use.
 
 # Precedence/associativity.
-## FIXME: AT_TEST([[%left "foo"
-## FIXME: %left foo
-## FIXME: %token foo "foo"
-## FIXME: %%
-## FIXME: exp: foo;
-## FIXME: ]],
-## FIXME: [[input.y:2.1-5: error: %left redeclaration for foo
-## FIXME:  %left foo
-## FIXME:  ^^^^^
-## FIXME: input.y:1.1-5:     previous declaration
-## FIXME:  %left "foo"
-## FIXME:  ^^^^^
-## FIXME: ]])
-
-## FIXME: # Printer.
-## FIXME: AT_TEST([[%printer {} "foo"
-## FIXME: %printer {} foo
-## FIXME: %token foo "foo"
-## FIXME: %%
-## FIXME: exp: foo;
-## FIXME: ]],
-## FIXME: [[input.y:2.10-11: error: %printer redeclaration for foo
-## FIXME:  %printer {} foo
-## FIXME:           ^^
-## FIXME: input.y:1.10-11:     previous declaration
-## FIXME:  %printer {} "foo"
-## FIXME:           ^^
-## FIXME: ]])
-
-## FIXME: # Destructor.
-## FIXME: AT_TEST([[%destructor {} "foo"
-## FIXME: %destructor {} foo
-## FIXME: %token foo "foo"
-## FIXME: %%
-## FIXME: exp: foo;
-## FIXME: ]],
-## FIXME: [[input.y:2.13-14: error: %destructor redeclaration for foo
-## FIXME:  %destructor {} foo
-## FIXME:              ^^
-## FIXME: input.y:1.13-14:     previous declaration
-## FIXME:  %destructor {} "foo"
-## FIXME:              ^^
-## FIXME: ]])
+AT_TEST([[%left "foo"
+%left foo
+%token foo "foo"
+%%
+exp: foo;
+]],
+[[input.y:2.1-5: error: %left redeclaration for foo
+ %left foo
+ ^^^^^
+input.y:1.1-5:     previous declaration
+ %left "foo"
+ ^^^^^
+]])
+
+# Printer.
+AT_TEST([[%printer {} "foo"
+%printer {} foo
+%token foo "foo"
+%%
+exp: foo;
+]],
+[[input.y:2.10-11: error: %printer redeclaration for foo
+ %printer {} foo
+          ^^
+input.y:1.10-11:     previous declaration
+ %printer {} "foo"
+          ^^
+]])
+
+# Destructor.
+AT_TEST([[%destructor {} "foo"
+%destructor {} foo
+%token foo "foo"
+%%
+exp: foo;
+]],
+[[input.y:2.13-14: error: %destructor redeclaration for foo
+ %destructor {} foo
+             ^^
+input.y:1.13-14:     previous declaration
+ %destructor {} "foo"
+             ^^
+]])
 
 m4_popdef([AT_TEST])
 AT_CLEANUP