RFC: generate the default semantic action

[Akim Demaille]

With this patch, I think Bison is now feature complete compared to
Frank’s C++17 skeleton.

This patch is prompted by C++, but I believe it’s a real improvement
even for the other languages.  However, it does mean that the default
action is now made twice in C for instance.  I would very much like
to disable the pre-initialization of $$ to $1, but I’m afraid people
might depend on this…


commit b6c9bcfec96fd674bf2f67a51c0942093cb257f0
Author: Akim Demaille <address@hidden>
Date:   Sun Oct 14 11:27:05 2018 +0200

    generate the default semantic action
    
    Currently, in C, the default semantic action is implemented by being
    always run before running the actual user semantic action.  As a
    consequence, when the user action is run, $$ is already set as $1.
    
    In C++ with variants, we don't do that, since we cannot manipulate the
    semantic value without knowing its exact type.  When variants are
    enabled, the only guarantee is that $$ is default contructed and ready
    to the used.
    
    Some users still would like the default action to be run with
    variants.  Frank Heckenbach's parser in
    C++17 (http://lists.gnu.org/archive/html/bug-bison/2018-04/msg00011.html)
    provides this feature, but relying on std::variant's dynamic typing,
    which we forbid in lalr1.cc.
    
    The simplest seems to be actually generating the default semantic
    action (in all languages/skeletons).  This makes the pre-action (that
    sets $$ to $1) useless.  But...  maybe some users depend on this, in
    spite of the comments that clearly warn againt this.  So let's not
    turn this off just yet.
    
    * src/reader.c (grammar_rule_check_and_complete): Rename as...
    (grammar_rule_check_and_complete): this.
    Install the default semantic action when applicable.
    * examples/variant-11.yy, examples/variant.yy, tests/calc.at:
    Exercise the default semantic action, even with variants.

diff --git a/NEWS b/NEWS
index d1bf802e..7a039021 100644
--- a/NEWS
+++ b/NEWS
@@ -68,6 +68,27 @@ GNU Bison NEWS
 
   The new examples/variant-11.yy shows these features in action.
 
+*** C++: The implicit default semantic action is always run
+
+  When variants are enabled, the default action was not run, so
+
+    exp: "number"
+
+  was equivalent to
+
+    exp: "number"  {}
+
+  It now behaves like in all the other cases, as
+
+    exp: "number"  { $$ = $1; }
+
+  possibly using std::move if automove is enabled.
+
+  We do not expect backward compatibility issues.  However, beware of
+  forward compatibility issues: if you rely on default actions with
+  variants, be sure to '%require "3.2"' to avoid older versions of Bison to
+  generate incorrect parsers.
+
 *** C++: Renaming location.hh
 
   When both %defines and %locations are enabled, Bison generates a
diff --git a/doc/bison.texi b/doc/bison.texi
index e88eb8e7..f633f1c9 100644
--- a/doc/bison.texi
+++ b/doc/bison.texi
@@ -6348,8 +6348,7 @@ Obsoleted by @code{api.namespace}
 
 @item Purpose: Issue runtime assertions to catch invalid uses.
 In C++, when variants are used (@pxref{C++ Variants}), symbols must be
-constructed and
-destroyed properly.  This option checks these constraints.
+constructed and destroyed properly.  This option checks these constraints.
 
 @item Accepted Values: Boolean
 
@@ -11594,13 +11593,13 @@ assignment:
 %left "+" "-";
 %left "*" "/";
 exp:
-  exp "+" exp   @{ $$ = $1 + $3; @}
+  "number"
+| "identifier"  @{ $$ = drv.variables[$1]; @}
+| exp "+" exp   @{ $$ = $1 + $3; @}
 | exp "-" exp   @{ $$ = $1 - $3; @}
 | exp "*" exp   @{ $$ = $1 * $3; @}
 | exp "/" exp   @{ $$ = $1 / $3; @}
 | "(" exp ")"   @{ std::swap ($$, $2); @}
-| "identifier"  @{ $$ = drv.variables[$1]; @}
-| "number"      @{ std::swap ($$, $1); @};
 %%
 @end example
 
diff --git a/examples/variant-11.yy b/examples/variant-11.yy
index d378f300..c7fd66f9 100644
--- a/examples/variant-11.yy
+++ b/examples/variant-11.yy
@@ -101,7 +101,7 @@ list:
 ;
 
 item:
-  TEXT    { $$ = $1; }
+  TEXT
 | NUMBER  { $$ = make_string_uptr (to_string ($1)); }
 ;
 %%
diff --git a/examples/variant.yy b/examples/variant.yy
index e18929e6..6df428da 100644
--- a/examples/variant.yy
+++ b/examples/variant.yy
@@ -89,7 +89,7 @@ list:
 ;
 
 item:
-  TEXT    { std::swap ($$, $1); }
+  TEXT
 | NUMBER  { $$ = to_string ($1); }
 ;
 %%
diff --git a/src/reader.c b/src/reader.c
index 5f0e5d4b..516f6aad 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -275,13 +275,14 @@ symbol_should_be_used (symbol_list const *s, bool 
*midrule_warning)
   return false;
 }
 
