On 11/2/18 5:04 PM, David Malcolm wrote: > On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: >> 2017-10-31 Mike Gulick <mgul...@mathworks.com> >> >> PR preprocessor/83173 >> * gcc/input.c (dump_location_info): Dump reason and >> included_from fields from line_map_ordinary struct. Fix >> indentation when location > 5 digits. >> >> * libcpp/location-example.txt: Update example >> -fdump-internal-locations output. >> --- >> gcc/input.c | 49 +++++- >> libcpp/location-example.txt | 333 +++++++++++++++++++++------------- >> -- >> 2 files changed, 241 insertions(+), 141 deletions(-) > > Sorry about the belated response. This is a nice enhancement; some > nits below. > >> diff --git a/gcc/input.c b/gcc/input.c >> index a94a010f353..f938a37f20e 100644 >> --- a/gcc/input.c >> +++ b/gcc/input.c >> @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE *stream, >> fprintf (stream, "\n"); >> } >> >> +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ >> + (x) >= 100000000 ? 9 : \ >> + (x) >= 10000000 ? 8 : \ >> + (x) >= 1000000 ? 7 : \ >> + (x) >= 100000 ? 6 : \ >> + (x) >= 10000 ? 5 : \ >> + (x) >= 1000 ? 4 : \ >> + (x) >= 100 ? 3 : \ >> + (x) >= 10 ? 2 : \ >> + 1) > > diagnostic-show-locus.c has a function "num_digits" (currently static) > and, fwiw, a unit test. It would be good to share the implementation. >
I initially tried to use this function by just adding "extern int num_digits(int);" into diagnostic-core.h, but that failed to link, so it seems like diagnostic-show-locus.c is not included in whatever library input.c gets linked with (I forget which library it was trying to link). Instead I moved num_digits and its unit test to diagnostic.c, and added the extern definition to diagnostic-core.h. That builds and tests successfully. Does that seem like a reasonable way to do this? >> /* Write a visualization of the locations in the line_table to >> STREAM. */ >> >> void >> @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) >> map->m_column_and_range_bits - map->m_range_bits); >> fprintf (stream, " range bits: %i\n", >> map->m_range_bits); >> + const char * reason; >> + switch (map->reason) { >> + case LC_ENTER: >> + reason = "LC_ENTER"; >> + break; >> + case LC_LEAVE: >> + reason = "LC_LEAVE"; >> + break; >> + case LC_RENAME: >> + reason = "LC_RENAME"; >> + break; >> + case LC_RENAME_VERBATIM: >> + reason = "LC_RENAME_VERBATIM"; >> + break; >> + case LC_ENTER_MACRO: >> + reason = "LC_RENAME_MACRO"; >> + break; >> + default: >> + reason = "Unknown"; >> + } >> + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); >> + >> + const line_map_ordinary *includer_map >> + = linemap_included_from_linemap (line_table, map); >> + fprintf (stream, " included from map: %d\n", >> + includer_map ? int (includer_map - line_table- >>> info_ordinary.maps) >> + : -1); > > I'm not a fan of "-1" here; it's a NULL pointer in the original data. > How about "n/a" for that case? > That's a good suggestion. Thanks. >> + fprintf (stream, " included from location: %d\n", >> + linemap_included_from (map)); > > ...or merging it with this line, for something like: > > included from location: 127 (in ordinary map 2) > > vs: > > included from location: 0 > > [...snip...] > > Other than that, this is OK for trunk, assuming your contributor > paperwork is in place. > > Dave > What is the preferred way to re-send this patch? Should I re-send the entire patch series as v4, or just an updated version of this single patch? Also, I'm waiting on FSF for assignment paperwork. I've re-pinged them after waiting a week. Thanks for the feedback and help. -Mike