Re: [PATCH 3/7] Emit macro expansion related diagnostics

[Jason Merrill <jason@redhat.com>]

On 09/19/2011 02:29 PM, Jason Merrill wrote:
> expansion location. So it seems like we ought to be able to get away
> with only storing one location per token in a macro expansion map. Am I
> missing something?

I notice that here:

> +        /* Resolve the location iter->where into the locus 1/ of the
> +           comment above.  */
> +        resolved_def_loc =
> +          linemap_resolve_location (line_table, iter->where,
> +                                    LRK_MACRO_PARM_REPLACEMENT_POINT, NULL);
> +
> +        /* Resolve the location of the expansion point of the macro
> +           which expansion gave the token represented by def_loc.
> +           This is the locus 2/ of the earlier comment.  */
> +        resolved_exp_loc =
> +          linemap_resolve_location (line_table,
> +                                    MACRO_MAP_EXPANSION_POINT_LOCATION (iter->map),
> +                                    LRK_MACRO_PARM_REPLACEMENT_POINT, NULL);

You're using LRK_MACRO_PARM_REPLACEMENT_POINT in both places for 
printing (and thus the second of the two token locations), whereas you 
used the first location in the unwinding process.

It is sounding to me like the first location (xI) gets you the next 
virtual location in the unwinding process, whereas the second location 
(yI) gets you the spelling location of the token in the definition of a 
macro.  Certainly all the calls to tokens_buff_add_token pass 
src->src_loc for the second.  So why don't we look up the second 
location in the macro definition when we need it rather than store a 
copy in the map?

Jason