diff mbox series

[2/7] GCOV: introduce usage of terminal colors.

Message ID b9c2d6b814e2aec9265c2542294d9fd3c299c7bd.1509005504.git.mliska@suse.cz
State New
Headers show
Series GCOV: another set of improvements | expand

Commit Message

Martin Liška Oct. 26, 2017, 8:11 a.m. UTC
I consider using colors in context of gcov as very useful. There's
example for tramp3d:
https://pste.eu/p/Tl2D.html

gcc/ChangeLog:

2017-10-23  Martin Liska  <mliska@suse.cz>

	* color-macros.h: New file.
	* diagnostic-color.c: Factor out color related to macros to
	color-macros.h.
	* doc/gcov.texi: Document -k option.
	* gcov.c (INCLUDE_STRING): Include string.h.
	(print_usage): Add -k option.
	(process_args): Parse it.
	(pad_count_string): New function.
	(output_line_beginning): Likewise.
	(DEFAULT_LINE_START): New macro.
	(output_lines): Support color output.
---
 gcc/color-macros.h     |  50 +++++++++++++++++++++++
 gcc/diagnostic-color.c |  27 +------------
 gcc/doc/gcov.texi      |   9 +++++
 gcc/gcov.c             | 105 +++++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 148 insertions(+), 43 deletions(-)
 create mode 100644 gcc/color-macros.h

Comments

Nathan Sidwell Oct. 30, 2017, 12:17 p.m. UTC | #1
On 10/26/2017 04:11 AM, marxin wrote:
> I consider using colors in context of gcov as very useful. There's
> example for tramp3d:
> https://pste.eu/p/Tl2D.html

nice!

> gcc/ChangeLog:
> 
> 2017-10-23  Martin Liska  <mliska@suse.cz>
> 
> 	* color-macros.h: New file.
> 	* diagnostic-color.c: Factor out color related to macros to
> 	color-macros.h.
> 	* doc/gcov.texi: Document -k option.
> 	* gcov.c (INCLUDE_STRING): Include string.h.
> 	(print_usage): Add -k option.
> 	(process_args): Parse it.
> 	(pad_count_string): New function.
> 	(output_line_beginning): Likewise.
> 	(DEFAULT_LINE_START): New macro.
> 	(output_lines): Support color output.

The gcov changes are ok.  I guess David has the review ball for the 
diagnostic refactoring?

nathan
David Malcolm Oct. 30, 2017, 2:49 p.m. UTC | #2
On Mon, 2017-10-30 at 08:17 -0400, Nathan Sidwell wrote:
> On 10/26/2017 04:11 AM, marxin wrote:
> > I consider using colors in context of gcov as very useful. There's
> > example for tramp3d:
> > https://pste.eu/p/Tl2D.html
> 
> nice!
> 
> > gcc/ChangeLog:
> > 
> > 2017-10-23  Martin Liska  <mliska@suse.cz>
> > 
> > 	* color-macros.h: New file.
> > 	* diagnostic-color.c: Factor out color related to macros to
> > 	color-macros.h.
> > 	* doc/gcov.texi: Document -k option.
> > 	* gcov.c (INCLUDE_STRING): Include string.h.
> > 	(print_usage): Add -k option.
> > 	(process_args): Parse it.
> > 	(pad_count_string): New function.
> > 	(output_line_beginning): Likewise.
> > 	(DEFAULT_LINE_START): New macro.
> > 	(output_lines): Support color output.
> 
> The gcov changes are ok.  I guess David has the review ball for the 
> diagnostic refactoring?
> 
> nathan

The comments beginning:
+/* Select Graphic Rendition (SGR, "\33[...m") strings.  */

and ending:
      It would be impractical for GCC to become a full-fledged
      terminal program linked against ncurses or the like, so it will
      not detect terminfo(5) capabilities.  */

are still in diagnostic-color.c after your patch, but they are
describing the macros, and in particular, if I'm reading them right,
are a rationale for why SGR_END contains a "\33[K".

Hence I think that those comments should also be moved to color-
macros.h.

Other than that the diagnostic changes mostly look good to me, but the
color-macros.h has:
+   Copyright (C) 2017 Free Software Foundation, Inc.

Shouldn't that copyright line express the full range of years for the
existing content that's being moved from diagnostic-color.c?

