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
This commit is contained in:
Dodji Seketeli 2012-04-30 11:42:12 +00:00 committed by Dodji Seketeli
parent 7eb918cc4e
commit c4ca1a0961
10 changed files with 185 additions and 7 deletions

View File

@ -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.

View File

@ -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;
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);
memset (&xloc, 0, sizeof (xloc));
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>");

View File

@ -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.

View File

@ -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" }
}

View File

@ -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" }
}

View File

@ -0,0 +1,3 @@
#pragma GCC system_header
#define INT_MAX __INT_MAX__

View File

@ -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. */

View File

@ -1,5 +1,12 @@
2012-04-30 Dodji Seketeli <dodji@redhat.com>
Strip "<built-in>" loc from displayed expansion context
* 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.
Fix expansion point loc for macro-like tokens
* macro.c (macro_of_context): New static function.
(_cpp_push_token_context, push_extended_tokens_context): If the

View File

@ -666,12 +666,30 @@ source_location linemap_resolve_location (struct line_maps *,
location L of the point where M got expanded. If L is a spelling
location inside a macro expansion M', then this function returns
the point where M' was expanded. LOC_MAP is an output parameter.
When non-NULL, *LOC_MAP is set the the map of the returned
When non-NULL, *LOC_MAP is set to the map of the returned
location. */
source_location linemap_unwind_toward_expansion (struct line_maps *,
source_location loc,
const struct line_map **loc_map);
/* If LOC is the virtual location of a token coming from the expansion
of a macro M and if its spelling location is reserved (e.g, a
location for a built-in token), then this function unwinds (using
linemap_unwind_toward_expansion) the location until a location that
is not reserved and is not in a system header is reached. In other
words, this unwinds the reserved location until a location that is
in real source code is reached.
Otherwise, if the spelling location for LOC is not reserved or if
LOC doesn't come from the expansion of a macro, the function
returns LOC as is and *MAP is not touched.
*MAP is set to the map of the returned location if the later is
different from LOC. */
source_location linemap_unwind_to_first_non_reserved_loc (struct line_maps *,
source_location loc,
const struct line_map **map);
/* Expand source code location LOC and return a user readable source
code location. LOC must be a spelling (non-virtual) location. If
it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source

View File

@ -1114,6 +1114,55 @@ linemap_unwind_toward_expansion (struct line_maps *set,
return resolved_location;
}
/* If LOC is the virtual location of a token coming from the expansion
of a macro M and if its spelling location is reserved (e.g, a
location for a built-in token), then this function unwinds (using
linemap_unwind_toward_expansion) the location until a location that
is not reserved and is not in a sytem header is reached. In other
words, this unwinds the reserved location until a location that is
in real source code is reached.
Otherwise, if the spelling location for LOC is not reserved or if
LOC doesn't come from the expansion of a macro, the function
returns LOC as is and *MAP is not touched.
*MAP is set to the map of the returned location if the later is
different from LOC. */
source_location
linemap_unwind_to_first_non_reserved_loc (struct line_maps *set,
source_location loc,
const struct line_map **map)
{
source_location resolved_loc;
const struct line_map *map0 = NULL, *map1 = NULL;
map0 = linemap_lookup (set, loc);
if (!linemap_macro_expansion_map_p (map0))
return loc;
resolved_loc = linemap_resolve_location (set, loc,
LRK_SPELLING_LOCATION,
&map1);
if (resolved_loc >= RESERVED_LOCATION_COUNT
&& !LINEMAP_SYSP (map1))
return loc;
while (linemap_macro_expansion_map_p (map0)
&& (resolved_loc < RESERVED_LOCATION_COUNT
|| LINEMAP_SYSP (map1)))
{
loc = linemap_unwind_toward_expansion (set, loc, &map0);
resolved_loc = linemap_resolve_location (set, loc,
LRK_SPELLING_LOCATION,
&map1);
}
if (map != NULL)
*map = map0;
return loc;
}
/* Expand source code location LOC and return a user readable source
code location. LOC must be a spelling (non-virtual) location. If
it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source