From: Joel E. Denny Date: Mon, 26 Jun 2006 04:16:50 +0000 (+0000) Subject: Get action warnings (grammar_rule_check) right even when symbol X-Git-Tag: v2.3b~352 X-Git-Url: https://git.saurik.com/bison.git/commitdiff_plain/ffa4ba3aeff61dee4bc4f64ee3abedafe142af31 Get action warnings (grammar_rule_check) right even when symbol declarations appear after the rules. Don't mistake the type of $$ in a midrule to be that of its parent rule's $$. * src/reader.c (grammar_current_rule_end): Don't invoke grammar_rule_check yet since not all symbol declarations may have been parsed yet. (grammar_midrule_action): Likewise. Don't record whether the midrule's $$ has been used yet since actions haven't been translated yet. Record the midrule's parent rule and its RHS index within the parent rule. (grammar_current_rule_action_append): Don't translate the action yet since not all symbol declarations may have been parsed yet and, thus, warnings about types for $$, $n, @$, and @n can't be reported yet. (packgram): Translate the action and invoke grammar_rule_check now that all symbol declarations have been parsed. * src/scan-code.l (handle_action_dollar): Now that this is invoked after parsing the entire grammar file, the symbol list here in the case of a midrule is actually the midrule's empty RHS, so reference its parent rule's RHS where necessary. On the other hand, now that you can already know it's a midrule, you aren't forced to think $$ has the same type as its parent rule's $$. (handle_action_at): In the case of a midrule, reference the parent rule where necessary. * src/symlist.c (symbol_list_new): Initialize new midrule-related members. (symbol_list_length): Now that this is invoked after all rules have been parsed, a NULL symbol (rather than a NULL symbol list node) terminates a rule. symbol_list_print already does this correctly. * src/symlist.h (symbol_list.midrule_parent_rule, symbol_list.midrule_parent_rhs_index): New members so that midrules can remember their relationships with their parents. * tests/input.at (Type Clashes): Extend to catch the midrule $$ error fixed by the above patch. (_AT_UNUSED_VALUES_DECLARATIONS, AT_CHECK_UNUSED_VALUES): New m4 macros implementing... (Unused values): ... this old test case and... (Unused values before symbol declarations): ... this new test case. This one is the same as `Unused values' except that all symbol declarations appear after the rules in order to catch the rest of the errors fixed by the above patch. --- diff --git a/ChangeLog b/ChangeLog index a48e4b9e..3abf8f82 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,47 @@ +2006-06-26 Joel E. Denny + + Get action warnings (grammar_rule_check) right even when symbol + declarations appear after the rules. Don't mistake the type of $$ in + a midrule to be that of its parent rule's $$. + * src/reader.c (grammar_current_rule_end): Don't invoke + grammar_rule_check yet since not all symbol declarations may have been + parsed yet. + (grammar_midrule_action): Likewise. + Don't record whether the midrule's $$ has been used yet since actions + haven't been translated yet. + Record the midrule's parent rule and its RHS index within the parent + rule. + (grammar_current_rule_action_append): Don't translate the action yet + since not all symbol declarations may have been parsed yet and, thus, + warnings about types for $$, $n, @$, and @n can't be reported yet. + (packgram): Translate the action and invoke grammar_rule_check now that + all symbol declarations have been parsed. + * src/scan-code.l (handle_action_dollar): Now that this is invoked + after parsing the entire grammar file, the symbol list here in the case + of a midrule is actually the midrule's empty RHS, so reference its + parent rule's RHS where necessary. + On the other hand, now that you can already know it's a midrule, you + aren't forced to think $$ has the same type as its parent rule's $$. + (handle_action_at): In the case of a midrule, reference the parent rule + where necessary. + * src/symlist.c (symbol_list_new): Initialize new midrule-related + members. + (symbol_list_length): Now that this is invoked after all rules have + been parsed, a NULL symbol (rather than a NULL symbol list node) + terminates a rule. symbol_list_print already does this correctly. + * src/symlist.h (symbol_list.midrule_parent_rule, + symbol_list.midrule_parent_rhs_index): New members so that midrules can + remember their relationships with their parents. + * tests/input.at (Type Clashes): Extend to catch the midrule $$ error + fixed by the above patch. + (_AT_UNUSED_VALUES_DECLARATIONS, AT_CHECK_UNUSED_VALUES): New m4 macros + implementing... + (Unused values): ... this old test case and... + (Unused values before symbol declarations): ... this new test case. + This one is the same as `Unused values' except that all symbol + declarations appear after the rules in order to catch the rest of the + errors fixed by the above patch. + 2006-06-26 Joel E. Denny More cleanup. diff --git a/src/reader.c b/src/reader.c index 2c05c19a..a0c3f855 100644 --- a/src/reader.c +++ b/src/reader.c @@ -293,7 +293,6 @@ grammar_current_rule_end (location loc) /* Put an empty link in the list to mark the end of this rule */ grammar_symbol_append (NULL, grammar_end->location); current_rule->location = loc; - grammar_rule_check (current_rule); } @@ -326,10 +325,9 @@ grammar_midrule_action (void) midrule->action = current_rule->action; midrule->action_location = dummy_location; current_rule->action = NULL; - /* If $$ was used in the action, the LHS of the enclosing rule was - incorrectly flagged as used. */ - midrule->used = current_rule->used; - current_rule->used = false; + /* The action has not been translated yet, so $$ use hasn't been + detected yet. */ + midrule->used = false; if (previous_rule_end) previous_rule_end->next = midrule; @@ -338,7 +336,6 @@ grammar_midrule_action (void) /* End the dummy's rule. */ midrule->next = symbol_list_new (NULL, dummy_location); - grammar_rule_check (midrule); midrule->next->next = current_rule; previous_rule_end = midrule->next; @@ -347,6 +344,8 @@ grammar_midrule_action (void) the current rule. Bind it to its dedicated rule. */ grammar_current_rule_symbol_append (dummy, dummy_location); grammar_end->midrule = midrule; + midrule->midrule_parent_rule = current_rule; + midrule->midrule_parent_rhs_index = symbol_list_length (current_rule->next); } /* Set the precedence symbol of the current rule to PRECSYM. */ @@ -405,9 +404,10 @@ grammar_current_rule_action_append (const char *action, location loc) { if (current_rule->action) grammar_midrule_action (); + /* After all symbol declarations have been parsed, packgram invokes + translate_rule_action. */ current_rule->action = action; current_rule->action_location = loc; - current_rule->action = translate_rule_action (current_rule); } @@ -444,9 +444,16 @@ packgram (void) rules[ruleno].precsym = NULL; rules[ruleno].location = p->location; rules[ruleno].useful = true; - rules[ruleno].action = p->action; + rules[ruleno].action = p->action ? translate_rule_action (p) : NULL; rules[ruleno].action_location = p->action_location; + /* If this rule contains midrules, rest assured that + grammar_midrule_action inserted the midrules into grammar before this + rule. Thus, the midrule actions have already been scanned in order to + set `used' flags for this rule's rhs, so grammar_rule_check will work + properly. */ + grammar_rule_check (p); + for (p = p->next; p && p->sym; p = p->next) { ++rule_length; diff --git a/src/scan-code.l b/src/scan-code.l index aa58521c..471b2a45 100644 --- a/src/scan-code.l +++ b/src/scan-code.l @@ -240,7 +240,19 @@ handle_action_dollar (symbol_list *rule, char *text, location dollar_loc) { const char *type_name = NULL; char *cp = text + 1; - int rule_length = symbol_list_length (rule->next); + symbol_list *effective_rule; + int effective_rule_length; + + if (rule->midrule_parent_rule) + { + effective_rule = rule->midrule_parent_rule; + effective_rule_length = rule->midrule_parent_rhs_index - 1; + } + else + { + effective_rule = rule; + effective_rule_length = symbol_list_length (rule->next); + } /* Get the type name if explicit. */ if (*cp == '<') @@ -257,8 +269,17 @@ handle_action_dollar (symbol_list *rule, char *text, location dollar_loc) if (!type_name) type_name = symbol_list_n_type_name_get (rule, dollar_loc, 0); if (!type_name && typed) - complain_at (dollar_loc, _("$$ of `%s' has no declared type"), - rule->sym->tag); + { + if (rule->midrule_parent_rule) + complain_at (dollar_loc, + _("$$ for the midrule at $%d of `%s' has no declared" + " type"), + rule->midrule_parent_rhs_index, + effective_rule->sym->tag); + else + complain_at (dollar_loc, _("$$ of `%s' has no declared type"), + rule->sym->tag); + } if (!type_name) type_name = ""; obstack_fgrow1 (&obstack_for_string, @@ -270,23 +291,23 @@ handle_action_dollar (symbol_list *rule, char *text, location dollar_loc) long int num; set_errno (0); num = strtol (cp, 0, 10); - if (INT_MIN <= num && num <= rule_length && ! get_errno ()) + if (INT_MIN <= num && num <= effective_rule_length && ! get_errno ()) { int n = num; if (1-n > max_left_semantic_context) max_left_semantic_context = 1-n; if (!type_name && n > 0) type_name = - symbol_list_n_type_name_get (rule, dollar_loc, n); + symbol_list_n_type_name_get (effective_rule, dollar_loc, n); if (!type_name && typed) complain_at (dollar_loc, _("$%d of `%s' has no declared type"), - n, rule->sym->tag); + n, effective_rule->sym->tag); if (!type_name) type_name = ""; obstack_fgrow3 (&obstack_for_string, "]b4_rhs_value(%d, %d, [%s])[", - rule_length, n, type_name); - symbol_list_n_used_set (rule, n, true); + effective_rule_length, n, type_name); + symbol_list_n_used_set (effective_rule, n, true); } else complain_at (dollar_loc, _("integer out of range: %s"), quote (text)); @@ -303,8 +324,13 @@ static void handle_action_at (symbol_list *rule, char *text, location at_loc) { char *cp = text + 1; - int rule_length = symbol_list_length (rule->next); locations_flag = true; + int effective_rule_length; + + if (rule->midrule_parent_rule) + effective_rule_length = rule->midrule_parent_rhs_index - 1; + else + effective_rule_length = symbol_list_length (rule->next); if (*cp == '$') obstack_sgrow (&obstack_for_string, "]b4_lhs_location["); @@ -314,11 +340,11 @@ handle_action_at (symbol_list *rule, char *text, location at_loc) set_errno (0); num = strtol (cp, 0, 10); - if (INT_MIN <= num && num <= rule_length && ! get_errno ()) + if (INT_MIN <= num && num <= effective_rule_length && ! get_errno ()) { int n = num; obstack_fgrow2 (&obstack_for_string, "]b4_rhs_location(%d, %d)[", - rule_length, n); + effective_rule_length, n); } else complain_at (at_loc, _("integer out of range: %s"), quote (text)); diff --git a/src/symlist.c b/src/symlist.c index 70db82f0..67766fa2 100644 --- a/src/symlist.c +++ b/src/symlist.c @@ -39,6 +39,8 @@ symbol_list_new (symbol *sym, location loc) res->location = loc; res->midrule = NULL; + res->midrule_parent_rule = NULL; + res->midrule_parent_rhs_index = 0; res->action = NULL; res->used = false; @@ -102,7 +104,7 @@ unsigned int symbol_list_length (const symbol_list *l) { int res = 0; - for (/* Nothing. */; l; l = l->next) + for (/* Nothing. */; l && l->sym; l = l->next) ++res; return res; } diff --git a/src/symlist.h b/src/symlist.h index 0423d016..2b901999 100644 --- a/src/symlist.h +++ b/src/symlist.h @@ -32,10 +32,17 @@ typedef struct symbol_list symbol *sym; location location; - /* If this symbol is the generated lhs for a mid-rule, a pointer to - that mid-rule. */ + /* If this symbol is the generated lhs for a midrule but this is the rule in + whose rhs it appears, MIDRULE = a pointer to that midrule. */ struct symbol_list *midrule; + /* If this symbol is the generated lhs for a midrule and this is that + midrule, MIDRULE_PARENT_RULE = a pointer to the rule in whose rhs it + appears, and MIDRULE_PARENT_RHS_INDEX = its rhs index (1-origin) in the + parent rule. */ + struct symbol_list *midrule_parent_rule; + int midrule_parent_rhs_index; + /* The action is attached to the LHS of a rule. */ const char *action; location action_location; diff --git a/tests/input.at b/tests/input.at index 4b66d32e..abbf1f38 100644 --- a/tests/input.at +++ b/tests/input.at @@ -48,89 +48,123 @@ AT_CLEANUP AT_SETUP([Type Clashes]) AT_DATA([input.y], -[[%token foo +[[%union { int bar; } +%token foo %type exp %% -exp: foo {} foo +exp: foo { $$; } foo { $2; } foo | foo | /* Empty. */ ; ]]) -AT_CHECK([bison input.y], [], [], -[[input.y:4.6-15: warning: type clash on default action: != <> -input.y:5.6-8: warning: type clash on default action: != <> -input.y:6.5: warning: empty rule for typed nonterminal, and no action +AT_CHECK([bison input.y], [1], [], +[[input.y:5.12-13: $$ for the midrule at $2 of `exp' has no declared type +input.y:5.24-25: $2 of `exp' has no declared type +input.y:5.6-32: warning: type clash on default action: != <> +input.y:6.6-8: warning: type clash on default action: != <> +input.y:7.5: warning: empty rule for typed nonterminal, and no action ]]) AT_CLEANUP -## --------------- ## -## Unused values. ## -## --------------- ## +# _AT_UNUSED_VALUES_DECLARATIONS() +# -------------------------------------------- +# Generate the token, type, and destructor +# declarations for the unused values tests. -AT_SETUP([Unused values]) +m4_define([_AT_UNUSED_VALUES_DECLARATIONS], +[[[%token INT; +%type a b c d e f g h i j k l; +%destructor { destroy ($$); } INT a b c d e f g h i j k l;]]]) -AT_DATA([input.y], -[[%token INT -%type a b c d e f g h i j k l -%destructor { destroy ($$); } INT a b c d e f g h i j k l -%% + +# AT_CHECK_UNUSED_VALUES(DECLARATIONS_AFTER) +# -------------------------------------------- +# Generate a grammar to test unused values, +# compile it, run it. If DECLARATIONS_AFTER +# is set, then the token, type, and destructor +# declarations are generated after the rules +# rather than before. + +m4_define([AT_CHECK_UNUSED_VALUES], +[AT_DATA([input.y], +m4_ifval($1, [ + + +], [_AT_UNUSED_VALUES_DECLARATIONS +])[[%% start: - 'a' a { $2 } | 'b' b { $2 } | 'c' c { $2 } | 'd' d { $2 } | 'e' e { $2 } -| 'f' f { $2 } | 'g' g { $2 } | 'h' h { $2 } | 'i' i { $2 } | 'j' j { $2 } -| 'k' k { $2 } | 'l' l { $2 } + 'a' a { $]2[ } | 'b' b { $]2[ } | 'c' c { $]2[ } | 'd' d { $]2[ } | 'e' e { $]2[ } +| 'f' f { $]2[ } | 'g' g { $]2[ } | 'h' h { $]2[ } | 'i' i { $]2[ } | 'j' j { $]2[ } +| 'k' k { $]2[ } | 'l' l { $]2[ } ; a: INT | INT { } INT { } INT { }; b: INT | /* empty */; -c: INT | INT { $1 } INT { } INT { }; -d: INT | INT { } INT { $1 } INT { }; -e: INT | INT { } INT { } INT { $1 }; -f: INT | INT { } INT { } INT { $$ = $1 + $3 + $5; }; -g: INT | INT { $$ } INT { $$ } INT { }; -h: INT | INT { $$ } INT { $$ = $2 } INT { }; -i: INT | INT INT { } { $$ = $1 + $2; }; -j: INT | INT INT { $$ = 1; } { $$ = $1 + $2; }; -k: INT | INT INT { $$; } { $$ = $3; } { }; -l: INT | INT { $$ = $1; } INT { $$ = $2 + $3; } INT { $$ = $4 + $5; }; +c: INT | INT { $]1[ } INT { } INT { }; +d: INT | INT { } INT { $]1[ } INT { }; +e: INT | INT { } INT { } INT { $]1[ }; +f: INT | INT { } INT { } INT { $]$[ = $]1[ + $]3[ + $]5[; }; +g: INT | INT { $]$[ } INT { $]$[ } INT { }; +h: INT | INT { $]$[ } INT { $]$[ = $]2[ } INT { }; +i: INT | INT INT { } { $]$[ = $]1[ + $]2[; }; +j: INT | INT INT { $$ = 1; } { $]$[ = $]1[ + $]2[; }; +k: INT | INT INT { $]$[; } { $]$[ = $]3[; } { }; +l: INT | INT { $]$[ = $]1[; } INT { $]$[ = $]2[ + $]3[; } INT { $]$[ = $]4[ + $]5[; };]]m4_ifval($1, [ +_AT_UNUSED_VALUES_DECLARATIONS]) +) + +AT_CHECK([bison input.y], [0], [], +[[input.y:11.10-32: warning: unset value: $]$[ +input.y:11.10-32: warning: unused value: $]1[ +input.y:11.10-32: warning: unused value: $]3[ +input.y:11.10-32: warning: unused value: $]5[ +input.y:12.9: warning: empty rule for typed nonterminal, and no action +input.y:13.10-35: warning: unset value: $]$[ +input.y:13.10-35: warning: unused value: $]3[ +input.y:13.10-35: warning: unused value: $]5[ +input.y:14.10-35: warning: unset value: $]$[ +input.y:14.10-35: warning: unused value: $]3[ +input.y:14.10-35: warning: unused value: $]5[ +input.y:15.10-36: warning: unset value: $]$[ +input.y:15.10-36: warning: unused value: $]3[ +input.y:15.10-36: warning: unused value: $]5[ +input.y:17.10-38: warning: unset value: $]$[ +input.y:17.10-38: warning: unused value: $]1[ +input.y:17.10-38: warning: unused value: $]2[ +input.y:17.10-38: warning: unused value: $]3[ +input.y:17.10-38: warning: unused value: $]4[ +input.y:17.10-38: warning: unused value: $]5[ +input.y:18.10-43: warning: unset value: $]$[ +input.y:18.10-43: warning: unused value: $]1[ +input.y:18.10-43: warning: unused value: $]3[ +input.y:18.10-43: warning: unused value: $]4[ +input.y:18.10-43: warning: unused value: $]5[ +input.y:20.10-55: warning: unused value: $]3[ +input.y:21.10-41: warning: unset value: $]$[ +input.y:21.10-41: warning: unused value: $]1[ +input.y:21.10-41: warning: unused value: $]2[ +input.y:21.10-41: warning: unused value: $]4[ +]])]) -]]) -AT_CHECK([bison input.y], [], [], -[[input.y:11.10-32: warning: unset value: $$ -input.y:11.10-32: warning: unused value: $1 -input.y:11.10-32: warning: unused value: $3 -input.y:11.10-32: warning: unused value: $5 -input.y:12.9: warning: empty rule for typed nonterminal, and no action -input.y:13.10-35: warning: unset value: $$ -input.y:13.10-35: warning: unused value: $3 -input.y:13.10-35: warning: unused value: $5 -input.y:14.10-35: warning: unset value: $$ -input.y:14.10-35: warning: unused value: $3 -input.y:14.10-35: warning: unused value: $5 -input.y:15.10-36: warning: unset value: $$ -input.y:15.10-36: warning: unused value: $3 -input.y:15.10-36: warning: unused value: $5 -input.y:17.10-38: warning: unset value: $$ -input.y:17.10-38: warning: unused value: $1 -input.y:17.10-38: warning: unused value: $2 -input.y:17.10-38: warning: unused value: $3 -input.y:17.10-38: warning: unused value: $4 -input.y:17.10-38: warning: unused value: $5 -input.y:18.10-43: warning: unset value: $$ -input.y:18.10-43: warning: unused value: $1 -input.y:18.10-43: warning: unused value: $3 -input.y:18.10-43: warning: unused value: $4 -input.y:18.10-43: warning: unused value: $5 -input.y:20.10-55: warning: unused value: $3 -input.y:21.10-41: warning: unset value: $$ -input.y:21.10-41: warning: unused value: $1 -input.y:21.10-41: warning: unused value: $2 -input.y:21.10-41: warning: unused value: $4 -]]) +## --------------- ## +## Unused values. ## +## --------------- ## + +AT_SETUP([Unused values]) +AT_CHECK_UNUSED_VALUES +AT_CLEANUP + + +## ------------------------------------------ ## +## Unused values before symbol declarations. ## +## ------------------------------------------ ## +AT_SETUP([Unused values before symbol declarations]) +AT_CHECK_UNUSED_VALUES([1]) AT_CLEANUP