Patchwork fix PR 44782, implement -fmax-errors as a common option

login
register
mail settings
Submitter Nathan Froyd
Date Nov. 9, 2010, 4:44 p.m.
Message ID <20101109164437.GE7991@nightcrawler>
Download mbox | patch
Permalink /patch/70561/
State New
Headers show

Comments

Nathan Froyd - Nov. 9, 2010, 4:44 p.m.
On Tue, Nov 09, 2010 at 03:28:09PM +0000, Joseph S. Myers wrote:
> On Tue, 9 Nov 2010, Nathan Froyd wrote:
> > The patch below implements -fmax-errors for the C family of languages,
> > as requested by PR 44782.  I chose -fmax-errors rather than
> > -ferror-limit for compatibility with the Fortran front end's option.  I
> > suppose if people wanted, we could add -ferror-limit as an undocumented
> > alias.
> > 
> > Needs C approval for the c-family changes and a diagnostic maintainer's
> > approval for diagnostic.* changes.
> 
> I don't think this should be a C-family option; just like -fshow-column, 
> for example, it should be a generic option in common.opt.  If some 
> languages' diagnostics aren't affected by it because they don't go through 
> the common machinery, that is a problem with those languages (and maybe a 
> consequence of missing features in the generic diagnostic code that need 
> implementing before those languages can switch to it).

Hm, OK.  The patch below implements -fmax-errors as a common option,
with some Fortran tweaks to make it work as expected.  I am unsure what
to do about the existing Fortran -fmax-errors documentation; I opted to
leave it in for now, as options like -fsyntax-only and -pedantic are
documented in both generic GCC and Fortran documentation.

Tested on x86_64-unknown-linux-gnu with -fmax-errors testcases for
C/C++/Fortran.  OK to commit?  (Not aimed at Joseph, as he is not
responsible for the areas these changes touch.)

-Nathan

gcc/
	PR c/44782
	* common.opt (fmax-errors=): New option.
	* opts.c (common_handle_option) [OPT_fmax_errors_]: Handle it.
	* diagnostic.h (struct diagnostic_context): Add max_errors field.
	* diagnostic.c (diagnostic_initialize): Initialize it.
	(diagnostic_action_after_output): Exit if more than max_errors
	have been output.
	* doc/invoke.texi (Warning Options): Add -fmax-errors.
	(-fmax-errors): Document.

gcc/fortran/
	PR c/44782
	* options.c (gfc_post_options): Initialize gfc_option.max_errors.
	(gfc_handle_option) [OPT_fmax_errors_]: Remove.
	* lang.opt (fmax-errors=): Remove.

gcc/testsuite/
	PR c/44782
	* c-c++-common/fmax-errors.c: New test.
Chris Lattner - Nov. 9, 2010, 6:43 p.m.
On Nov 9, 2010, at 8:44 AM, Nathan Froyd wrote:

> On Tue, Nov 09, 2010 at 03:28:09PM +0000, Joseph S. Myers wrote:
>> On Tue, 9 Nov 2010, Nathan Froyd wrote:
>>> The patch below implements -fmax-errors for the C family of languages,
>>> as requested by PR 44782.  I chose -fmax-errors rather than
>>> -ferror-limit for compatibility with the Fortran front end's option.  I
>>> suppose if people wanted, we could add -ferror-limit as an undocumented
>>> alias.
>>> 
>>> Needs C approval for the c-family changes and a diagnostic maintainer's
>>> approval for diagnostic.* changes.
>> 
>> I don't think this should be a C-family option; just like -fshow-column, 
>> for example, it should be a generic option in common.opt.  If some 
>> languages' diagnostics aren't affected by it because they don't go through 
>> the common machinery, that is a problem with those languages (and maybe a 
>> consequence of missing features in the generic diagnostic code that need 
>> implementing before those languages can switch to it).
> 
> Hm, OK.  The patch below implements -fmax-errors as a common option,
> with some Fortran tweaks to make it work as expected.  I am unsure what
> to do about the existing Fortran -fmax-errors documentation; I opted to
> leave it in for now, as options like -fsyntax-only and -pedantic are
> documented in both generic GCC and Fortran documentation.
> 
> Tested on x86_64-unknown-linux-gnu with -fmax-errors testcases for
> C/C++/Fortran.  OK to commit?  (Not aimed at Joseph, as he is not
> responsible for the areas these changes touch.)

It would be nice to support -ferror-limit as an alias at least.  People are much more likely to want to write makefiles that work with both clang and gcc than with gcc and gfortran.

