[llvm-commits] Patch: add contains to ImmutableList

Nick Lewycky nicholas at mxc.ca
Mon Jul 4 19:53:22 PDT 2011


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) {

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.

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?

Do you have commit access?

Nick



More information about the llvm-commits mailing list