Re: [PATCH 06/11] Strip built-in loc from displayed expansion context

2012-04-11 Thread Jason Merrill

Good thought.  OK.

Jason


[PATCH 06/11] Strip built-in loc from displayed expansion context

2012-04-10 Thread Dodji Seketeli
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.
---
 gcc/input.c|   38 ---
 .../g++.dg/warn/Wconversion-real-integer-3.C   |   20 
 .../g++.dg/warn/Wconversion-real-integer2.C|   33 +
 .../g++.dg/warn/conversion-real-integer-3.h|3 +
 gcc/tree-diagnostic.c  |   12 +
 libcpp/include/line-map.h  |   20 -
 libcpp/line-map.c  |   49 
 7 files changed, 167 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h

diff --git a/gcc/input.c b/gcc/input.c
index dcd348b..8edf05b 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;
-
-  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);
+  enum location_resolution_kind lrk = LRK_MACRO_EXPANSION_POINT;
+
+  memset (xloc, 0, sizeof (xloc));
+
+  if (loc = RESERVED_LOCATION_COUNT)
+{
+  if (!expansion_point_p)
+   {
+