On Mon, 2018-11-12 at 21:13 +0000, Mike Gulick wrote: > 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).
Both input.o and diagnostic-show-locus.o are in OBJS-libcommon, so I'm not sure what went wrong. > 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? Thanks. That sounds good (maybe put the decl in diagnostic.h rather than diagnostic-core.h; the latter is used in lots of places, whereas the former is more about implementation details). > > > /* 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? The latter: just an updated version of the changed patch. IIRC the rest is all approved. > > Also, I'm waiting on FSF for assignment paperwork. I've re-pinged > them after > waiting a week. Thanks. > Thanks for the feedback and help. > > -Mike