[cfe-commits] r95546 - in /cfe/trunk: include/clang/Checker/DomainSpecific/CocoaConventions.h lib/Checker/CFRefCount.cpp lib/Checker/CocoaConventions.cpp

[Ted Kremenek]

On Feb 8, 2010, at 9:45 AM, Benjamin Kramer wrote:

> 
> Am 08.02.2010 um 17:48 schrieb Ted Kremenek:
> 
>> Hi Ben,
>> 
>> Thanks for doing this, but you've actually change the algorithmic properties of the lookup.  By dispatching in the switch statement on the length of the string, we do few string comparisons in practice.  With your patch, we have a chain of string comparisons, which means that all of them need to be performed until we resolve the last one.
>> 
>> I've gone and reverted this change because I don't think the string lookup slowdown is worth the minor improvement in simplicity, but I strongly agree that we could use StringRef here to make it much cleaner (instead of using memcmp), so I like the premise of the change overall.
>> 
>> Ted
> 
> StringRef::operator== expands into a length comparison and a memcmp, a smart compiler will turn that into a switch + memcmps, doing the same stuff as the old code did.
> 
> I didn't verify the generated assembly in this specific case, but I'm quite confident that both gcc and llvm are smart enough to do this optimization. Otherwise there'll be some unnecessary integer (length) comparisons but the actual string data won't be touched more than once.

Hi Ben,

Thanks so much for explaining this.  The motivation behind removing the switch statement is much clearer now.  I'm fine with you re-applying this patch.

Best,
Ted