implement fdiagnostics-print-source-range-info

[Manuel López-Ibáñez <lopezibanez at gmail dot com>]

The following patch implements printing range info in diagnostics. It
is a very rough initial version but comments are appreciated. With
this patch for this code:

void bar(void)
{
  _Complex float Gamma;
  int *P;
  P = (P-42) + Gamma*4;
}

we print:

test.c: In function ‘bar’:
test.c:11:14:11.9-14:11.14-21: error: invalid operands to binary +
(have ‘int *’ and ‘complex float’)

which is not as good as Clang prints because we do not track the
boundaries of expressions. On the other hand, the above follows GNU
coding conventions [*] while Clang doesn't.

[*] http://www.gnu.org/prep/standards/standards.html#Errors

Bootstrapped and regression tested on x86_64-linux-gnu with enable-languages=all

Comments?

2009-08-04  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	* doc/invoke.texi (fdiagnostics-print-source-range-info): New.
	* diagnostic.c (diagnostic_add_range): New.
	(diagnostic_set_info_translated): Init num_ranges.
	(diagnostic_build_prefix): Handle ranges.
	(warning_at2,warning_at3,error_at2,error_at3): New.
	* diagnostic.h (diagnostic_info): Handle ranges.
	* input.h (UNKNOWN_LOCATION_RANGE): Declare.
	(location_t): Move typedef to line-map.h.
	* toplev.c (UNKNOWN_LOCATION_RANGE): Define.
	* toplev.h (warning_at2,warning_at3,error_at2,error_at3): Declare.
	* c-typeck.c (build_binary_op): Track ranges.
	* common.opt (fdiagnostics-print-source-range-info): New.
	* c-common.c (binary_op_error): Rename as
	binary_op_error_ranges. Handle ranges.
	* c-common.h (binary_op_error): New macro.
libcpp/
	* include/line-map.h (location_t): Define here.
	(struct location_range): New.
	(location_range_t): New.

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 150311)
+++ gcc/doc/invoke.texi	(working copy)
@@ -220,10 +220,11 @@ Objective-C and Objective-C++ Dialects}.
 -Wundeclared-selector}
 
 @item Language Independent Options
 @xref{Language Independent Options,,Options to Control Diagnostic Messages Formatting}.
 @gccoptlist{-fmessage-length=@var{n}  @gol
+-fdiagnostics-print-source-range-info @gol
 -fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]}  @gol
 -fdiagnostics-show-option}
 
 @item Warning Options
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
@@ -2625,10 +2626,15 @@ Try to format error messages so that the
 characters.  The default is 72 characters for @command{g++} and 0 for the rest of
 the front ends supported by GCC@.  If @var{n} is zero, then no
 line-wrapping will be done; each error message will appear on a single
 line.
 
+@opindex fdiagnostics-print-source-range-info
+@item -fdiagnostics-print-source-range-info
+Show range information after location information (file:line:column)
+in some diagnostic messages. Default is off.
+
 @opindex fdiagnostics-show-location
 @item -fdiagnostics-show-location=once
 Only meaningful in line-wrapping mode.  Instructs the diagnostic messages
 reporter to emit @emph{once} source location information; that is, in
 case the message is too long to fit on a single physical line and has to
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c	(revision 150311)
+++ gcc/diagnostic.c	(working copy)
@@ -114,10 +114,19 @@ diagnostic_initialize (diagnostic_contex
   context->last_module = 0;
   context->last_function = NULL;
   context->lock = 0;
 }
 
