[PATCH] D18578: Introduce BitSet: A BitVector based class behaving like std::set/DenseSet

[David Blaikie via llvm-commits]

On Wed, Apr 13, 2016 at 11:50 AM, Matthias Braun via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> MatzeB added a comment.
>
> In http://reviews.llvm.org/D18578#400148, @dblaikie wrote:
>
> > This seems to have strange behavior - returning the size/empty status of
> >  the BitVector for the BitSets operations is probably wrong, no? (the
> >  BitVector's length is the length of the field, not the number of set (1)
> >  bits in it)
>
>
> BitSet::size() should return the number of elements in the set which
> corresponds to the number of set bits in the bitvector which should be
> BitVector::count().
>

Ah, looks like you're right - my mistake.


> empty() checks BitSet::size() against 0 which should be equivalent to
> BitVector::count() == 0.


Seems expensive, though - that makes empty O(N)?

(you might be able to simplify 'begin()' by sharing some of the
implementation of "advance" - usually filtering iterators like this would
call "advance" in their ctor, to search through to find the first relevant
element (advance, then, would probably be designed to do nothing if the
current element is valid - so ++ would have to increment the iterator, then
call advance, rather than just calling advance))

My other general question is: Do we need both these abstractions (BitSet
and BitVector) or should we just change BitVector into BitSet & check all
the callers/users?

- Dave


>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18578
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160413/b9e8fcff/attachment.html>