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

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?

Thank you,

Alex.
Received on Mon Feb 21 2011 - 18:31:45 MST