Re: [PATCH] -Werror: fix for rules useless in parser after conflicts.

[Joel E. Denny]

On Mon, 2 Aug 2010, Joel E. Denny wrote:

> On Mon, 2 Aug 2010, Paul Eggert wrote:
> 
> > On 08/01/10 21:36, Joel E. Denny wrote:
> > > it seems that any diagnostic printed 
> > > on stderr is at least a warning, and I can see how the user might 
> > > appreciate a guarantee that -Werror will prevent him from missing any 
> > > such 
> > > diagnostic.  That seems to be the thinking at comp.compilers.
> > 
> > It also sounds reasonable to me.
> 
> Also, we can consider that -Werror is really a tool for the *maintainers* 
> of any project that depends on bison.  That is, a formal release of such a 
> project should either distribute the output of bison so that the release 
> does not require users to run bison, or -Werror should be turned off by 
> default in the release.  Otherwise, in general, users who install a new 
> bison with new warnings will see build failures for the project.  From 
> that viewpoint, there is no legitimate backward compatibility issue to 
> consider here.  We can feel free to promote conflicts to warnings.
> 
> Nevertheless, we might lose a desired -Werror use case.  That is, there 
> might be maintainers of projects that depend on bison who argue that they 
> want -Werror for all the other warnings from bison.  However, because 
> conflicts are inherent in the design of their grammars and fluctuate 
> significantly as the grammar evolves, they don't want to be forced to fuss 
> with maintaining a count in %expect or %expect-rr in order to suppress the 
> conflict warning.  To satisfy them, we could add a -Wconflicts that would 
> be on by default.  They could then combine -Werror with -Wno-conflicts.  
> Such a maintainer might appreciate being able to use -Wno-conflicts to 
> fully suppress the conflicts report in a formal release anyway.
> 
> Actually, we should probably split -Wconflicts into -Wconflicts-sr and 
> -Wconflicts-rr given that S/R and R/R conflicts are usually considered to 
> be very different levels of severity.

Implemented below, but I still need to add some test cases, so I'll do 
more work later and push if there are no objections.

If all my fixes from today are fine with everyone, we still need to look 
into updating gnulib, but then I think it'll be time for a 2.5 release 
candidate.

>From 47c4d6a3ff9c1f434ed07ec4983d9a782d7c6e84 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sun, 27 Mar 2011 22:38:32 -0400
Subject: [PATCH] Add -Wconflicts-sr and -Wconflicts-rr.

Thus, conflict reports are now affected by -Werror and -Wnone
(unless %expect or %expect-rr is specified).  Reported by George
Neuner at
<http://lists.gnu.org/archive/html/bug-bison/2010-08/msg00002.html>.
* NEWS (2.5): Document.
* doc/bison.texinfo (Bison Options): Document.
* src/complain.c, src/complain.h (set_warning_issued): Export
function.
* src/conflicts.c (conflicts_print): Suppress conflict report
based on -Wno-conflicts-sr and -Wno-conflicts-rr, and treat
conflicts as errors if -Werror.
* src/getargs.c (warnings_flag): Initialize with
warnings_conflicts_sr and warnings_conflicts_rr as well.
(warnings_args, warnings_types): Add entries for
warnings_conflicts_sr and warnings_conflicts_rr.
(usage): Update.
* src/getargs.h (enum warnings): Add entries for
warnings_conflicts_sr and warnings_conflicts_rr.
* tests/local.at (AT_BISON_CHECK_NO_XML): Update now that the
conflict report can produce a "warnings being treated as errors"
message.
---
 ChangeLog         |   25 +++++++++++++++++++++++++
 NEWS              |   18 ++++++++++++++++++
 doc/bison.texinfo |    8 ++++++++
 src/complain.c    |    2 +-
 src/complain.h    |    7 +++++++
 src/conflicts.c   |    8 ++++++++
 src/getargs.c     |    9 ++++++++-
 src/getargs.h     |    4 +++-
 tests/local.at    |    7 +++++++
 9 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 893beb3..bf4c16c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,30 @@
 2011-03-27  Joel E. Denny  <address@hidden>
 
