This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/7] Emit macro expansion related diagnostics
On 09/20/2011 10:05 AM, Dodji Seketeli wrote:
Jason Merrill<jason@redhat.com> writes:
In linemap_macro_map_loc_to_def_point you get the token number and
then use that to index into MACRO_MAP_LOCATIONS. Can't you use the
same token number to index into macro->exp.tokens instead?
No, because a macro argument can be made of several tokens.
Ah, I see.
I don't understand how that unwinding operation maps onto a function
called linemap_macro_map_loc_to_def_point,
In the example I used in the comment of that function, suppose that
during 'stepping out', as you put it, we don't deal with the place in
the source where '<<' appears in the definition of a macro. We'd then
only deal with the expansion points of the macros involved.
I think I wasn't expressing clearly enough the point I was trying to
make. My point was more that the name of the function is confusing; if
what you get back is another virtual location, that's not what I would
consider a "def point". The only time you get a source location in the
actual definition of the macro is when you ask for the "macro parm usage
point". And so when you go to print actual diagnostics you use
LRK_MACRO_PARM_REPLACEMENT_POINT.
When we start out, we have a virtual location that represents << in the
expansion of OPERATE. Then we call linemap_macro_map_loc_to_def_point
to get a virtual location that represents << in the expansion of SHIFTL.
This is not part of the definition of OPERATE, and shouldn't be
described as such.
It seems that this function steps out until we hit the spelling
location, and then we have a real source location and stop, which is why
the loop in maybe_unwind_expanded_macro_loc needs to use
linemap_macro_map_loc_to_exp_point as well. Right? In the example,
once we hit << in SHIFTL unwinding needs to switch to the expansion point.
It seems to me that we should encapsulate all of this in a function that
expresses this unwinding operation, say
"linemap_macro_map_loc_unwind_once". So the loop would look like
+ do
+ {
+ loc.where = where;
+ loc.map = map;
+
+ VEC_safe_push (loc_t, heap, loc_vec, &loc);
+
+ /* WHERE is the location of a token inside the expansion of a
+ macro. MAP is the map holding the locations of that macro
+ expansion. Let's get the location of the token in the
+ context that triggered the expansion of this macro.
+ This is basically how we go "down" in the trace of macro
+ expansions that led to WHERE. */
+ where = linemap_unwind_once (where, &map);
+ } while (linemap_macro_expansion_map_p (map));
I think that linemap_macro_loc_to_def_point when called with false
returns the unwound spelling location of the token (and thus is used for
LRK_SPELLING_LOCATION), or when called with true returns the
(not-unwound) location in the expanded macro (and so I think
LRK_MACRO_PARM_REPLACEMENT_POINT should be renamed to
LRK_MACRO_EXPANDED_LOCATION or something similar).
It seems to me that either we should split those functions apart in to
two functions with clearer names, or bundle them and
linemap_macro_loc_to_exp_point into linemap_resolve_location; if
linemap_location_in_system_header_p used linemap_resolve_location it
would be more readable anyway.
I'm having trouble thinking of a less misleading name for
linemap_macro_map_loc_to_def_point, since the two locations serve
completely different purposes. Maybe something vague like
linemap_macro_map_loc_get_stored_loc. Or split it into two functions
like linemap_virtual_loc_unwind_once_toward_spelling and
linemap_virtual_loc_get_expanded_location or something like that.
What do you think?
Jason