Message ID | m3zkaj5puy.fsf@redhat.com |
---|---|
State | New |
Headers | show |
On 04/10/2012 03:49 PM, Dodji Seketeli wrote: > Apparently, quite some places in the compiler (like the C/C++ > preprocessor, the debug info machinery) expect expand_location to > resolve to locations that are in the main source file, even if the > token at stake comes from a macro that was defined in a header > somewhere. Turning on -ftrack-macro-expansion by default was > triggering a lot of failures (not necessarily related to diagnostics) > because expand_location resolves to spelling locations instead. Can you elaborate on these failures? For debug info I would think that the spelling location would be useful information, though I suppose without any way to "unwind" to the expansion context it could be a bit confusing. What is the problem for the preprocessor? Jason
> On 04/10/2012 03:49 PM, Dodji Seketeli wrote: >> Apparently, quite some places in the compiler (like the C/C++ >> preprocessor, the debug info machinery) expect expand_location to >> resolve to locations that are in the main source file, even if the >> token at stake comes from a macro that was defined in a header >> somewhere. Turning on -ftrack-macro-expansion by default was >> triggering a lot of failures (not necessarily related to diagnostics) >> because expand_location resolves to spelling locations instead. > > Can you elaborate on these failures? For debug info I would think > that the spelling location would be useful information, though I > suppose without any way to "unwind" to the expansion context it could > be a bit confusing. What is the problem for the preprocessor? For the preprocessor, consider a short excerpt of the the test gcc/testsuite/gcc.dg/cpp/avoidpaste1.c: #define f(x) x #define g #define tricky 1.0e ## -1 :: :g: :f(): :f(^): tricky As the comment in the text says, it should preprocess as: :: : : : : :^: 1.0e- 1 but it actually preprocess as: # 1 "/home/dodji/devel/git/gcc/tmp/gcc/testsuite/gcc.dg/cpp/avoidpaste1.c" # 1 "<command-line>" # 1 "/home/dodji/devel/git/gcc/tmp/gcc/testsuite/gcc.dg/cpp/avoidpaste1.c" # 25 "/home/dodji/devel/git/gcc/tmp/gcc/testsuite/gcc.dg/cpp/avoidpaste1.c" :: : : : : :^: # 14 "/home/dodji/devel/git/gcc/tmp/gcc/testsuite/gcc.dg/cpp/avoidpaste1.c" 1.0e- 1 Note how the "1.0e- 1" is not on the same line as its preceding ":: : : : : :^:". The problem is that the pre-processor code indirectly uses expand_location in many spots. For that kind of existing code that is not related to diagnostics, I am really not confident in changing the underlying implicit assumption of the that function which is, to return the expansion point location. I don't even know before hand where all these spots are in the code base, so auditing it is hard for me. I forgot to say that are also other weird failure like: FAIL: g++.dg/cdce3.C -std=gnu++98 scan-tree-dump cdce "cdce3.C:92: note: function call is shrink-wrapped into error conditions." due to that change. That's why I prefer erring on the safe side by not changing the assumption of existing code, and provide a way to expand locations to their spelling point, just for diagnostics. I think this is already an improvement over what we previously had. And for the debug info cases, I'd propose that if we find specific examples where the unwinding to the spelling locations is better, then we'll consider using it at that moment.
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index a68d6ce..337328c 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -183,7 +183,7 @@ diagnostic_build_prefix (diagnostic_context *context, "must-not-happen" }; const char *text = _(diagnostic_kind_text[diagnostic->kind]); - expanded_location s = expand_location (diagnostic->location); + expanded_location s = expand_location_to_spelling_point (diagnostic->location); if (diagnostic->override_column) s.column = diagnostic->override_column; gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND); diff --git a/gcc/input.c b/gcc/input.c index 4077f9e..dcd348b 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -32,16 +32,22 @@ struct line_maps *line_table; /* Expand the source location LOC into a human readable location. If LOC resolves to a builtin location, the file name of the readable - location is set to the string "<built-in>". */ - -expanded_location -expand_location (source_location loc) + 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. */ + +static expanded_location +expand_location_1 (source_location loc, + bool expansion_point_p) { expanded_location xloc; const struct line_map *map; loc = linemap_resolve_location (line_table, loc, - LRK_SPELLING_LOCATION, &map); + expansion_point_p + ? LRK_MACRO_EXPANSION_POINT + : LRK_SPELLING_LOCATION, &map); xloc = linemap_expand_location (line_table, map, loc); if (loc <= BUILTINS_LOCATION) @@ -50,6 +56,29 @@ expand_location (source_location loc) return xloc; } +/* Expand the source location LOC into a human readable location. If + LOC is virtual, it resolves to the expansion point of the involved + macro. If LOC resolves to a builtin location, the file name of the + readable location is set to the string "<built-in>". */ + +expanded_location +expand_location (source_location loc) +{ + return expand_location_1 (loc, /*expansion_point_p=*/true); +} + +/* Expand the source location LOC into a human readable location. If + LOC is virtual, it resolves to the expansion location of the + relevant macro. If LOC resolves to a builtin location, the file + name of the readable location is set to the string + "<built-in>". */ + +expanded_location +expand_location_to_spelling_point (source_location loc) +{ + return expand_location_1 (loc, /*expansion_piont_p=*/false); +} + #define ONE_K 1024 #define ONE_M (ONE_K * ONE_K) diff --git a/gcc/input.h b/gcc/input.h index f2f3513..5838153 100644 --- a/gcc/input.h +++ b/gcc/input.h @@ -38,6 +38,7 @@ extern char builtins_location_check[(BUILTINS_LOCATION < RESERVED_LOCATION_COUNT) ? 1 : -1]; extern expanded_location expand_location (source_location); +extern expanded_location expand_location_to_spelling_point (source_location); /* Historically GCC used location_t, while cpp used source_location. This could be removed but it hardly seems worth the effort. */ @@ -49,6 +50,14 @@ extern location_t input_location; #define LOCATION_LINE(LOC) ((expand_location (LOC)).line) #define LOCATION_COLUMN(LOC)((expand_location (LOC)).column) +#define EXPANSION_POINT_LOCATION_FILE(LOC) \ + ((expand_location_to_expansion_point (LOC)).file) +#define EXPANSION_POINT_LOCATION_LINE(LOC) \ + ((expand_location_to_expansion_point (LOC)).line) +#define EXPANSION_POINT_LOCATION_COLUMN(LOC) \ + ((expand_location_to_expansion_point (LOC)).column) + + #define input_line LOCATION_LINE (input_location) #define input_filename LOCATION_FILE (input_location) #define in_system_header_at(LOC) \