+       Add -Wconflicts-sr and -Wconflicts-rr.
+       Thus, conflict reports are now affected by -Werror and -Wnone
+       (unless %expect or %expect-rr is specified).  Reported by George
+       Neuner at
+       <http://lists.gnu.org/archive/html/bug-bison/2010-08/msg00002.html>.
+       * NEWS (2.5): Document.
+       * doc/bison.texinfo (Bison Options): Document.
+       * src/complain.c, src/complain.h (set_warning_issued): Export
+       function.
+       * src/conflicts.c (conflicts_print): Suppress conflict report
+       based on -Wno-conflicts-sr and -Wno-conflicts-rr, and treat
+       conflicts as errors if -Werror.
+       * src/getargs.c (warnings_flag): Initialize with
+       warnings_conflicts_sr and warnings_conflicts_rr as well.
+       (warnings_args, warnings_types): Add entries for
+       warnings_conflicts_sr and warnings_conflicts_rr.
+       (usage): Update.
+       * src/getargs.h (enum warnings): Add entries for
+       warnings_conflicts_sr and warnings_conflicts_rr.
+       * tests/local.at (AT_BISON_CHECK_NO_XML): Update now that the
+       conflict report can produce a "warnings being treated as errors"
+       message.
+
+2011-03-27  Joel E. Denny  <address@hidden>
+
        Pacify maintainer-check-posix.
        Adding command-line options after the grammar file name is not
        permitted, so disable checks that do that when
diff --git a/NEWS b/NEWS
index 7584f18..3d02e03 100644
--- a/NEWS
+++ b/NEWS
@@ -309,6 +309,24 @@ Bison News
 
     bison -Wall,no-yacc gram.y
 
+*** Bison now treats S/R and R/R conflicts like other warnings:
+
+  Previously, conflict reports were independent of Bison's normal
+  warning system.  Now, Bison recognizes the warning categories
+  "conflicts-sr" and "conflicts-rr".  This change has important
+  consequences for the -W and --warnings command-line options.  For
+  example:
+
+    bison -Wno-conflicts-sr gram.y  # S/R conflicts not reported
+    bison -Wno-conflicts-rr gram.y  # R/R conflicts not reported
+    bison -Wnone            gram.y  # no conflicts are reported
+    bison -Werror           gram.y  # any conflict is an error
+
+  However, as before, if the %expect or %expect-rr directive is
+  specified, an unexpected number of conflicts is an error, and an
+  expected number of conflicts is not reported, so -W and --warning
+  then have no effect on the conflict report.
+
 *** The "none" category no longer disables a preceding "error":
 
   For example, for the following command line, Bison now reports
diff --git a/doc/bison.texinfo b/doc/bison.texinfo
index cb50a01..e0d5aa5 100644
--- a/doc/bison.texinfo
+++ b/doc/bison.texinfo
@@ -8429,6 +8429,14 @@ be false alarms in existing grammars employing the Yacc 
constructs
 @item yacc
 Incompatibilities with POSIX Yacc.
 
address@hidden conflicts-sr
address@hidden conflicts-rr
+S/R and R/R conflicts.  These warnings are enabled by default.  However, if
+the @code{%expect} or @code{%expect-rr} directive is specified, an
+unexpected number of conflicts is an error, and an expected number of
+conflicts is not reported, so @option{-W} and @option{--warning} then have
+no effect on the conflict report.
+
 @item other
 All warnings not categorized above.  These warnings are enabled by default.
 