-/*----------------------------------------------------------------.
-| Check that the rule R is properly defined.  For instance, there |
-| should be no type clash on the default action.                  |
-`----------------------------------------------------------------*/
+/*-----------------------------------------------------------------.
+| Check that the rule R is properly defined.  For instance, there  |
+| should be no type clash on the default action.  Possibly install |
+| the default action.                                              |
+`-----------------------------------------------------------------*/
 
 static void
-grammar_rule_check (const symbol_list *r)
+grammar_rule_check_and_complete (symbol_list *r)
 {
   /* Type check.
 
@@ -303,6 +304,16 @@ grammar_rule_check (const symbol_list *r)
             complain (&r->location, Wother,
                       _("type clash on default action: <%s> != <%s>"),
                       lhs_type, rhs_type);
+          else
+            {
+              /* Install the default action.  */
+              code_props_rule_action_init (&r->action_props, "{ $$ = $1; }",
+                                           r->location, r,
+                                           /* name */ NULL,
+                                           /* type */ NULL,
+                                           /* is_predicate */ false);
+              code_props_translate_code (&r->action_props);
+            }
         }
       /* Warn if there is no default for $$ but we need one.  */
       else
@@ -439,14 +450,14 @@ grammar_current_rule_prec_set (symbol *precsym, location 
loc)
 {
   /* POSIX says that any identifier is a nonterminal if it does not
      appear on the LHS of a grammar rule and is not defined by %token
-     or by one of the directives that assigns precedence to a token.  We
-     ignore this here because the only kind of identifier that POSIX
-     allows to follow a %prec is a token and because assuming it's a
-     token now can produce more logical error messages.  Nevertheless,
-     grammar_rule_check does obey what we believe is the real intent of
-     POSIX here: that an error be reported for any identifier that
-     appears after %prec but that is not defined separately as a
-     token.  */
+     or by one of the directives that assigns precedence to a token.
+     We ignore this here because the only kind of identifier that
+     POSIX allows to follow a %prec is a token and because assuming
+     it's a token now can produce more logical error messages.
+     Nevertheless, grammar_rule_check_and_complete does obey what we
+     believe is the real intent of POSIX here: that an error be
+     reported for any identifier that appears after %prec but that is
+     not defined separately as a token.  */
   symbol_class_set (precsym, token_sym, loc, false);
   if (current_rule->ruleprec)
     duplicate_directive ("%prec",
@@ -593,7 +604,7 @@ packgram (void)
          symbols may appear unused, but the parsing algorithm ensures that
          %destructor's are invoked appropriately.  */
       if (p != grammar)
-        grammar_rule_check (p);
+        grammar_rule_check_and_complete (p);
 
       rules[ruleno].user_number = ruleno;
       rules[ruleno].number = ruleno;
@@ -799,12 +810,13 @@ check_and_convert_grammar (void)
      symbols_pack above) so that token types are set correctly before the rule
      action type checking.
 
-     Before invoking grammar_rule_check (in packgram below) on any rule, make
-     sure all actions have already been scanned in order to set 'used' flags.
-     Otherwise, checking that a midrule's $$ should be set will not always work
-     properly because the check must forward-reference the midrule's parent
-     rule.  For the same reason, all the 'used' flags must be set before
-     checking whether to remove '$' from any midrule symbol name (also in
+     Before invoking grammar_rule_check_and_complete (in packgram
+     below) on any rule, make sure all actions have already been
+     scanned in order to set 'used' flags.  Otherwise, checking that a
+     midrule's $$ should be set will not always work properly because
+     the check must forward-reference the midrule's parent rule.  For
+     the same reason, all the 'used' flags must be set before checking
+     whether to remove '$' from any midrule symbol name (also in
      packgram).  */
   for (symbol_list *sym = grammar; sym; sym = sym->next)
     code_props_translate_code (&sym->action_props);
diff --git a/tests/calc.at b/tests/calc.at
index 5c799c44..f05514cc 100644
--- a/tests/calc.at
+++ b/tests/calc.at
@@ -303,7 +303,7 @@ line:
 ;
 
 exp:
-  NUM                { $$ = $1;             }
+  NUM
 | exp '=' exp
   {
     if ($1 != $3)