Re: [PATCH 0/6] Tracking locations of tokens resulting from macro expansion

[Dodji Seketeli <dodji@redhat.com>]

Jeff Law <law@redhat.com> writes:

> On 12/10/10 04:11, Dodji Seketeli wrote:
>> The problem is tokens '1.0', '<<' and '1' all have their location set
>> to {line 11, column 3} ({11, 3} for short). That actually is the
>> location of the expansion point of the MULT macro. That's is an
>> interesting observation -- in the current incarnation of GCC the
>> location of each tokens resulting from a macro expansion is set to the
>> location of the expansion point of the macro.
> Presumably you meant {13, 3}, right?

Yes. Sorry for the confusion.

>
>> Several obversations can be made in that hypothetical example.
>>
>> - The location mentioned in the error message
>>
>>   test.c:5:14: error: invalid operands to binary<<  (have ‘double’ and ‘int’)
>>
>> is the spelling location of the token '<<'. I believe this makes more
>> sense than the original behaviour.
> Agreed.  However, I suspect some will disagree and it may cause havoc
> in automated code that parses error messages.  Presumably there's a
> way to get the old behavior, even if you're tracking all three
> locations?

The linemap API allows us to get the old behaviour (i.e, resolve virtual
locations to the macro expansion point). But as most of the client code
does that through the expand_location function, I wasn't sure how to
introduce choice there in a convenient way. Maybe I should add another
flag to control the "user interface" so to speak?

>
>
>> - The macro expansion stack following the error message unwinds from
>> the macro which expansion most directly yielded token "<<". Each
>> element of the stack mentions the argument replacement point and the
>> expansion point locations that were exposed earlier.
> I like it.  Though you might want a parameter which defines how many
> levels in the stack you want to display.  I'm not sure on this one.

Indeed. I thought about this as well. But then what stopped me really is
wasn't sure about *what* to display. User interface again :-) I am open
to suggestions. Technically, limiting the displayed levels of the stack
is not hard to do.

>
>
>>
>> * Limits
>>
>> I guess a pure marketing dude would elide this part :-)
>>
>> The most annoying limit is obviously memory consumption. Tracking the
>> location of every token across every macro expansion does consume
>> memory. I tried compiling some a translation here that heavily uses
>> the C++ standard library and I could see some 13% increase of memory
>> consumption over the entire compilation; note that the compilation was
>> consuming around 250MB without the feature. This is why the feature is
>> triggered by the -ftrack-macro-expansion flag. Without that flag, the
>> memory consumption stays roughly flat. The noticeable downside of that
>> flag is that it makes the code more complex. I think there possibly is
>> room to decrease this, but fundamentaly the consumption is going to
>> increase with the number of macro expanded multiplied by the number of
>> tokens per expansion.
> I'm surprised at the increase, not that it exists, but the magnitude.
> But if that's the cost, then we'll learn to deal with it.  Having the
> flag definitely helps -- though I can see a point in the future where
> if this stuff works really well, we'll just turn it on by default and
> be done with it.

I got very annoyed by the magnitude. At least now the overhead scales
linearly with the number of macros and tokens per expansion. Tom and I
have been discussing about a way to encode the mapping virtual locations
--> {different kind of physical locations} in a more compact manner. I
really need and want to investigate that and see what I can
gain. Looking the dumps of the memory consumption of the macro maps did
suggest that there is some room of improvement there.

-- 
	Dodji