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 <[email protected]> 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)
