[LLVMdev] [llvm-commits] Patch: add contains to ImmutableList

[Rui Paulo]

On Jul 4, 2011, at 7:53 PM, Nick Lewycky wrote:

> Rui Paulo wrote:
>> Hi,
>> 
>> Attached is a patch to add a contains method to ImmutableList. I need this in the clang Static Analyzer.
> 
> Do you really need "this->" here?:
> 
> +    for (iterator I = this->begin(), E = this->end(); I != E; ++I) {

Oops, I guess not.

> 
> Yes the type is templated, but it doesn't have a template base.
> 
> Secondly:
> 
> +      if (*I == V)
> 
> The existing code in ImmutableList does not care at all about how T::operator== works, instead assuming that the objects in the list are uniqued by address. If that assumption is okay with you, you may want to write "if (I == &V)" instead.
> 
> Finally, are you sure that doing an O(n) scan is OK for your use case? I'll take your word for it if it's really the right thing to do, but just having this may encourage poor algorithm choices by others.

It's OK in my case because n is usually 1 to 3.

> As an alternative, how about a function which takes a T* and returns the ImmutableList<T>::iterator pointing to the right element or the end() iterator if it's not a member?

Would this be faster? I could write that, but I was trying to follow the semantics of contains() in ImmutableSet.

> Do you have commit access?


No. :-)

Regards,
--
Rui Paulo