contain-rs / bit-set

A Set of Bits
Apache License 2.0
63 stars 25 forks source link

Move BitSet constructors from impl<u32> to impl<B> #6

Open polazarus opened 8 years ago

polazarus commented 8 years ago

The way things are, BitSet<B> for any B except u32 is almost unusable. The only constructor is pretty much hidden (default()). I propose to move the constructors around from the specialized implementation to the generic one. It may impact the performances for some use cases. I'm prepared to look into it. But first, I would love to hear your thought on this.

bluss commented 8 years ago

This is by design for now, to make BitSet easy to use for the usual case (where you don't care about which B is used).

There is no real nice way to provide a default B. When rust-lang/rust#27336 is turned on, this will be much easier.

polazarus commented 8 years ago

A default seems nice indeed. But I think functionality trumps convenience.

As it is, BitSet<B> is rather useless. I cannot create a full set for instance. The worst is for from_bit_vec which does not make sense: B could be inferred from the bit_vec itself.

bluss commented 8 years ago

BitSet::default() isn't really hidden. Only that the documentation doesn't point it out. Or anything else either ;)

bluss commented 8 years ago

As it is now, changing for example BitSet::new() to be generic over B will break compilation for some users, where B can not inferred. That's the kind of drawback waiting for the above linked feature will remove.

polazarus commented 8 years ago

As a middle ground, I would like to just move from_bit_vec. This way it would be possible, if a bit complicated, to create a set with capacity or with initialization.

bluss commented 8 years ago

Moving from_bit_vec looks simple enough. Note that BitVec has the same u32 specialization for its constructors as BitSet, for the same reasons.

polazarus commented 8 years ago

:cry: yes i didn't see that one. So there is no way to efficiently initialize a bit set (or a bit vector)...

Gankra commented 8 years ago

BitVec has these reverse dependencies (15): https://crates.io/crates/bit-vec/reverse_dependencies

BitSet has these reverse dependencies (9): https://crates.io/crates/bit-set/reverse_dependencies

If we do a 0.x version bump, it's arguable a sound "reasonable breakage". I had hoped that the default functionality would have been flipped on by now, but it seems there's been some technical difficulties.

pczarn commented 2 months ago

Waiting for a larger overhaul of bit-vec.