The macros there were introduced by the creation of diagnostic-color.c
in r197842 in April 2013 (aka dc604d41825b3cbd09045baeef09b1b88fc5a02),
which had the copyright line:

+   Copyright 2011-2013 Free Software Foundation, Inc.


FWIW, the macros seem to come from this patch by Manu:
"RFC: color diagnostics markers"
  https://gcc.gnu.org/ml/gcc-patches/2013-03/msg01365.html

where the code in question has:

/* Based on code from: */
+/* grep.c - main driver file for grep.
+   Copyright (C) 1992, 1997-2002, 2004-2013 Free Software Foundation,
Inc.

though as far as I can tell the only material taken directly from
"grep" are the comments I mentioned above beginning i.e. it appears to
me that the actual macros in the new file were written by Manu in 2013.

The comments mentioned above come from GNU grep; looking at the history
in grep's git repo shows it comes from commit
56623b5129d487cb6673fa5a582d094edc1fe983:

2005-06-20  Charles Levert  <charles_levert@gna.org>

       * src/grep.c: Extensively document the SGR/EL-to-Right issue.

Hence I believe the color-macros.h copyright line should range from 
2005-2017 if you move the comments (please do), or from 2013-2017 as-
is.

Diagnostic refactoring is OK otherwise.

Dave
Martin Liška Oct. 31, 2017, 11:12 a.m. UTC | #3
On 10/30/2017 03:49 PM, David Malcolm wrote:
> On Mon, 2017-10-30 at 08:17 -0400, Nathan Sidwell wrote:
>> On 10/26/2017 04:11 AM, marxin wrote:
>>> I consider using colors in context of gcov as very useful. There's
>>> example for tramp3d:
>>> https://pste.eu/p/Tl2D.html
>>
>> nice!
>>
>>> gcc/ChangeLog:
>>>
>>> 2017-10-23  Martin Liska  <mliska@suse.cz>
>>>
>>> 	* color-macros.h: New file.
>>> 	* diagnostic-color.c: Factor out color related to macros to
>>> 	color-macros.h.
>>> 	* doc/gcov.texi: Document -k option.
>>> 	* gcov.c (INCLUDE_STRING): Include string.h.
>>> 	(print_usage): Add -k option.
>>> 	(process_args): Parse it.
>>> 	(pad_count_string): New function.
>>> 	(output_line_beginning): Likewise.
>>> 	(DEFAULT_LINE_START): New macro.
>>> 	(output_lines): Support color output.
>>
>> The gcov changes are ok.  I guess David has the review ball for the
>> diagnostic refactoring?
>>
>> nathan
> 
> The comments beginning:
> +/* Select Graphic Rendition (SGR, "\33[...m") strings.  */
> 
> and ending:
>        It would be impractical for GCC to become a full-fledged
>        terminal program linked against ncurses or the like, so it will
>        not detect terminfo(5) capabilities.  */
> 
> are still in diagnostic-color.c after your patch, but they are
> describing the macros, and in particular, if I'm reading them right,
> are a rationale for why SGR_END contains a "\33[K".
> 
> Hence I think that those comments should also be moved to color-
> macros.h.

Hi.

Will do that.

