Re: [PATCH 1/7] Linemap infrastructure for virtual locations

[Dodji Seketeli <dodji@redhat.com>]

Jason Merrill <jason@redhat.com> writes:

> On 07/16/2011 07:37 AM, Dodji Seketeli wrote:
>> +/* Returns the highest location [of a token resulting from macro
>> +   expansion] encoded in this line table.  */
>> +#define LINEMAPS_MACRO_HIGHEST_LOCATION(SET) \
>> +  LINEMAPS_HIGHEST_LOCATION(SET, true)
>> +
>> +/* Returns the location of the begining of the highest line
>> +   -- containing a token resulting from macro expansion --  encoded
>> +   in the line table SET.  */
>> +#define LINEMAPS_MACRO_HIGHEST_LINE(SET) \
>> +  LINEMAPS_HIGHEST_LINE(SET, true)
>
> What is the use of these?  The ordinary highest line/location are used
> for various things, but I can't think of a reason you would want the
> above, nor are they used in any of the patches.

The former is necessary because locations yielded by original macro maps
grow from zero to upward while locations yielded by macro maps grow from
a MAX_SOURCE_LOCATION downward.  As locations of macro locations
decrease monotonically we need LINEMAPS_MACRO_HIGHEST_LOCATION (used in
linemap_enter_macro) for reasons that are similar to why we need
LINEMAPS_ORDINARY_HIGHEST_LOCATION.

I added LINEMAPS_MACRO_HIGHEST_LINE for symmetry, as we had
LINEMAPS_ORDINARY_HIGHEST_LINE.  It's also because at some point I
thought that I'd need a function like linemap_position_for_column, but
for macro locations.  This could be useful to e.g, create source
locations for macro tokens that we are re-constructing.  E.g, to support
macro locations with -fpreprocessed.  I stepped my ambitions down for
this first batch of patches and I forgot to remove this macro.  I'll
remove it from my local updated version of the patch.

>  Maybe these should be in line_maps instead of maps_info?

I think the former should stay in maps_info as each type of map (macro
and ordinary) need its own highest location.

Jason Merrill <jason@redhat.com> writes:

> On 07/16/2011 07:37 AM, Dodji Seketeli wrote:

> Apart from the added declaration, these changes seem like unnecessary
> churn; let's avoid reformatting unchanged comments and removing
> "extern" from declarations that are correct as they are.

I am cleaning up this in my local version of the patch.

Thanks.

-- 
		Dodji