-Chris
Tobias Burnus - Nov. 9, 2010, 6:48 p.m.
Chris Lattner wrote:
> It would be nice to support -ferror-limit as an alias at least.
> People are much more likely to want to write makefiles
> that work with both clang and gcc than with gcc and gfortran.

I am not sure which is more likely. However, I do know many people which 
use Makefile written for gcc and gfortran. While those typically use 
CFLAG and FFLAG, I think the users would be puzzled if GCC has two 
different option names depending on the front end.

However, how about adding -fmax-error as alias to CLANG? Then the users 
of CLANG are compatible to both gcc and gfortran.

Tobias
Chris Lattner - Nov. 9, 2010, 6:56 p.m.
On Nov 9, 2010, at 10:48 AM, Tobias Burnus wrote:

> Chris Lattner wrote:
>> It would be nice to support -ferror-limit as an alias at least.
>> People are much more likely to want to write makefiles
>> that work with both clang and gcc than with gcc and gfortran.
> 
> I am not sure which is more likely. However, I do know many people which use Makefile written for gcc and gfortran. While those typically use CFLAG and FFLAG, I think the users would be puzzled if GCC has two different option names depending on the front end.
> 
> However, how about adding -fmax-error as alias to CLANG? Then the users of CLANG are compatible to both gcc and gfortran.

That's fine, and perhaps also a good idea.  However, there are lots of already shipped versions of clang out there.  They all support -ferror-limit but not -fmax-error.  IMO, -ferror-limit is a better name anyway, why not enhance gfortran to accept -ferror-limit?

-Chris
Tobias Burnus - Nov. 9, 2010, 7:03 p.m.
Chris Lattner wrote:
> That's fine, and perhaps also a good idea.  However, there are lots of already shipped versions of clang out there.  They all support -ferror-limit but not -fmax-error.  IMO, -ferror-limit is a better name anyway, why not enhance gfortran to accept -ferror-limit?

Ditto for gfortran. Regarding the name: -fmax-errors=<n>  (with "-s") 
and -ferror-limit=<n> sound both fine to me - I do not find the limit 
version superior. Regarding the accepting the other option additionally, 
that's what Nathan already proposed:

> > On Tue, 9 Nov 2010, Nathan Froyd wrote:

>>>  I suppose if people wanted, we could add -ferror-limit as an undocumented
>>>  alias.

Tobias
Steve Kargl - Nov. 11, 2010, 9:16 p.m.
On Tue, Nov 09, 2010 at 11:44:38AM -0500, Nathan Froyd wrote:
> 
> gcc/fortran/
> 	PR c/44782
> 	* options.c (gfc_post_options): Initialize gfc_option.max_errors.
> 	(gfc_handle_option) [OPT_fmax_errors_]: Remove.
> 	* lang.opt (fmax-errors=): Remove.

This part looks ok to me.  I can't approve the other parts of
the patch.  Note, I have no opinion of the -ferror-limit vs
-fmax-errors issue.  Of course, I chose the name -fmax-errors. :)
Gabriel Dos Reis - Nov. 11, 2010, 9:24 p.m.
On Thu, Nov 11, 2010 at 3:16 PM, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Tue, Nov 09, 2010 at 11:44:38AM -0500, Nathan Froyd wrote:
>>
>> gcc/fortran/
>>       PR c/44782
>>       * options.c (gfc_post_options): Initialize gfc_option.max_errors.
>>       (gfc_handle_option) [OPT_fmax_errors_]: Remove.
>>       * lang.opt (fmax-errors=): Remove.
>
> This part looks ok to me.  I can't approve the other parts of
> the patch.  Note, I have no opinion of the -ferror-limit vs
> -fmax-errors issue.  Of course, I chose the name -fmax-errors. :)


The path is OK.

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 551c335..b319456 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1119,6 +1119,10 @@  fmath-errno
 Common Report Var(flag_errno_math) Init(1) Optimization
 Set errno after built-in math functions
 
+fmax-errors=
+Common Joined RejectNegative UInteger Var(flag_max_errors)
+-fmax-errors=<number>	Maximum number of errors to report
+
 fmem-report
 Common Report Var(mem_report)
 Report on permanent memory allocation
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 40ddc23..d297cdd 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -109,6 +109,7 @@  diagnostic_initialize (diagnostic_context *context, int n_opts)
   context->fatal_errors = false;
   context->dc_inhibit_warnings = false;
   context->dc_warn_system_headers = false;
+  context->max_errors = 0;
   context->internal_error = NULL;
   diagnostic_starter (context) = default_diagnostic_starter;
   diagnostic_finalizer (context) = default_diagnostic_finalizer;
@@ -219,6 +220,17 @@  diagnostic_action_after_output (diagnostic_context *context,
 	  diagnostic_finish (context);
 	  exit (FATAL_EXIT_CODE);
 	}