diff --git a/src/complain.c b/src/complain.c
index 5629dd3..5c07fb3 100644
--- a/src/complain.c
+++ b/src/complain.c
@@ -94,7 +94,7 @@ error_message (location *loc,
 | Report a warning, and proceed.  |
 `--------------------------------*/
 
-static void
+void
 set_warning_issued (void)
 {
   static bool warning_issued = false;
diff --git a/src/complain.h b/src/complain.h
index 3d867f5..07d401c 100644
--- a/src/complain.h
+++ b/src/complain.h
@@ -25,6 +25,13 @@
 extern "C" {
 # endif
 
+/** Record that a warning is about to be issued, and treat it as an
+    error if <tt>warnings_flag & warnings_error</tt>.  This is exported
+    only for the sake of Yacc-compatible conflict reports in conflicts.c.
+    All other warnings should be implemented in complain.c and should use
+    the normal warning format.  */
+void set_warning_issued (void);
+
 /** Informative messages, but we proceed.  Report iff
     <tt>warnings_flag & warnings_other</tt>.  */
 
diff --git a/src/conflicts.c b/src/conflicts.c
index 0437670..ef8d648 100644
--- a/src/conflicts.c
+++ b/src/conflicts.c
@@ -594,8 +594,16 @@ conflicts_print (void)
     return;
 
   /* Report the total number of conflicts on STDERR.  */
+  if (expected_sr_conflicts == -1 && expected_rr_conflicts == -1)
+    {
+      if (!(warnings_flag & warnings_conflicts_sr))
+        src_total = 0;
+      if (!(warnings_flag & warnings_conflicts_rr))
+        rrc_total = 0;
+    }
   if (src_total | rrc_total)
     {
+      set_warning_issued ();
       if (! yacc_flag)
        fprintf (stderr, "%s: ", current_file);
       conflict_report (stderr, src_total, rrc_total);
diff --git a/src/getargs.c b/src/getargs.c
index 03c7a3d..9f8a72d 100644
--- a/src/getargs.c
+++ b/src/getargs.c
@@ -63,7 +63,8 @@ bool glr_parser = false;
 
 int report_flag = report_none;
 int trace_flag = trace_none;
-int warnings_flag = warnings_other;
+int warnings_flag = warnings_conflicts_sr | warnings_conflicts_rr
+                    | warnings_other;
 
 static struct bison_language const valid_languages[] = {
   { "c", "c-skel.m4", ".c", ".h", true },
@@ -232,6 +233,8 @@ static const char * const warnings_args[] =
   "none            - no warnings",
   "midrule-values  - unset or unused midrule values",
   "yacc            - incompatibilities with POSIX Yacc",
+  "conflicts-sr    - S/R conflicts",
+  "conflicts-rr    - R/R conflicts",
   "other           - all other warnings",
   "all             - all of the above",
   "error           - warnings are errors",
@@ -243,6 +246,8 @@ static const int warnings_types[] =
   warnings_none,
   warnings_midrule_values,
   warnings_yacc,
+  warnings_conflicts_sr,
+  warnings_conflicts_rr,
   warnings_other,
   warnings_all,
   warnings_error
@@ -333,6 +338,8 @@ Output:\n\
 Warning categories include:\n\
   `midrule-values'  unset or unused midrule values\n\
   `yacc'            incompatibilities with POSIX Yacc\n\
+  `conflicts-sr'    S/R conflicts (enabled by default)\n\
+  `conflicts-rr'    R/R conflicts (enabled by default)\n\
   `other'           all other warnings (enabled by default)\n\
   `all'             all the warnings\n\
   `no-CATEGORY'     turn off warnings in CATEGORY\n\
diff --git a/src/getargs.h b/src/getargs.h
index dc92584..20a4143 100644
--- a/src/getargs.h
+++ b/src/getargs.h
@@ -123,7 +123,9 @@ enum warnings
     warnings_error            = 1 << 0, /**< Warnings are treated as errors.  
*/
     warnings_midrule_values   = 1 << 1, /**< Unset or unused midrule values.  
*/
     warnings_yacc             = 1 << 2, /**< POSIXME.  */
-    warnings_other            = 1 << 3, /**< All other warnings.  */
+    warnings_conflicts_sr     = 1 << 3, /**< S/R conflicts.  */
+    warnings_conflicts_rr     = 1 << 4, /**< R/R conflicts.  */
+    warnings_other            = 1 << 5, /**< All other warnings.  */
     warnings_all              = ~warnings_error /**< All above warnings.  */
   };
 /** What warnings are issued.  */
diff --git a/tests/local.at b/tests/local.at
index 1aaab9b..a194c59 100644
--- a/tests/local.at
+++ b/tests/local.at
@@ -302,6 +302,13 @@ m4_if(m4_bregexp([$4], [: warning: ]), [-1], [],
             sed -n '/: warning: /=' at-bison-check-warnings \
             | sed -n 1p \
           `"
+          at_bison_check_first_tmp="` \
+            sed -n '/conflicts: [0-9].*reduce$/=' at-bison-check-warnings \
+            | sed -n 1p \
+          `"
+          if test $at_bison_check_first_tmp -lt $at_bison_check_first; then
+            at_bison_check_first=$at_bison_check_first_tmp
+          fi
           if test $at_bison_check_first -gt 1; then
             sed -n "1,`expr $at_bison_check_first - 1`"p \
               at-bison-check-warnings > experr
-- 
1.7.0.4