+static void
+diagnostic_add_range (diagnostic_info *diagnostic,
+		      location_range_t range)
+{
+  gcc_assert (diagnostic->num_ranges < 2);
+  diagnostic->range[diagnostic->num_ranges] = range;
+  diagnostic->num_ranges++;
+}
+
 /* Initialize DIAGNOSTIC, where the message MSG has already been
    translated.  */
 void
 diagnostic_set_info_translated (diagnostic_info *diagnostic, const char *msg,
 				va_list *args, location_t location,
@@ -128,10 +137,11 @@ diagnostic_set_info_translated (diagnost
   diagnostic->message.format_spec = msg;
   diagnostic->location = location;
   diagnostic->override_column = 0;
   diagnostic->kind = kind;
   diagnostic->option_index = 0;
+  diagnostic->num_ranges = 0;
 }
 
 /* Initialize DIAGNOSTIC, where the message GMSGID has not yet been
    translated.  */
 void
@@ -145,22 +155,71 @@ diagnostic_set_info (diagnostic_info *di
 /* Return a malloc'd string describing a location.  The caller is
    responsible for freeing the memory.  */
 char *
 diagnostic_build_prefix (diagnostic_info *diagnostic)
 {
+  
   static const char *const diagnostic_kind_text[] = {
 #define DEFINE_DIAGNOSTIC_KIND(K, T) (T),
 #include "diagnostic.def"
 #undef DEFINE_DIAGNOSTIC_KIND
     "must-not-happen"
   };
   const char *text = _(diagnostic_kind_text[diagnostic->kind]);
+  int num_ranges = diagnostic->num_ranges;
   expanded_location s = expand_location (diagnostic->location);
   if (diagnostic->override_column)
     s.column = diagnostic->override_column;
   gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND);
 
+  if (flag_show_ranges && num_ranges)
+    {
+      int i;
+      char * rangestr = NULL;
+      char * tmpstr;
+      
+      asprintf(&tmpstr, "%s", "");
+
+      for (i = 0; i < num_ranges; i++)
+	{
+	  expanded_location range_begin =
+	    expand_location (diagnostic->range[i].begin);
+	  expanded_location range_end =
+	    expand_location (diagnostic->range[i].end);
+
+	  if (range_begin.file != s.file || range_end.file != s.file)
+	    continue;
+
+	  if (rangestr)
+	    tmpstr = rangestr;
+
+	  if (range_begin.line == range_end.line)
+	    rangestr = build_message_string ("%s%d.%d-%d:",
+					     tmpstr,
+					     range_end.line,
+					     range_begin.column,
+					     range_end.column);
+	  else 
+	    rangestr = build_message_string ("%s%d.%d-%d.%d:",
+					     tmpstr,
+					     range_begin.line,
+					     range_begin.column,
+					     range_end.line,
+					     range_end.column);
+	  free (tmpstr);
+	}
+
+      if (!rangestr)
+	rangestr = tmpstr;
+
+      gcc_assert (s.file != NULL);
+      tmpstr = build_message_string ("%s:%d:%d:%s %s", s.file, s.line, s.column,
+				     rangestr, text);
+      free (rangestr);
+      return tmpstr;
+    }
+
   return
     (s.file == NULL
      ? build_message_string ("%s: %s", progname, text)
      : flag_show_column
      ? build_message_string ("%s:%d:%d: %s", s.file, s.line, s.column, text)
@@ -553,10 +612,46 @@ warning_at (location_t location, int opt
   diagnostic.option_index = opt;
   va_end (ap);
   return report_diagnostic (&diagnostic);
 }
 
+/* A warning at LOCATION.  Use this for code which is correct according to the
+   relevant language specification but is likely to be buggy anyway.
+   Returns true if the warning was printed, false if it was inhibited.  */
+
+bool
+warning_at2 (location_t location, location_range_t range,
+	     int opt, const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING);
+  diagnostic_add_range (&diagnostic, range);
+  diagnostic.option_index = opt;
+  va_end (ap);
+  return report_diagnostic (&diagnostic);
+}
+
+bool
+warning_at3 (location_t location, 
+	     location_range_t range1, location_range_t range2,
+	     int opt, const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_WARNING);
+  diagnostic_add_range (&diagnostic, range1);
+  diagnostic_add_range (&diagnostic, range2);
+  diagnostic.option_index = opt;
+  va_end (ap);
+  return report_diagnostic (&diagnostic);
+}
+
 /* A "pedantic" warning at LOCATION: issues a warning unless
    -pedantic-errors was given on the command line, in which case it
    issues an error.  Use this for diagnostics required by the relevant
    language standard, if you have chosen not to make them errors.
 
@@ -627,10 +722,38 @@ error_at (location_t loc, const char *gm
   diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR);
   report_diagnostic (&diagnostic);
   va_end (ap);
 }
 
+void
+error_at2 (location_t loc, location_range_t range, const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR);
+  diagnostic_add_range (&diagnostic, range);
+  report_diagnostic (&diagnostic);
+  va_end (ap);
+}
+
+void
+error_at3 (location_t loc, location_range_t range1, location_range_t range2,
+	   const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, loc, DK_ERROR);
+  diagnostic_add_range (&diagnostic, range1);
+  diagnostic_add_range (&diagnostic, range2);
+  report_diagnostic (&diagnostic);
+  va_end (ap);
+}
+
 /* "Sorry, not implemented."  Use for a language feature which is
    required by the relevant specification but not implemented by GCC.
    An object file will not be produced.  */
 void
 sorry (const char *gmsgid, ...)
Index: gcc/diagnostic.h
===================================================================
--- gcc/diagnostic.h	(revision 150311)
+++ gcc/diagnostic.h	(working copy)
@@ -39,10 +39,15 @@ typedef enum
    list in diagnostic.def.  */
 typedef struct diagnostic_info
 {
   text_info message;
   location_t location;
+  /* Location ranges.
+     Currently we only support three ranges. */
+  int num_ranges;
+  location_range_t range[3];
+
   unsigned int override_column;
   /* TREE_BLOCK if the diagnostic is to be reported in some inline
      function inlined into other function, otherwise NULL.  */
   tree abstract_origin;
   /* The kind of diagnostic it is about.  */
Index: gcc/input.h
===================================================================
--- gcc/input.h	(revision 150311)
+++ gcc/input.h	(working copy)
@@ -30,10 +30,12 @@ extern GTY(()) struct line_maps *line_ta
 #define UNKNOWN_LOCATION ((source_location) 0)
 
 /* The location for declarations in "<built-in>" */
 #define BUILTINS_LOCATION ((source_location) 2)
 
+extern const location_range_t UNKNOWN_LOCATION_RANGE;
+
 typedef struct GTY (())
 {
   /* The name of the source file involved.  */
   const char *file;
 
@@ -46,14 +48,10 @@ typedef struct GTY (())
   bool sysp;
 } expanded_location;
 
 extern expanded_location expand_location (source_location);
 
-/* Historically GCC used location_t, while cpp used source_location.
-   This could be removed but it hardly seems worth the effort.  */
-typedef source_location location_t;
-
 /* Top-level source file.  */
 extern const char *main_input_filename;
 
 extern location_t input_location;
 
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(revision 150311)
+++ gcc/toplev.c	(working copy)
@@ -144,10 +144,12 @@ const char *main_input_filename;
 
 /* Current position in real source file.  */
 
 location_t input_location;
 
+const location_range_t UNKNOWN_LOCATION_RANGE = {UNKNOWN_LOCATION, UNKNOWN_LOCATION};
+
 struct line_maps *line_table;
 
 /* Name to use as base of names for dump output files.  */
 
 const char *dump_base_name;
Index: gcc/toplev.h
===================================================================
--- gcc/toplev.h	(revision 150311)
+++ gcc/toplev.h	(working copy)
@@ -59,12 +59,24 @@ extern void internal_error (const char *
      ATTRIBUTE_NORETURN;
 /* Pass one of the OPT_W* from options.h as the first parameter.  */
 extern bool warning (int, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+bool
+warning_at2 (location_t location, 
+	     location_range_t range,
+	     int opt, const char *gmsgid, ...)
+    ATTRIBUTE_GCC_DIAG(4,5);
+bool
+warning_at3 (location_t location, 
+	     location_range_t range1, location_range_t range2,
+	     int opt, const char *gmsgid, ...)
+    ATTRIBUTE_GCC_DIAG(5,6);
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
+extern void error_at2 (location_t, location_range_t, const char *, ...) ATTRIBUTE_GCC_DIAG(3,4);
+extern void error_at3 (location_t, location_range_t, location_range_t, const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
 extern void fatal_error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
      ATTRIBUTE_NORETURN;
 /* Pass one of the OPT_W* from options.h as the second parameter.  */
 extern bool pedwarn (location_t, int, const char *, ...) 
      ATTRIBUTE_GCC_DIAG(3,4);
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 150311)
+++ gcc/c-typeck.c	(working copy)
@@ -8787,10 +8787,12 @@ build_binary_op (location_t location, en
   tree op0, op1;
   tree ret = error_mark_node;
   const char *invalid_op_diag;
   bool op0_int_operands, op1_int_operands;
   bool int_const, int_const_or_overflow, int_operands;
+  location_range_t loc_range0, loc_range1;
+
 
   /* Expression code to give to the expression when it is built.
      Normally this is CODE, which is what the caller asked for,
      but in some special cases we change it.  */
   enum tree_code resultcode = code;
@@ -8844,10 +8846,15 @@ build_binary_op (location_t location, en
   bool may_need_excess_precision;
 
   if (location == UNKNOWN_LOCATION)
     location = input_location;
 
+  loc_range0.begin = EXPR_LOCATION (orig_op0);
+  loc_range0.end = location;
+  loc_range1.begin = location;
+  loc_range1.end = EXPR_LOCATION (orig_op1);
+
   op0 = orig_op0;
   op1 = orig_op1;
 
   op0_int_operands = EXPR_INT_CONST_OPERANDS (orig_op0);
   if (op0_int_operands)
@@ -9314,11 +9321,11 @@ build_binary_op (location_t location, en
   if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE
       && (!tree_int_cst_equal (TYPE_SIZE (type0), TYPE_SIZE (type1))
 	  || !same_scalar_type_ignoring_signedness (TREE_TYPE (type0),
 						    TREE_TYPE (type1))))
     {
-      binary_op_error (location, code, type0, type1);
+      binary_op_error_ranges (location, loc_range0, loc_range1, code, type0, type1);
       return error_mark_node;
     }
 
   if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE || code0 == COMPLEX_TYPE
        || code0 == FIXED_POINT_TYPE || code0 == VECTOR_TYPE)
@@ -9550,11 +9557,12 @@ build_binary_op (location_t location, en
      It will be given type FINAL_TYPE if that is nonzero;
      otherwise, it will be given type RESULT_TYPE.  */
 
   if (!result_type)
     {
-      binary_op_error (location, code, TREE_TYPE (op0), TREE_TYPE (op1));
+      binary_op_error_ranges (location, loc_range0, loc_range1, code,
+			      TREE_TYPE (op0), TREE_TYPE (op1));
       return error_mark_node;
     }
 
   if (!converted)
     {
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 150311)
+++ gcc/common.opt	(working copy)
@@ -444,10 +444,14 @@ Attempt to fill delay slots of branch in
 
 fdelete-null-pointer-checks
 Common Report Var(flag_delete_null_pointer_checks) Init(1) Optimization
 Delete useless null pointer checks
 
+fdiagnostics-print-source-range-info
+Common Report Var(flag_show_ranges) Init(1)
+Show range information in diagnostic messages, when available
+
 fdiagnostics-show-location=
 Common Joined RejectNegative
 -fdiagnostics-show-location=[once|every-line]	How often to emit source location at the beginning of line-wrapped diagnostics
 
 fdiagnostics-show-option
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 150311)
+++ gcc/c-common.c	(working copy)
@@ -3250,12 +3250,13 @@ c_register_builtin_type (tree type, cons
 /* Print an error message for invalid operands to arith operation
    CODE with TYPE0 for operand 0, and TYPE1 for operand 1.
    LOCATION is the location of the message.  */
 
 void
-binary_op_error (location_t location, enum tree_code code,
-		 tree type0, tree type1)
+binary_op_error_ranges (location_t location,
+			location_range_t range0, location_range_t range1,
+			enum tree_code code, tree type0, tree type1)
 {
   const char *opname;
 
   switch (code)
     {
@@ -3302,13 +3303,13 @@ binary_op_error (location_t location, en
     case BIT_XOR_EXPR:
       opname = "^"; break;
     default:
       gcc_unreachable ();
     }
-  error_at (location,
-	    "invalid operands to binary %s (have %qT and %qT)", opname,
-	    type0, type1);
+  error_at3 (location, range0, range1,
+	     "invalid operands to binary %s (have %qT and %qT)", opname,
+	     type0, type1);
 }
 
 /* Subroutine of build_binary_op, used for comparison operations.
    See if the operands have both been converted from subword integer types
    and, if so, perhaps change them both back to their original type.
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 150311)
+++ gcc/c-common.h	(working copy)
@@ -795,13 +795,19 @@ extern tree decl_constant_value_for_opti
 extern tree c_save_expr (tree);
 extern tree c_common_truthvalue_conversion (location_t, tree);
 extern void c_apply_type_quals_to_decl (int, tree);
 extern tree c_sizeof_or_alignof_type (location_t, tree, bool, int);
 extern tree c_alignof_expr (location_t, tree);
+
+#define binary_op_error(LOC,CODE,T1,T2)\
+        binary_op_error_ranges (LOC, UNKNOWN_LOCATION_RANGE,\
+                               UNKNOWN_LOCATION_RANGE, CODE, T1, T2)
 /* Print an error message for invalid operands to arith operation CODE.
    NOP_EXPR is used as a special case (see truthvalue_conversion).  */
-extern void binary_op_error (location_t, enum tree_code, tree, tree);
+extern void binary_op_error_ranges (location_t,
+				    location_range_t, location_range_t,
+				    enum tree_code, tree, tree);
 extern tree fix_string_type (tree);
 struct varray_head_tag;
 extern void constant_expression_warning (tree);
 extern void constant_expression_error (tree);
 extern bool strict_aliasing_warning (tree, tree, tree);
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h	(revision 150311)
+++ libcpp/include/line-map.h	(working copy)
@@ -37,13 +37,22 @@ enum lc_reason {LC_ENTER = 0, LC_LEAVE, 
 
 /* The type of line numbers.  */
 typedef unsigned int linenum_type;
 
 /* A logical line/column number, i.e. an "index" into a line_map.  */
-/* Long-term, we want to use this to replace struct location_s (in input.h),
-   and effectively typedef source_location location_t.  */
 typedef unsigned int source_location;
+/* Historically GCC used location_t, while cpp used source_location.
+   This could be removed but it hardly seems worth the effort.  */
+typedef source_location location_t;
+
+/* A range of locations.  */
+struct location_range {
+  location_t begin;
+  location_t end;
+};
+
+typedef struct location_range location_range_t;
 
 /* Memory allocation function typedef.  Works like xrealloc.  */
 typedef void *(*line_map_realloc) (void *, size_t);
 
 /* Physical source file TO_FILE at line TO_LINE at column 0 is represented