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

[Daniel Dunbar]

On Mon, Feb 8, 2010 at 9:45 AM, Benjamin Kramer
<benny.kra at googlemail.com> 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.

Indeed. This is part of the assumptions inherent in the use of
StringRef and StringSwitch. The code should be written cleanly, and if
it doesn't compile to the right thing, its a compiler bug and we
should fix it.

 - Daniel

> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>