> 
> Other than that the diagnostic changes mostly look good to me, but the
> color-macros.h has:
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> 
> Shouldn't that copyright line express the full range of years for the
> existing content that's being moved from diagnostic-color.c?
> 
> The macros there were introduced by the creation of diagnostic-color.c
> in r197842 in April 2013 (aka dc604d41825b3cbd09045baeef09b1b88fc5a02),
> which had the copyright line:
> 
> +   Copyright 2011-2013 Free Software Foundation, Inc.
> 
> 
> FWIW, the macros seem to come from this patch by Manu:
> "RFC: color diagnostics markers"
>    https://gcc.gnu.org/ml/gcc-patches/2013-03/msg01365.html
> 
> where the code in question has:
> 
> /* Based on code from: */
> +/* grep.c - main driver file for grep.
> +   Copyright (C) 1992, 1997-2002, 2004-2013 Free Software Foundation,
> Inc.
> 
> though as far as I can tell the only material taken directly from
> "grep" are the comments I mentioned above beginning i.e. it appears to
> me that the actual macros in the new file were written by Manu in 2013.
> 
> The comments mentioned above come from GNU grep; looking at the history
> in grep's git repo shows it comes from commit
> 56623b5129d487cb6673fa5a582d094edc1fe983:
> 
> 2005-06-20  Charles Levert  <charles_levert@gna.org>
> 
>         * src/grep.c: Extensively document the SGR/EL-to-Right issue.
> 
> Hence I believe the color-macros.h copyright line should range from
> 2005-2017 if you move the comments (please do), or from 2013-2017 as-
> is.

Thanks for clarification, 2005-2017 will be in final version of patch
I'm planning to install.

Martin

> 
> Diagnostic refactoring is OK otherwise.
> 
> Dave
>
diff mbox series

Patch

diff --git a/gcc/color-macros.h b/gcc/color-macros.h
new file mode 100644
index 00000000000..b23cae49df7
--- /dev/null
+++ b/gcc/color-macros.h
@@ -0,0 +1,50 @@ 
+/* Terminal color manipulation macros.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_COLOR_MACROS_H
+#define GCC_COLOR_MACROS_H
+
+#define COLOR_SEPARATOR		";"
+#define COLOR_NONE		"00"
+#define COLOR_BOLD		"01"
+#define COLOR_UNDERSCORE	"04"
+#define COLOR_BLINK		"05"
+#define COLOR_REVERSE		"07"
+#define COLOR_FG_BLACK		"30"
+#define COLOR_FG_RED		"31"
+#define COLOR_FG_GREEN		"32"
+#define COLOR_FG_YELLOW		"33"
+#define COLOR_FG_BLUE		"34"
+#define COLOR_FG_MAGENTA	"35"
+#define COLOR_FG_CYAN		"36"
+#define COLOR_FG_WHITE		"37"
+#define COLOR_BG_BLACK		"40"
+#define COLOR_BG_RED		"41"
+#define COLOR_BG_GREEN		"42"
+#define COLOR_BG_YELLOW		"43"
+#define COLOR_BG_BLUE		"44"
+#define COLOR_BG_MAGENTA	"45"
+#define COLOR_BG_CYAN		"46"
+#define COLOR_BG_WHITE		"47"
+#define SGR_START		"\33["
+#define SGR_END			"m\33[K"
+#define SGR_SEQ(str)		SGR_START str SGR_END
+#define SGR_RESET		SGR_SEQ("")
+
+#endif  /* GCC_COLOR_MACROS_H */
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index b8cf6f2c045..0ee4bb287e0 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -81,32 +81,7 @@ 
       It would be impractical for GCC to become a full-fledged
       terminal program linked against ncurses or the like, so it will
       not detect terminfo(5) capabilities.  */
-#define COLOR_SEPARATOR		";"
-#define COLOR_NONE		"00"
-#define COLOR_BOLD		"01"
-#define COLOR_UNDERSCORE	"04"
-#define COLOR_BLINK		"05"
-#define COLOR_REVERSE		"07"
-#define COLOR_FG_BLACK		"30"
-#define COLOR_FG_RED		"31"
-#define COLOR_FG_GREEN		"32"
-#define COLOR_FG_YELLOW		"33"
-#define COLOR_FG_BLUE		"34"
-#define COLOR_FG_MAGENTA	"35"
-#define COLOR_FG_CYAN		"36"
-#define COLOR_FG_WHITE		"37"
-#define COLOR_BG_BLACK		"40"
-#define COLOR_BG_RED		"41"
-#define COLOR_BG_GREEN		"42"
-#define COLOR_BG_YELLOW		"43"
-#define COLOR_BG_BLUE		"44"
-#define COLOR_BG_MAGENTA	"45"
-#define COLOR_BG_CYAN		"46"
-#define COLOR_BG_WHITE		"47"
-#define SGR_START		"\33["
-#define SGR_END			"m\33[K"
-#define SGR_SEQ(str)		SGR_START str SGR_END
-#define SGR_RESET		SGR_SEQ("")
+#include "color-macros.h"
 
 
 /* The context and logic for choosing default --color screen attributes
diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index c527b89f13b..2aa7166e35d 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -125,6 +125,7 @@  gcov [@option{-v}|@option{--version}] [@option{-h}|@option{--help}]
      [@option{-d}|@option{--display-progress}]
      [@option{-f}|@option{--function-summaries}]
      [@option{-i}|@option{--intermediate-format}]
+     [@option{-k}|@option{--use-colors}]
      [@option{-l}|@option{--long-file-names}]
      [@option{-m}|@option{--demangled-names}]
      [@option{-n}|@option{--no-output}]
@@ -215,6 +216,14 @@  lcount:26,1
 branch:28,nottaken
 @end smallexample
 
+@item -k
+@itemx --use-colors
+
+Use colors for lines of code that have zero coverage.  We use red color for
+non-exceptional lines and cyan for exceptional.  Same colors are used for
+basic blocks with @option{-a} option.
+
+
 @item -l
 @itemx --long-file-names
 Create long file names for included source files.  For example, if the
diff --git a/gcc/gcov.c b/gcc/gcov.c
index c56bac20278..e53bcf0fd88 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -33,6 +33,7 @@  along with Gcov; see the file COPYING3.  If not see
 #include "config.h"
 #define INCLUDE_ALGORITHM
 #define INCLUDE_VECTOR
+#define INCLUDE_STRING
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
@@ -40,6 +41,7 @@  along with Gcov; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "version.h"
 #include "demangle.h"
+#include "color-macros.h"
 
 #include <getopt.h>
 
@@ -381,6 +383,10 @@  static int flag_hash_filenames = 0;
 
 static int flag_verbose = 0;
 
+/* Print colored output.  */
+
+static int flag_use_colors = 0;
+
 /* Output count information for every basic block, not merely those
    that contain line number information.  */
 
