Re: [PATCH] improve hack in src/base/TextException.h

Hi all,

On 02/21/2011 08:31 PM, Alex Rousskov wrote:
> On 02/21/2011 12:27 AM, Kinkie wrote:
>>> Sorry, I am not sure I understand the above. Should I reverse r11236 and
>>> commit a casting fix? Or do you want be to post a patch that some Hudson
>>> job will pick up and test somehow first?
>>
>> Why?
>> Judging from the comment in there it's just meant to trick the
>> compiler into not (mis)understanding that the FileNameHash function is
>> unused, because in the end it's invoked via trickery.
>
> FileNameHash is _sometimes_ unused. The function is used in compilation
> units where certain macros are used and only in those compilation units.
>
>
>> IMVHO there are two clean ways out of this:
>> - use a compiler-dependent thing such as __attribute__((unused)) to
>> politely inform the compiler that sometimes the programmer knows more
>> about stuff than the compiler
>
> A compiler-specific attribute is not a "clean way", especially when it
> is introduced as a fix for a rather different problem and actually kind
> of lies to the compiler in some cases.
>
>
>> - just get rid of this whole FileNameHashCached thing as a premature
>> optimization, and just recalculate the filename hash when we throw a
>> TextException (which is hopefully not that often)
>
> Sure, let's revisit the necessity of this hash caching optimization.
>
> Christos, do you remember what were the key reasons for adding it in the
> first place?

OK, with a little research I found it :-)
This is a part of the log error details patch.

The original comment was:
> ... For the files the Must is not used, I got a compiler error that " the FileNameHashCached defined but not used"
> To solve it I just define a foo_class....

I am not sure about the "__attribute__((unused))" definition but if it
is only suppressing the warning, it is not bad idea...

>
>
> Thank you,
>
> Alex.
>
>
Received on Tue Feb 22 2011 - 10:54:56 MST