Re: %prec using undefined token

[Joel E. Denny]

On Tue, 29 Dec 2009, Florian Krohm wrote:

> I was debugging shift/reduce conflicts in a grammar and tracked
> down the problem to a typo. I had misspelled a token name in a
> %prec directive. The misspelled token name was an undefined token.
> Like so:
> 
> %token INT
> %left '-'
> %left UMINUS
> 
> %%
> 
> expr: term
>     | expr '-' expr
>     | '-' expr %prec UNINUS
>     ;
> 
> term: INT
>     | '(' expr ')'
>     ;
> 
> bison 2.4.1 was silent about it even with --warnings=all
> 
> It would be consistent to issue an error about this situation.
> After all one cannot use an undefined token elsewhere in a rule.
> So why should this be allowed in %prec?

Thanks for the report.

Bison used to complain here about UNINUS being undefined, but I 
accidentally broke that in 2.3b when I made Bison complain about 
nonterminals used in %prec.  My faulty assumption was that %prec was 
enough to indicate that a symbol is a token, but POSIX does not agree:

  Every name used in grammar not defined by a %token, %left, %right, or 
  %nonassoc declaration is assumed to represent a non-terminal symbol. 
  The yacc utility shall report an error for any non-terminal symbol that 
  does not appear on the left side of at least one grammar rule.

However, POSIX does at least agree that %prec symbols shouldn't be 
nonterminals:

  The reserved symbol %prec can appear immediately after the body of the 
  grammar rule and can be followed by a token name or a literal.

It occurs to me that Bison could handle all this by complaining if UNINUS 
doesn't have a precedence assigned.  However, I think that would be 
stricter than what POSIX requires.  Someone might be using %prec to merely 
remove a rule's default precedence.

The patch below restores the complaint (as an error not a warning) for the 
case when a %prec symbol is not defined, but it maintains the complaint 
for the case when the symbol is a nonterminal.  This patch might break 
compatibility with Bison 2.4 and 2.4.1.  I'm trying to decide if I should 
rewrite it to be a warning in Bison 2.4.2 but an error in Bison 2.5 and 
later.

>From da063083063aff422fac84b1131f2a3d7274b402 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Wed, 30 Dec 2009 00:10:17 -0500
Subject: [PATCH] POSIX: complain if %prec's token was not declared.

Reported by Florian Krohm at
<http://lists.gnu.org/archive/html/bug-bison/2009-12/msg00005.html>.
* NEWS (2.4.2): Document.
* src/reader.c (grammar_rule_check): Complain if %prec's symbol
is a nonterminal.
(grammar_current_rule_prec_set): Remove the nonterminal
complaint from here where it was implemented by simply setting
the class of the symbol to a token.  Thus, later complaints
about the symbol being undefined were suppressed contrary to
POSIX.
* tests/input.at (%prec takes a token): Update expected output.
(%prec's token must be declared): New test group.
---
 ChangeLog      |   16 ++++++++++++++++
 NEWS           |    8 ++++++++
 src/reader.c   |   15 ++++++++++++++-
 tests/input.at |   24 +++++++++++++++++++++++-
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f32a46e..ea3e69b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2009-12-29  Joel E. Denny  <address@hidden>
+
+       POSIX: complain if %prec's token was not declared.
+       Reported by Florian Krohm at
+       <http://lists.gnu.org/archive/html/bug-bison/2009-12/msg00005.html>.
+       * NEWS (2.4.2): Document.
+       * src/reader.c (grammar_rule_check): Complain if %prec's symbol
+       is a nonterminal.
+       (grammar_current_rule_prec_set): Remove the nonterminal
+       complaint from here where it was implemented by simply setting
+       the class of the symbol to a token.  Thus, later complaints
+       about the symbol being undefined were suppressed contrary to
+       POSIX.
+       * tests/input.at (%prec takes a token): Update expected output.
+       (%prec's token must be declared): New test group.
+
 2009-12-21  Joel E. Denny  <address@hidden>
 
        YYFAIL: deprecate.
diff --git a/NEWS b/NEWS
index 9ef543e..9e1a44e 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,14 @@ Bison News
 
 * Changes in version 2.4.2 (????-??-??):
 
+** `%prec IDENTIFIER' requires IDENTIFIER to be declared separately.
+
+  POSIX specifies that an error be reported for any identifier that does
+  not appear on the LHS of a grammar rule and that is not declared by
+  %token, %left, %right, or %nonassoc.  Bison 2.3b and later lost this
+  error report for the case when an identifier appears after a %prec
+  directive.  It is now restored.
+
 ** Detection of GNU M4 1.4.6 or newer during configure is improved.
 
 ** Warnings from gcc's -Wundef option about undefined YYENABLE_NLS,
diff --git a/src/reader.c b/src/reader.c
index 74d7dca..ac26e00 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -292,6 +292,20 @@ grammar_rule_check (const symbol_list *r)
            warn_at (r->location, _("unset value: $$"));
        }
   }
+
+  /* POSIX says that any identifier following %prec is a token, but
+     POSIX doesn't include %prec as one of the directives that can
+     declare an identifier to be a token.  However, if some complaint
+     was already issued, it might be about this token being undefined,
+     so don't risk a redundant complaint.  */
+  if (r->ruleprec && r->ruleprec->class != token_sym
+      && !complaint_issued)
+    {
+      aver (r->ruleprec->class == nterm_sym);
+      complain_at (r->location,
+                   _("identifier for %%prec is a nonterminal: %s"),
+                   r->ruleprec->tag);
+    }
 }
 
 
@@ -364,7 +378,6 @@ grammar_midrule_action (void)
 void
 grammar_current_rule_prec_set (symbol *precsym, location loc)
 {
-  symbol_class_set (precsym, token_sym, loc, false);
   if (current_rule->ruleprec)
     complain_at (loc, _("only one %s allowed per rule"), "%prec");
   current_rule->ruleprec = precsym;
diff --git a/tests/input.at b/tests/input.at
index be84f9c..9a5cc66 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -696,7 +696,29 @@ PREC: ;
 ]])
 
 AT_BISON_CHECK([input.y], [1], [],
-[[input.y:3.1-4: rule given for PREC, which is a token
+[[input.y:2.8-22: identifier for %prec is a nonterminal: PREC
+]])
+
+AT_CLEANUP
+
+
+## -------------------------------- ##
+## %prec's token must be declared.  ##
+## -------------------------------- ##
+
+AT_SETUP([[%prec's token must be declared]])
+
+# According to POSIX, %prec is not one of the directives that declares
+# an identifier to be a token even though only tokens can be specified
+# with it.
+
+AT_DATA([[input.y]],
+[[%%
+start: %prec PREC ;
+]])
+
+AT_BISON_CHECK([[input.y]], [[1]], [],
+[[input.y:2.14-17: symbol PREC is used, but is not defined as a token and has 
no rules
 ]])
 
 AT_CLEANUP
-- 
1.5.4.3