This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output
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 <mgulick@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