Re: [PATCH 07/22] Implement token range tracking within libcpp and C/C++ FEs

[David Malcolm <dmalcolm at redhat dot com>]

On Tue, 2015-09-15 at 12:20 +0200, Jakub Jelinek wrote:
> On Tue, Sep 15, 2015 at 12:14:22PM +0200, Richard Biener wrote:
> > > diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
> > > index 760467c..c7558a0 100644
> > > --- a/gcc/cp/parser.h
> > > +++ b/gcc/cp/parser.h
> > > @@ -61,6 +61,8 @@ struct GTY (()) cp_token {
> > >    BOOL_BITFIELD purged_p : 1;
> > >    /* The location at which this token was found.  */
> > >    location_t location;
> > > +  /* The source range at which this token was found.  */
> > > +  source_range range;
> > 
> > Is it just me or does location now feel somewhat redundant with range?  Can't we
> > compress that somehow?
> 
> For a token I'd expect it is redundant, I don't see how it would be useful
> for a single preprocessing token to have more than start and end locations.

Indeed.   Patch 7 of the patch kit is merely considering tokens (in each
of libcpp and the C and C++ FEs), and in each of these three cases, once
we have a range, the "location" is just the start of the range.   I kept
the location to keep the patch smaller.   I can look into eliminating
the location field from the 3 relevant structures.

(I considered that we could have just the location as the start, and
calculate the end from the length of the token, but AFAIK this can't
cover token concatenation by the preprocessor, and if we're going to
have ranges later on, it's simpler to use the same representation).

> But generally, for expressions, 3 locations make sense.
> If you have
> abc + def
> ~~~~^~~~~
> then having a range is useful.  In any case, I'm surprised that the ranges aren't encoded in
> location_t (the data structures behind it, where we already stick also
> BLOCK pointer).

I posted a few ideas I had for implementing ranges in:
"[PATCH 12/22] Add source-ranges for trees"
https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00739.html

but putting them into the ad-hoc location data is one that hadn't
occurred to me.  Thanks; I'll look into it.

Dave