+      if (context->max_errors != 0
+	  && ((unsigned) (diagnostic_kind_count (context, DK_ERROR)
+			  + diagnostic_kind_count (context, DK_SORRY))
+	      >= context->max_errors))
+	{
+	  fnotice (stderr,
+		   "compilation terminated due to -fmax-errors=%u.\n",
+		   context->max_errors);
+	  diagnostic_finish (context);
+	  exit (FATAL_EXIT_CODE);
+	}
       break;
 
     case DK_ICE:
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 99671c6..8074354 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -128,6 +128,9 @@  struct diagnostic_context
   /* True if warnings should be given in system headers.  */
   bool dc_warn_system_headers;
 
+  /* Maximum number of errors to report.  */
+  unsigned int max_errors;
+
   /* This function is called before any message is printed out.  It is
      responsible for preparing message prefix and such.  For example, it
      might say:
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d3b702b..57014c0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -229,7 +229,8 @@  Objective-C and Objective-C++ Dialects}.
 
 @item Warning Options
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
-@gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
+@gccoptlist{-fsyntax-only  fmax-errors=@var{n}  -pedantic @gol
+-pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  -Warray-bounds @gol
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc++-compat -Wc++0x-compat -Wcast-align  -Wcast-qual  @gol
@@ -2775,6 +2776,15 @@  warnings but control the kinds of diagnostics produced by GCC.
 @opindex fsyntax-only
 Check the code for syntax errors, but don't do anything beyond that.
 
+@item -fmax-errors=@var{n}
+@opindex fmax-errors
+Limits the maximum number of error messages to @var{n}, at which point
+GCC bails out rather than attempting to continue processing the source
+code.  If @var{n} is 0 (the default), there is no limit on the number
+of error messages produced.  If @option{-Wfatal-errors} is also
+specified, then @option{-Wfatal-errors} takes precedence over this
+option.
+
 @item -w
 @opindex w
 Inhibit all warning messages.
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 6088730..371b71d 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -438,10 +438,6 @@  fmax-array-constructor=
 Fortran RejectNegative Joined UInteger
 -fmax-array-constructor=<n>	Maximum number of objects in an array constructor
 
-fmax-errors=
-Fortran RejectNegative Joined UInteger
--fmax-errors=<n>	Maximum number of errors to report
-
 fmax-identifier-length=
 Fortran RejectNegative Joined UInteger
 -fmax-identifier-length=<n>	Maximum identifier length
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index 36dd9c8..eaaef14 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -273,6 +273,10 @@  gfc_post_options (const char **pfilename)
   if (flag_compare_debug)
     gfc_option.dump_fortran_original = 0;
 
+  /* Make -fmax-errors visible to gfortran's diagnostic machinery.  */
+  if (global_options_set.x_flag_max_errors)
+    gfc_option.max_errors = flag_max_errors;
+
   /* Verify the input file name.  */
   if (!filename || strcmp (filename, "-") == 0)
     {
@@ -760,10 +764,6 @@  gfc_handle_option (size_t scode, const char *arg, int value,
       gfc_option.flag_max_array_constructor = value > 65535 ? value : 65535;
       break;
 
-    case OPT_fmax_errors_:
-      gfc_option.max_errors = value;
-      break;
-
     case OPT_fmax_stack_var_size_:
       gfc_option.flag_max_stack_var_size = value;
       break;
diff --git a/gcc/opts.c b/gcc/opts.c
index b2019c6..91ef609 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2217,6 +2217,10 @@  common_handle_option (struct gcc_options *opts,
       global_dc->dc_inhibit_warnings = true;
       break;
 
+    case OPT_fmax_errors_:
+      global_dc->max_errors = value;
+      break;
+
     case OPT_fuse_linker_plugin:
       /* No-op. Used by the driver and passed to us because it starts with f.*/
       break;
diff --git a/gcc/testsuite/c-c++-common/fmax-errors.c b/gcc/testsuite/c-c++-common/fmax-errors.c
new file mode 100644
index 0000000..1ef78eb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fmax-errors.c
@@ -0,0 +1,11 @@ 
+/* PR c/44782 */
+/* { dg-do compile } */
+/* { dg-options "-fmax-errors=3" } */
+
+void foo (unsigned int i, unsigned int j)
+{
+  (i) ();			/* { dg-error "" } */
+  (j) ();			/* { dg-error "" } */
+  (i+j) ();			/* { dg-error "" } */
+  (i*j) ();			/* no error here due to -fmax-errors */
+} /* { dg-prune-output "compilation terminated" } */