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

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