This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

patch: enable -fshow-column by default


> I agree in principle that we should enable -fshow-column by default.
> Obviously, it will break some existing scripts, but I tend to think that
> for something like this that is OK.  Any scripts which break are already
> slightly broken in that preprocessor error messages already display a
> column number.

Well, here it is, so we all know what we're talking about.

This patch enables -fshow-column by default.  

Janis has been kind enough to fix the testsuite harness to handle
columns.  I'll let her do the explaining, but basically the regexp
matching the line in the error message is deep within dejagnu itself,
and can get confused if an error column number is the same as an error
column.  To avoid this ambiguity, short of changing all tests, the
harness patch expects all error messages to have a column.  If an error
does not have a column, a 0 should be used in the dg-{message,bogus,etc}
for the test (see patch for examples in the testsuite).

The rest of the patch are changes to the compiler to provide columns
where none were being printed.

The harness changes are Janis'.  The rest is my fault.

Bootstrapped and tested on x86-64 Linux.  The only regressions with this
patch are the libcpp errors which pass -fno-show-column, since the
harness expects errors to have a column.  I'll coordinate something with
Papa Tom next.

How does this look?
Aldy

--------------

gcc/
	* diagnostic.c (diagnostic_build_prefix): Always print columns.
	(diagnostic_report_current_module): Print columns.
	* common.opt (flag_show_column): Enable by default.
gcc/testsuite/
	* lib/gcc-dg.exp (dg-bogus): Override dg-bogus.
	(process-message): Expect column numbers.
	* gcc.dg/va-arg-2.c: Use line 0 to indicate no column.
	* gcc.dg/pch/counter-2.c: Same.
	* gcc.dg/pch/valid-2.c: Same.
	* gcc.dg/pch/warn-1.c: Same.
	* gcc.dg/pch/valid-1.c: Same.
gcc/cp/
	* error.c (print_instantiation_partial_context): Print column
	numbers.
libcpp/
	* include/line-map.h (LAST_SOURCE_COLUMN): New.

Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c	(revision 147662)
+++ gcc/diagnostic.c	(working copy)
@@ -162,7 +162,7 @@ diagnostic_build_prefix (diagnostic_info
   return
     (s.file == NULL
      ? build_message_string ("%s: %s", progname, text)
-     : flag_show_column && s.column != 0
+     : flag_show_column
      ? build_message_string ("%s:%d:%d: %s", s.file, s.line, s.column, text)
      : build_message_string ("%s:%d: %s", s.file, s.line, text));
 }
