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

[Jason Merrill <jason@redhat.com>]

On 09/27/2011 01:52 PM, Dodji Seketeli wrote:
> +     Remember we are at the expansion point of MACRO.  Each xI is the
> +     location of the Ith token of the replacement-list. Now it gets
> +     confusing. the xI is the location of the Ith token of the
> +     replacement-list at the macro *definition* point. Not at the
> +     macro replacement point. Okay, let's try to explain this below.

This should be yI.

> +     The token '+' has two locations, so to speak. One in the context
> +     of the macro *expansion* of PLUS in #2 and one in the context of
> +     the macro *definition* of PLUS in #1. These two locations are
> +     encoded in the the latter context, somehow in the xI we are
> +     talking about.

The location of '+' in #2 is not encoded in xI or yI, we reach it 
through the expansion point location of the macro.  The location in #1 
is yI (and xI, in this case).

> +     xI is roughly the index of the token inside the replacement-list
> +     at the expansion point. So for '+', it's index would then be 1

"its"

> +     [The index of token '1' would be 0 and the index of token 2 would
> +     be 1]. So if '+' is our current xI, it is actualy an x1.

Are we still talking about #1 here?  It looks to me like the index of 
"1" would be 2, the index of "+" would be 4, and the index of token "2" 
would be 6.  I bet PLUS used to just be "A + B", and this section of 
comment didn't get updated when it changed.

Keep changing xI to yI.

> +     Now what's the y1 then? Remember, we said macro_locations is an
> +     array of pairs (xI,yI). We now know what the xI is, now let's
> +     look at the yI.

xI allows us to find where the token was actually written.  If the 
current macro context is also the spelling location of the token (e.g. 
#1 for "+"), then xI is the same as yI, i.e. the source location of that 
token.

If the current macro context is not the spelling location of the token 
(e.g. #0 or #1 for "1"), then xI is the location outside the current 
macro context.  For "1" in #0, this the location of "1" in #1, which is 
a virtual location.  For "1" in #1, this is the location of "1" in #2, 
which is a source location.

> +   * If LRK is set to LRK_MACRO_EXPANSION_POINT
> +   -------------------------------
> +
> +   The virtual location is resolved to the location to the locus of
> +   the expansion point of the macro.

The first macro expansion point that led to this macro expansion.

> +   * If LRK is set to LRK_MACRO_DEFINITION_LOCATION
> +   --------------------------------------

The virtual location is resolved to the locus of the token in the 
context of the macro definition.

> +   If LOC is the locus of a token that is an argument of a
> +   function-like macro [replacing a parameter in the replacement list
> +   of the macro] the virtual location is resolved to the locus of the
> +   parameter that is replaced, in the context of the definition of the
> +   macro.
> +
> +   If LOC is the locus of a token that is not an argument of a
> +   function-like macro, then the function behaves as if LRK was set to
> +   LRK_SPELLING_LOCATION.

(and then keep these two paragraphs)

> +   Finally, if SPELLING_LOC is not NULL, *RESULTING_LOC is set to the
> +   location to which LOC was resolved

SPELLING_LOC doesn't exist anymore.

> +   ORIG_LOC is the orginal location of the token at the definition
> +   point of the macro. If you read the extensive comments of struct
> +   line_map_macro in line-map.h, this is the xI.
> +
> +   If the token is part of a macro argument, ORIG_PARM_REPLACEMENT_LOC
> +   is the location of the point at wich the token (the argument)
> +   replaces the macro parameter in the context of the relevant macro
> +   definition. If you read the comments of struct line_map_macro in
> +   line-map.h, this is the yI.  */
> +
> +source_location
> +linemap_add_macro_token (const struct line_map *map,

ORIG_LOC is the location of the token outside this macro expansion.  If 
the token comes originally from the macro definition, it is the locus in 
the macro definition; otherwise it is a location in the caller of this 
macro expansion (which is a virtual location or a source location if the 
caller is itself a macro expansion or not).

MACRO_DEFINITION_LOC is the location in the macro definition, either of 
the token itself or of a macro parameter that it replaces.

> +/* If LOCATION is the locus of a token that is an argument of a
> +   function-like macro M and appears in the expansion of M, return the
> +   locus of that argument in the context of the caller of M.  Note
> +   that the caller of M is necessarily another macro.

Why is the caller of M necessarily another macro?  In the PLUS example 
above, if we have the location for "1" in #1, won't it give us the 
location of "1" in #2?

>   The context of M is a macro definition.

What does this mean?

> +/* Return the source line number corresponding to source location
> +   LOCATION.  SET is the line map set LOCATION comes from.  If
> +   LOCATION is the source location of token that is part of the
> +   replacement-list of a macro expansion return the line number of the
> +   macro expansion point.  */
> +
> +int
> +linemap_get_source_line (struct line_maps *set,
> +                        source_location location)

Let's call this linemap_get_expansion_line.

> +linemap_get_source_column (struct line_maps *set,

This seems unused.

> +linemap_get_file_path (struct line_maps *set,

Shouldn't we use this for BT_FILE in _cpp_builtin_macro_text?  And call 
it linemap_get_expansion_filename.

> +   Note that this function returns 0 if LOCATION belongs to a token
> +   that is part of a macro replacement-list defined in a system
> +   header, but expanded in a non-system file.  */
> +
> +int
> +linemap_location_in_system_header_p (struct line_maps *set,
> +                                    source_location location)
> +{
> +  const struct line_map *map = NULL;
> +
> +  if (location < RESERVED_LOCATION_COUNT)
> +    return false;
> +
> +  location =
> +    linemap_resolve_location (set, location, LRK_SPELLING_LOCATION, &map);

The comment seems incorrect here.  If the location passed in corresponds 
to a non-parameter token in a macro in a system header, we'll end up 
with the source location of that token, not the expansion location. 
What behavior do we want here?

> +linemap_macro_loc_unwind (struct line_maps *set,

Let's call this linemap_macro_loc_to_spelling_point.

> +/* If LOCATION is the locus of a token that is an argument of a
> +   function-like macro M, return the location of that token in the
> +   context of the definition of the first macro P which expansion
> +   triggered the expansion of M.  Note that the token must be actually
> +   present in the source of the definition of P.  If LOCATION is the
> +   locus of a token that belongs to a macro replacement-list but is
> +   not an argument to a function-like macro, return the same thing as
> +   what linemap_macor_loc_to_def_point would have returned.

This seems unnecessarily complex.  This function returns the spelling 
location of the token wherever it comes from, whether part of a macro 
definition or not.

> linemap_macro_loc_to_exp_point,
> linemap_macro_map_loc_unwind_once and linemap_macro_map_loc_to_def_point
> are now static functions that are only subroutines of
> linemap_resolve_location.  They are not part of the public line map API
> anymore.

I still see them in line-map.h.

Let's rename linemap_macro_map_loc_unwind_once to 
"linemap_macro_map_loc_unwind_toward_spelling", and 
linemap_step_out_once to "linemap_macro_map_loc_unwind_toward_expansion".

Jason