Strip "<built-in>" loc from displayed expansion context

Now that diagnostics for tokens coming from macro expansions point to
the spelling location of the relevant token (and then displays the
context of the expansion), some ugly (not so seldom) corner cases can
happen.

When the relevant token is a built-in token (which means the location
of that token is BUILTINS_LOCATION) the location prefix displayed to
the user in the diagnostic line is the "<built-in>:0:0" string.  For
instance:

    <built-in>:0:0: warning: conversion to 'float' alters 'int' constant value

For the user, I think this is surprising and useless.

A more user-friendly approach would be to refer to the first location
that (in the reported macro expansion context) is for a location in
real source code, like what is shown in the new test case
gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C accompanying
this patch.

To do this, I am making the line-map module provide a new
linemap_unwind_to_first_non_reserved_loc function that resolves a
virtual location to the first spelling location that is in real source
code.

I am then using that facility in the diagnostics printing module and
in the macro unwinder to avoid printing diagnostics lines that refer
to the locations for built-ins or more generally for reserved
locations.  Note that when I start the dance of skipping a built-in
location I also skip locations that are in system headers, because it
turned out that a lot of those built-ins are actually used in system
headers (e.g, "#define INT_MAX __INT_MAX__" where __INT_MAX__ is a
built-in).

Besides the user-friendliness gain, this patch allows a number of
regression tests to PASS unchanged with and without
-ftrack-macro-expansion.

Tested and bootstrapped on x86_64-unknown-linux-gnu against trunk.

Note that the bootstrap with -ftrack-macro-expansion exhibits other
separate issues that are addressed in subsequent patches.  This patch
just fixes one class of problems.

The patch does pass bootstrap with -ftrack-macro-expansion turned off,
though.

libcpp/

	* include/line-map.h (linemap_unwind_toward_expansion): Fix typo
	in comment.
	(linemap_unwind_to_first_non_reserved_loc): Declare new function.
	* line-map.c (linemap_unwind_to_first_non_reserved_loc): Define
	new function.

gcc/

	* input.c (expand_location_1): When expanding to spelling location
	in a context of a macro expansion, skip reserved system header
	locations.  Update comments.  * tree-diagnostic.c
	(maybe_unwind_expanded_macro_loc): Likewise.

gcc/testsuite/

	* g++.dg/warn/Wconversion-real-integer2.C: New test.
	* g++.dg/warn/Wconversion-real-integer-3.C: Likewise.
	* g++.dg/warn/conversion-real-integer-3.h: New header used by the
	new test above.

From-SVN: r186970

7 files changed, 110 insertions, 6 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a6e8cbb..9ec7dde 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,11 @@
 2012-04-30  Dodji Seketeli  <dodji@redhat.com>
 
+	Strip "<built-in>" loc from displayed expansion context
+	* input.c (expand_location_1): When expanding to spelling location
+	in a context of a macro expansion, skip reserved system header
+	locations.  Update comments.  * tree-diagnostic.c
+	(maybe_unwind_expanded_macro_loc): Likewise.
+
 	Make expand_location resolve to locus in main source file
 	* input.c (expand_location_1): New.  Takes a parameter to choose
 	whether to resolve the location to spelling or expansion point.
diff --git a/gcc/input.c b/gcc/input.c
index e9ba301..260be7e 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -35,7 +35,14 @@ struct line_maps *line_table;
    location is set to the string "<built-in>". If EXPANSION_POINT_P is
    TRUE and LOC is virtual, then it is resolved to the expansion
    point of the involved macro.  Otherwise, it is resolved to the