@@ -703,6 +709,7 @@  print_usage (int error_p)
   fnotice (file, "  -f, --function-summaries        Output summaries for each function\n");
   fnotice (file, "  -h, --help                      Print this help, then exit\n");
   fnotice (file, "  -i, --intermediate-format       Output .gcov file in intermediate text format\n");
+  fnotice (file, "  -k, --use-colors                Emit colored output\n");
   fnotice (file, "  -l, --long-file-names           Use long output file names for included\n\
                                     source files\n");
   fnotice (file, "  -m, --demangled-names           Output demangled function names\n");
@@ -756,6 +763,7 @@  static const struct option options[] =
   { "unconditional-branches", no_argument,     NULL, 'u' },
   { "display-progress",     no_argument,       NULL, 'd' },
   { "hash-filenames",	    no_argument,       NULL, 'x' },
+  { "use-colors",	    no_argument,       NULL, 'k' },
   { 0, 0, 0, 0 }
 };
 
@@ -766,7 +774,7 @@  process_args (int argc, char **argv)
 {
   int opt;
 
-  const char *opts = "abcdfhilmno:prs:uvwx";
+  const char *opts = "abcdfhiklmno:prs:uvwx";
   while ((opt = getopt_long (argc, argv, opts, options, NULL)) != -1)
     {
       switch (opt)
@@ -789,6 +797,9 @@  process_args (int argc, char **argv)
 	case 'l':
 	  flag_long_names = 1;
 	  break;
+	case 'k':
+	  flag_use_colors = 1;
+	  break;
 	case 'm':
 	  flag_demangled_names = 1;
 	  break;
@@ -2468,6 +2479,65 @@  read_line (FILE *file)
   return pos ? string : NULL;
 }
 
+/* Pad string S with spaces from left to have total width equal to 9.  */
+
+static void
+pad_count_string (string &s)
+{
+  if (s.size () < 9)
+    s.insert (0, 9 - s.size (), ' ');
+}
+
+/* Print GCOV line beginning to F stream.  If EXISTS is set to true, the
+   line exists in source file.  UNEXCEPTIONAL indicated that it's not in
+   an exceptional statement.  The output is printed for LINE_NUM of given
+   COUNT of executions.  EXCEPTIONAL_STRING and UNEXCEPTIONAL_STRING are
+   used to indicate non-executed blocks.  */
+
+static void
+output_line_beginning (FILE *f, bool exists, bool unexceptional,
+		       gcov_type count, unsigned line_num,
+		       const char *exceptional_string,
+		       const char *unexceptional_string)
+{
+  string s;
+  if (exists)
+    {
+      if (count > 0)
+	{
+	  s = format_gcov (count, 0, -1);
+	  pad_count_string (s);
+	}
+      else
+	{
+	  if (flag_use_colors)
+	    {
+	      s = "0";
+	      pad_count_string (s);
+	      if (unexceptional)
+		s.insert (0, SGR_SEQ (COLOR_BG_RED
+				      COLOR_SEPARATOR COLOR_FG_WHITE));
+	      else
+		s.insert (0, SGR_SEQ (COLOR_BG_CYAN
+				      COLOR_SEPARATOR COLOR_FG_WHITE));
+	      s += SGR_RESET;
+	    }
+	  else
+	    {
+	      s = unexceptional ? unexceptional_string : exceptional_string;
+	      pad_count_string (s);
+	    }
+	}
+    }
+  else
+    {
+      s = "-";
+      pad_count_string (s);
+    }
+
+  fprintf (f, "%s:%5u", s.c_str (), line_num);
+}
+
 /* Read in the source file one line at a time, and output that line to
    the gcov file preceded by its execution count and other
    information.  */
@@ -2475,21 +2545,23 @@  read_line (FILE *file)
 static void
 output_lines (FILE *gcov_file, const source_t *src)
 {
+#define  DEFAULT_LINE_START "        -:    0:"
+
   FILE *source_file;
   unsigned line_num;	/* current line number.  */
   const line_t *line;           /* current line info ptr.  */
   const char *retval = "";	/* status of source file reading.  */
   function_t *fn = NULL;
 
-  fprintf (gcov_file, "%9s:%5d:Source:%s\n", "-", 0, src->coverage.name);
+  fprintf (gcov_file, DEFAULT_LINE_START "Source:%s\n", src->coverage.name);
   if (!multiple_files)
     {
-      fprintf (gcov_file, "%9s:%5d:Graph:%s\n", "-", 0, bbg_file_name);
-      fprintf (gcov_file, "%9s:%5d:Data:%s\n", "-", 0,
+      fprintf (gcov_file, DEFAULT_LINE_START "Graph:%s\n", bbg_file_name);
+      fprintf (gcov_file, DEFAULT_LINE_START "Data:%s\n",
 	       no_data_file ? "-" : da_file_name);
-      fprintf (gcov_file, "%9s:%5d:Runs:%u\n", "-", 0, object_runs);
+      fprintf (gcov_file, DEFAULT_LINE_START "Runs:%u\n", object_runs);
     }
-  fprintf (gcov_file, "%9s:%5d:Programs:%u\n", "-", 0, program_count);
+  fprintf (gcov_file, DEFAULT_LINE_START "Programs:%u\n", program_count);
 
   source_file = fopen (src->name, "r");
   if (!source_file)
@@ -2498,7 +2570,7 @@  output_lines (FILE *gcov_file, const source_t *src)
       retval = NULL;
     }
   else if (src->file_time == 0)
-    fprintf (gcov_file, "%9s:%5d:Source is newer than graph\n", "-", 0);
+    fprintf (gcov_file, DEFAULT_LINE_START "Source is newer than graph\n");
 
   if (flag_branches)
     fn = src->functions;
@@ -2537,11 +2609,10 @@  output_lines (FILE *gcov_file, const source_t *src)
 	 Otherwise, print the execution count before the source line.
 	 There are 16 spaces of indentation added before the source
 	 line so that tabs won't be messed up.  */
-      fprintf (gcov_file, "%9s:%5u:%s\n",
-	       !line->exists ? "-" : line->count
-	       ? format_gcov (line->count, 0, -1)
-	       : line->unexceptional ? "#####" : "=====", line_num,
-	       retval ? retval : "/*EOF*/");
+      output_line_beginning (gcov_file, line->exists, line->unexceptional,
+			     line->count, line_num,
+			     "=====", "#####");
+      fprintf (gcov_file, ":%s\n", retval ? retval : "/*EOF*/");
 
       if (flag_all_blocks)
 	{
@@ -2554,11 +2625,11 @@  output_lines (FILE *gcov_file, const source_t *src)
 	    {
 	      if (!block->is_call_return)
 		{
-		  fprintf (gcov_file, "%9s:%5u-block %2d",
-			   !line->exists ? "-" : block->count
-			   ? format_gcov (block->count, 0, -1)
-			   : block->exceptional ? "%%%%%" : "$$$$$",
-			   line_num, ix++);
+		  output_line_beginning (gcov_file, line->exists,
+					 block->exceptional,
+					 block->count, line_num,
+					 "%%%%%", "$$$$$");
+		  fprintf (gcov_file, "-block %2d", ix++);
 		  if (flag_verbose)
 		    fprintf (gcov_file, " (BB %u)", block->id);
 		  fprintf (gcov_file, "\n");