@@ -244,9 +244,15 @@ diagnostic_report_current_module (diagno
       if (! MAIN_FILE_P (map))
 	{
 	  map = INCLUDED_FROM (line_table, map);
-	  pp_verbatim (context->printer,
-		       "In file included from %s:%d",
-		       map->to_file, LAST_SOURCE_LINE (map));
+	  if (flag_show_column)
+	    pp_verbatim (context->printer,
+			 "In file included from %s:%d:%d",
+			 map->to_file,
+			 LAST_SOURCE_LINE (map), LAST_SOURCE_COLUMN (map));
+	  else
+	    pp_verbatim (context->printer,
+			 "In file included from %s:%d",
+			 map->to_file, LAST_SOURCE_LINE (map));
 	  while (! MAIN_FILE_P (map))
 	    {
 	      map = INCLUDED_FROM (line_table, map);
Index: gcc/testsuite/gcc.dg/va-arg-2.c
===================================================================
--- gcc/testsuite/gcc.dg/va-arg-2.c	(revision 147662)
+++ gcc/testsuite/gcc.dg/va-arg-2.c	(working copy)
@@ -5,7 +5,7 @@
 
 #include <varargs.h>  /* { dg-bogus "varargs.h" "missing file" } */
 
-/* { dg-message "" "In file included from" { target *-*-* } 6 } */
+/* { dg-message "file included from" "file included from" { target *-*-* } 0 } */
 /* { dg-error "no longer implements" "#error 1" { target *-*-* } 4 } */
 /* { dg-error "Revise your code" "#error 2" { target *-*-* } 5 } */
 
Index: gcc/testsuite/gcc.dg/pch/counter-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pch/counter-2.c	(revision 147662)
+++ gcc/testsuite/gcc.dg/pch/counter-2.c	(working copy)
@@ -8,7 +8,7 @@
 #endif
 
 #include "counter-2.h" /* { dg-warning "not used because `__COUNTER__' is invalid" } */
-/* { dg-error "counter-2.h: No such file or directory" "no such file" { target *-*-* } 10 } */
+/* { dg-error "counter-2.h: No such file or directory" "no such file" { target *-*-* } 0 } */
 /* { dg-error "one or more PCH files were found, but they were invalid" "invalid files" { target *-*-* } 10 } */
 /* { dg-message "terminated" "" { target *-*-* } 0 } */
 
Index: gcc/testsuite/gcc.dg/pch/valid-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pch/valid-2.c	(revision 147662)
+++ gcc/testsuite/gcc.dg/pch/valid-2.c	(working copy)
@@ -1,7 +1,7 @@
 /* { dg-options "-I. -Winvalid-pch -fexceptions" } */
 
 #include "valid-2.h" /* { dg-warning "settings for -fexceptions do not match" } */
-/* { dg-error "No such file" "no such file" { target *-*-* } 3 } */
-/* { dg-error "they were invalid" "invalid files" { target *-*-* } 3 } */
+/* { dg-error "No such file" "no such file" { target *-*-* } 0 } */
+/* { dg-error "they were invalid" "invalid files" { target *-*-* } 0 } */
 /* { dg-message "terminated" "" { target *-*-* } 0 } */
 int x;
Index: gcc/testsuite/gcc.dg/pch/warn-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pch/warn-1.c	(revision 147662)
+++ gcc/testsuite/gcc.dg/pch/warn-1.c	(working copy)
@@ -3,8 +3,8 @@
 #define DEFINED_VALUE 3
 
 #include "warn-1.h"/* { dg-warning "not used because .DEFINED_VALUE. is defined" } */
-/* { dg-error "No such file" "no such file" { target *-*-* } 5 } */
-/* { dg-error "they were invalid" "invalid files" { target *-*-* } 5 } */
+/* { dg-error "No such file" "no such file" { target *-*-* } 0 } */
+/* { dg-error "they were invalid" "invalid files" { target *-*-* } 0 } */
 /* { dg-message "terminated" "" { target *-*-* } 0 } */
 
 
Index: gcc/testsuite/gcc.dg/pch/valid-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pch/valid-1.c	(revision 147662)
+++ gcc/testsuite/gcc.dg/pch/valid-1.c	(working copy)
@@ -1,8 +1,8 @@
 /* { dg-options "-I. -Winvalid-pch -g" } */
 
 #include "valid-1.h"/* { dg-warning "created with -gnone, but used with -g" } */
-/* { dg-error "No such file" "no such file" { target *-*-* } 3 } */
-/* { dg-error "they were invalid" "invalid files" { target *-*-* } 3 } */
+/* { dg-error "No such file" "no such file" { target *-*-* } 0 } */
+/* { dg-error "they were invalid" "invalid files" { target *-*-* } 0 } */
 /* { dg-message "terminated" "" { target *-*-* } 0 } */
 
 int x;
Index: gcc/testsuite/lib/gcc-dg.exp
===================================================================
--- gcc/testsuite/lib/gcc-dg.exp	(revision 147662)
+++ gcc/testsuite/lib/gcc-dg.exp	(working copy)
@@ -620,6 +620,17 @@ if { [info procs saved-dg-error] == [lis
 
 	process-message saved-dg-error "$gcc_error_prefix" "$args"
     }
+
+    # Override dg-bogus at the same time.  It doesn't handle a prefix
+    # but its expression should include a column number.  Otherwise the
+    # line number can match the column number for other messages, leading
+    # to insanity.
+    rename dg-bogus saved-dg-bogus
+
+    proc dg-bogus { args } {
+	upvar dg-messages dg-messages
+	process-message saved-dg-bogus "" $args
+    }
 }
 
 # Modify the regular expression saved by a DejaGnu message directive to
@@ -641,20 +652,26 @@ proc process-message { msgproc msgprefix
 	return;
     }
 
-    # Prepend the message prefix to the regular expression and make
-    # it match a single line.
+    # Get the entry for the new message.  Prepend the message prefix to
+    # the regular expression and make it match a single line.
     set newentry [lindex ${dg-messages} end]
     set expmsg [lindex $newentry 2]
 
-    # If we have a column...
+    # Handle column numbers from the specified expression (if there is
+    # one) and set up the search expression that will be used by DejaGnu.
     if [regexp "^(\[0-9\]+):" $expmsg "" column] {
-	# Remove "COLUMN:"
+	# The expression in the directive included a column number.
+	# Remove "COLUMN:" from the original expression and move it
+	# to the proper place in the search expression.
 	regsub "^\[0-9\]+:" $expmsg "" expmsg
-
-	# Include the column in the search expression.
-	set expmsg "$column: $msgprefix\[^\n]*$expmsg"
+	set expmsg "$column: $msgprefix\[^\n\]*$expmsg"
+    } elseif [string match "" [lindex $newentry 0]] {
+	# The specified line number is 0; don't expect a column number.
+	set expmsg "$msgprefix\[^\n\]*$expmsg"
     } else {
-	set expmsg "$msgprefix\[^\n]*$expmsg"
+	# There is no column number in the search expression, but we
+	# should expect one in the message itself.
+	set expmsg "\[0-9\]+: $msgprefix\[^\n\]*$expmsg"
     }
 
     set newentry [lreplace $newentry 2 2 $expmsg]
Index: gcc/cp/error.c
===================================================================
--- gcc/cp/error.c	(revision 147662)
+++ gcc/cp/error.c	(working copy)
@@ -2705,19 +2705,29 @@ print_instantiation_partial_context (dia
 				     struct tinst_level *t, location_t loc)
 {
   expanded_location xloc;
+  const char *str;
   for (; ; t = t->next)
     {
       xloc = expand_location (loc);
       if (t == NULL)
 	break;
-      pp_verbatim (context->printer, _("%s:%d:   instantiated from %qs\n"),
-		   xloc.file, xloc.line,
-		   decl_as_string_translate (t->decl,
-					     TFF_DECL_SPECIFIERS | TFF_RETURN_TYPE));
+      str = decl_as_string (t->decl, TFF_DECL_SPECIFIERS | TFF_RETURN_TYPE);
+      if (flag_show_column)
+	pp_verbatim (context->printer,
+		     _("%s:%d:%d:   instantiated from %qs\n"),
+		     xloc.file, xloc.line, xloc.column, str);
+      else
+	pp_verbatim (context->printer,
+		     _("%s:%d:   instantiated from %qs\n"),
+		     xloc.file, xloc.line, str);
       loc = t->locus;
     }
-  pp_verbatim (context->printer, _("%s:%d:   instantiated from here"),
-	       xloc.file, xloc.line);
+  if (flag_show_column)
+    pp_verbatim (context->printer, _("%s:%d:%d:   instantiated from here"),
+		 xloc.file, xloc.line, xloc.column);
+  else
+    pp_verbatim (context->printer, _("%s:%d:   instantiated from here"),
+		 xloc.file, xloc.line);
   pp_base_newline (context->printer);
 }
 
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 147662)
+++ gcc/common.opt	(working copy)
@@ -1049,8 +1049,8 @@ Common Report Var(flag_see) Init(0)
 Eliminate redundant sign extensions using LCM.
 
 fshow-column
-Common C ObjC C++ ObjC++ Report Var(flag_show_column) Init(0)
-Show column numbers in diagnostics, when available.  Default off
+Common C ObjC C++ ObjC++ Report Var(flag_show_column) Init(1)
+Show column numbers in diagnostics, when available.  Default on
 
 fsignaling-nans
 Common Report Var(flag_signaling_nans) Optimization
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h	(revision 147662)
+++ libcpp/include/line-map.h	(working copy)
@@ -154,6 +154,8 @@ extern const struct line_map *linemap_lo
    of the #include, or other directive, that caused a map change.  */
 #define LAST_SOURCE_LINE(MAP) \
   SOURCE_LINE (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
+#define LAST_SOURCE_COLUMN(MAP) \
+  SOURCE_COLUMN (MAP, LAST_SOURCE_LINE_LOCATION (MAP))
 #define LAST_SOURCE_LINE_LOCATION(MAP) \
   ((((MAP)[1].start_location - 1 - (MAP)->start_location) \
     & ~((1 << (MAP)->column_bits) - 1))			  \


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]