-   spelling location of the token.  */
+   spelling location of the token.
+
+   When resolving to the spelling location of the token, if the
+   resulting location is for a built-in location (that is, it has no
+   associated line/column) in the context of a macro expansion, the
+   returned location is the first one (while unwinding the macro
+   location towards its expansion point) that is in real source
+   code.  */
 
 static expanded_location
 expand_location_1 (source_location loc,
@@ -43,12 +50,29 @@ expand_location_1 (source_location loc,
 {
   expanded_location xloc;
   const struct line_map *map;
+  enum location_resolution_kind lrk = LRK_MACRO_EXPANSION_POINT;
+
+  memset (&xloc, 0, sizeof (xloc));
 
-  loc = linemap_resolve_location (line_table, loc,
-				  expansion_point_p
-				  ? LRK_MACRO_EXPANSION_POINT
-				  : LRK_SPELLING_LOCATION, &map);
-  xloc = linemap_expand_location (line_table, map, loc);
+  if (loc >= RESERVED_LOCATION_COUNT)
+    {
+      if (!expansion_point_p)
+	{
+	  /* We want to resolve LOC to its spelling location.
+
+	     But if that spelling location is a reserved location that
+	     appears in the context of a macro expansion (like for a
+	     location for a built-in token), let's consider the first
+	     location (toward the expansion point) that is not reserved;
+	     that is, the first location that is in real source code.  */
+	  loc = linemap_unwind_to_first_non_reserved_loc (line_table,
+							  loc, &map);
+	  lrk = LRK_SPELLING_LOCATION;
+	}
+      loc = linemap_resolve_location (line_table, loc,
+				      lrk, &map);
+      xloc = linemap_expand_location (line_table, map, loc);
+    }
 
   if (loc <= BUILTINS_LOCATION)
     xloc.file = loc == UNKNOWN_LOCATION ? NULL : _("<built-in>");
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index ed2779e..7194809 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2012-04-30  Dodji Seketeli  <dodji@redhat.com>
 
+	Strip "<built-in>" loc from displayed expansion context
+	* g++.dg/warn/Wconversion-real-integer2.C: New test.
+	* g++.dg/warn/Wconversion-real-integer-3.C: Likewise.
+	* g++.dg/warn/conversion-real-integer-3.h: New header used by the
+	new test above.
+
 	Fix expansion point loc for macro-like tokens
 	* gcc.dg/debug/dwarf2/pr41445-5.c: Adjust.
     	* gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C
new file mode 100644
index 0000000..a4df010
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C
@@ -0,0 +1,20 @@
+// { dg-do compile }
+// { dg-options "-Wconversion -ftrack-macro-expansion=2" }
+// { dg-require-effective-target int32plus }
+
+#include "conversion-real-integer-3.h"
+
+float  vfloat;
+
+void h (void)
+{
+    // We want to trigger an error on the token INT_MAX below, that is
+    // a macro that expands to the built-in __INT_MAX__.  Furthermore,
+    // INT_MAX is defined inside a system header.
+    //
+    // The behaviour we want is that the diagnostic should point to
+    // the locus that inside the source code here, at the relevant
+    // line below, even with -ftrack-macro-expansion.  We don't want
+    // it to point to the any locus that is inside the system header.
+    vfloat = INT_MAX; // { dg-warning "conversion to .float. alters .int. constant value" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C
new file mode 100644
index 0000000..29130f1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C
@@ -0,0 +1,33 @@
+/* { dg-do compile }
+/* { dg-options "-Wconversion -ftrack-macro-expansion=2" } */
+/* { dg-require-effective-target int32plus } */
+
+// Before the fix that came with this test, we'd output an error for
+// the __INT_MAX__ token.  That token has a BUILTINS_LOCATION
+// location, so the the location prefix in the warning message would
+// be:
+//     <built-in>:0:0: warning: conversion to 'float' alters 'int' constant value
+//
+// Note the useless and confusing <built-in>:0:0 prefix.  This is
+// because '__INT_MAX__' being an internal macro token, it has a
+// BUILTINS_LOCATION location.
+//
+// In this case, we want the error message to refer to the first
+// location (in the macro expansion context) that is not a location
+// for a built-in token.  That location would be the one for where (in
+// real source code) the __INT_MAX__ macro has been expanded.
+//
+// That would be something like:
+//
+//     gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C:21:17: warning: conversion to 'float' alters 'int' constant value
+//
+// That is more useful.
+
+#define INT_MAX __INT_MAX__ // { dg-warning "conversion to .float. alters .int. constant value" }
+
+float  vfloat;
+
+void h (void)
+{
+    vfloat = INT_MAX; // { dg-message "expanded from here" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h b/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h
new file mode 100644
index 0000000..6ed5b2c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h
@@ -0,0 +1,3 @@
+#pragma GCC system_header
+
+#define INT_MAX __INT_MAX__
diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
index 48c7800..0a55925f 100644
--- a/gcc/tree-diagnostic.c
+++ b/gcc/tree-diagnostic.c
@@ -166,6 +166,18 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
           linemap_resolve_location (line_table, iter->where,
                                     LRK_MACRO_DEFINITION_LOCATION, NULL);
 
+	/* Don't print trace for locations that are reserved or from
+	   within a system header.  */
+	{
+	  const struct line_map *m = NULL;
+	  source_location l = linemap_resolve_location (line_table, resolved_def_loc,
+							LRK_SPELLING_LOCATION,
+							&m);
+	  if (l < RESERVED_LOCATION_COUNT
+	      || LINEMAP_SYSP (m))
+	    continue;
+	}
+
         /* Resolve the location of the expansion point of the macro
            which expansion gave the token represented by def_loc.
            This is the locus 2/ of the earlier comment.  */