User account creation filtered due to spam.

Bug 26061 - error and warning count
Summary: error and warning count
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: unknown
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2006-02-01 10:41 UTC by Kevin André
Modified: 2008-07-21 11:32 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-02-01 12:49:35


Attachments
Better patch (774 bytes, patch)
2007-12-15 22:05 UTC, İsmail Dönmez
Details | Diff
Unbreak lib{gomp,stdc++,ffi} tests (1.58 KB, patch)
2007-12-16 16:12 UTC, İsmail Dönmez
Details | Diff
Fix typo in the last patch (1.43 KB, patch)
2007-12-16 16:14 UTC, İsmail Dönmez
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin André 2006-02-01 10:41:53 UTC
I think it would be nice if gcc would give an error/warning count at the end of its output. Something like:

$ gcc -Wall test.c
test.c:3: error: conflicting types for 'p'
test.c:2: error: previous declaration of 'p' was here
test.c:3: error: conflicting types for 'p'
test.c:2: error: previous declaration of 'p' was here
test.c: In function `main':
test.c:7: error: `d' undeclared (first use in this function)
test.c:7: error: (Each undeclared identifier is reported only once
test.c:7: error: for each function it appears in.)
test.c:6: warning: unused variable `b'
gcc: *** 3 errors, 1 warning

I currently use gcc 3.4.4, and I think gcc/g++ don't have such a feature yet because AFAIK it is not listed in the 'new features' list of versions 4.0 and 4.1.

Just in case, the test file I used is this:

int p(int);
int p(float);

int main() {
	int a, b, c;
	c = a + d;
	p('C');
}
Comment 1 Andrew Pinski 2006-02-01 12:49:35 UTC
Confirmed, I really don't know if this is useful or not.  
Comment 2 Kevin André 2006-02-02 14:39:49 UTC
IMO this is a useful feature because the number of lines of error output that GCC produces for a file is not (always) a correct measure for the amount of errors and warnings produced for that file. This is because GCC produces not only the error messages themselves, but also extra helpful output. And it is also nice to see in one fast look how many errors I still have to fix in my code.
Comment 3 Andrew Pinski 2006-02-02 14:42:49 UTC
Subject: Re:  error and warning count

> 
> 
> 
> ------- Comment #2 from hyperquantum at gmail dot com  2006-02-02 14:39 -------
> IMO this is a useful feature because the number of lines of error output that
> GCC produces for a file is not (always) a correct measure for the amount of
> errors and warnings produced for that file. This is because GCC produces not
> only the error messages themselves, but also extra helpful output. And it is
> also nice to see in one fast look how many errors I still have to fix in my
> code.

It is not really a good measure as sometimes fixing one error will fix the rest.

-- Pinski
Comment 4 Gabriel Dos Reis 2006-02-02 15:12:40 UTC
Subject: Re:  error and warning count

"pinskia at physics dot uc dot edu" <gcc-bugzilla@gcc.gnu.org> writes:

| > IMO this is a useful feature because the number of lines of error output that
| > GCC produces for a file is not (always) a correct measure for the amount of
| > errors and warnings produced for that file. This is because GCC produces not
| > only the error messages themselves, but also extra helpful output. And it is
| > also nice to see in one fast look how many errors I still have to fix in my
| > code.
| 
| It is not really a good measure as sometimes fixing one error will fix the
| rest.

true, but it is still a helpful information.  That reminds of when I
first iused sun's CC -- it really was a pleasant experience.

-- Gaby
Comment 5 Manuel López-Ibáñez 2007-01-21 23:35:52 UTC
Gabriel, what do you think about this? Does it need testcases? Should I submit it?


Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c        (revision 121027)
+++ gcc/toplev.c        (working copy)
@@ -2059,6 +2059,10 @@ finalize (void)

   /* Language-specific end of compilation actions.  */
   lang_hooks.finish ();
+
+  /* Print error / warning counts.  */
+  fprintf (stderr, "\n%s: *** %d errors, %d warnings",
+          progname, errorcount, warningcount);
 }

 /* Initialize the compiler, and compile the input file.  */
Comment 6 İsmail Dönmez 2007-01-26 21:29:50 UTC
Maybe a better version could be like this,

--- gcc/toplev.c        2006-10-09 19:27:14.000000000 +0300
+++ gcc/toplev.c        2007-01-26 20:59:19.395519510 +0200
@@ -1975,6 +1975,12 @@

   /* Language-specific end of compilation actions.  */
   lang_hooks.finish ();
+
+
+  /* Print error / warning counts.  */
+  if ( errorcount || warningcount )
+    fprintf (stderr, "\n%s: *** %d errors, %d warnings",
+             progname, errorcount, warningcount);
 }

 /* Initialize the compiler, and compile the input file.  */
Comment 7 Manuel López-Ibáñez 2007-01-26 21:59:31 UTC
Whatever version is fine for me. Gabriel, any preference?
Comment 8 Kevin André 2007-01-26 23:18:19 UTC
I prefer the second version. The output is only useful in case there are errors or warnings, not when you have a flawless compilation.
Comment 9 İsmail Dönmez 2007-01-26 23:29:03 UTC
There should also be a newline,

--- gcc/toplev.c        2006-10-09 19:27:14.000000000 +0300
+++ gcc/toplev.c        2007-01-26 20:59:19.395519510 +0200
@@ -1975,6 +1975,12 @@

   /* Language-specific end of compilation actions.  */
   lang_hooks.finish ();
+
+
+  /* Print error / warning counts.  */
+  if ( errorcount || warningcount )
+    fprintf (stderr, "\n%s: *** %d errors, %d warnings\n",
+             progname, errorcount, warningcount);
 }

 /* Initialize the compiler, and compile the input file.  */
Comment 10 Manuel López-Ibáñez 2007-01-28 17:17:54 UTC
I have a patch bootstrapped and regression tested that implements my version but I am sure it will require minimal changes to implement whatever is decided. So if you don't get an answer here, please raise the issue at gcc@gcc.gnu.org and let's see what others think. If you convince someone who can approve the patch, I will implement it and submit it.
Comment 11 patchapp@dberlin.org 2007-02-14 05:26:42 UTC
Subject: Bug number PR26061

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01190.html
Comment 12 Geoff Keating 2007-07-03 00:45:09 UTC
[Here's what I sent to gcc-patches as a review of this patch:]
Doing this will certainly break many tools which parse the output of GCC, especially in the case of a successful compilation which produced some warnings.  I think, though, that it would be safe to produce one extra notice in the case of a compilation which produced errors, saying how many errors were output; that would help in the case where the last few hundred messages from GCC are warnings and the user wonders why their build failed.  It should be combined with the message we already have about 'treating warnings about errors because of -Werror'.

I'm hard-pressed to think of a reason why, if this is implemented, you would want to be able to switch it off, so I see no reason to have a flag (unless someone else thinks of a reason).
Comment 13 Manuel López-Ibáñez 2007-07-03 09:03:03 UTC
(In reply to comment #12)
> [Here's what I sent to gcc-patches as a review of this patch:]
> Doing this will certainly break many tools which parse the output of GCC,

In the same way that adding any other output message would break those tools if those tools aren't prepared to deal with undefined output. We have a lot of messages that don't follow any prefix: "warnings being treated as errors",  "compilation terminated due to -Wfatal-errors", etc. They are special cases in the testsuite. The message "gcc: *** 3 errors, 1 warning" cannot be confused with any existing output and any tool that parses the output of GCC should ignore messages that it cannot understand. In the worst case, the modification for any existing parser to ignore that line would be trivial.

> especially in the case of a successful compilation which produced some
> warnings.  

I don't get how a parser can be *especially* confused in this case unless you are parsing for the word "error" to determine the success of the compilation. And that would be doubly stupid: 1) you should actually look for " error: " and 2) the return value of GCC already tells whether the compilation succeeded. 

> I'm hard-pressed to think of a reason why, if this is implemented, you would
> want to be able to switch it off, so I see no reason to have a flag (unless
> someone else thinks of a reason).

I cannot think of a reason either and if you strongly need to filter out that from the output, the regexp would be trivial and it will only match that line.

Thanks for the review.
Comment 14 İsmail Dönmez 2007-12-15 22:05:50 UTC
Created attachment 14769 [details]
Better patch
Comment 15 İsmail Dönmez 2007-12-15 22:06:53 UTC
Attached is a better patch which adds -f{no}-show-error-count and uses it in regression tests so regtests works now. IDE's also can use this option.

Is it possible to get this in for gcc 4.3 or gcc 4.4?
Comment 16 İsmail Dönmez 2007-12-16 16:12:57 UTC
Created attachment 14780 [details]
Unbreak lib{gomp,stdc++,ffi} tests
Comment 17 İsmail Dönmez 2007-12-16 16:14:22 UTC
Created attachment 14781 [details]
Fix typo in the last patch
Comment 18 Manuel López-Ibáñez 2007-12-16 16:51:35 UTC
Ismail,

I am pretty sure that this won't be accepted for GCC 4.3, since it is not a regression fix. Actually, I am unsure it would be accepted at all following the comments to my original patch.

Nevertheless, if you want to take the risk of wasting your time, some advice:

* My original patch modified several testsuite files: http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01190.html, I guess you would need something equivalent.

* Joseph pointed that: 
"A human readable message can't just be passed to fprintf like that, you 
need to arrange for it to be extracted for translation and translated into 
the user's language.  Calling existing diagnostic functions does that, 
otherwise the string needs surrounding with _()."

* Also he suggested:
"this change to output should perhaps be off by 
default, with a new option to enable it people wanting this style can put 
in their Makefiles."

Good luck!

Comment 19 İsmail Dönmez 2007-12-16 16:55:42 UTC
Hi,

(In reply to comment #18)
> * My original patch modified several testsuite files:
> http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01190.html, I guess you would need
> something equivalent.

I added a flag to disable this to testsuite files (-fno-show-error-count)

> * Joseph pointed that: 
> "A human readable message can't just be passed to fprintf like that, you 
> need to arrange for it to be extracted for translation and translated into 
> the user's language.  Calling existing diagnostic functions does that, 
> otherwise the string needs surrounding with _()."

True, I'll fix.

> * Also he suggested:
> "this change to output should perhaps be off by 
> default, with a new option to enable it people wanting this style can put 
> in their Makefiles."

I really don't want to make it default off as it's really useful. I hope to fix the patch and make it default on for gcc 4.4 at least.

Thanks for the suggestions,
ismail
Comment 20 Kevin André 2008-07-21 11:32:20 UTC
(In reply to comment #19)

> I really don't want to make it default off as it's really useful. I hope to fix
> the patch and make it default on for gcc 4.4 at least.

I agree. What's the status for getting it into 4.4?

People complain that changing GCC's output will break tools that parse that output. I can understand why they're complaining, but IMHO this reveals a more fundamental problem. The current diagnostic output tries to serve two purposes: being readable for humans and being parseable by tools. And I think those two should be separated. Why not have the default output be human-friendly, and introduce a separate option, say "-fparseable-diagnostics" to switch to an easily parseable output format?