moves-rwth / storm

A Modern Probabilistic Model Checker
https://www.stormchecker.org
GNU General Public License v3.0
138 stars 75 forks source link

Made BitVector conform to the std::ranges::range concept #622

Closed weiss-i-net closed 1 month ago

weiss-i-net commented 2 months ago

The BitVector iterators weren't default-construtable and were missing postfix increment operators and thus not fulfilling the range concept (https://en.cppreference.com/w/cpp/ranges/range). Changing this makes it possible to use BitVector with C++20s range adaptors. E.g. for code like this:

for (auto i : some_bitvector | std::views::filter(some_lambda)) {
  do_something(i);
}
volkm commented 2 months ago

Thanks for the PR.

I personally find it a bit strange that a default constructor is needed (which is not properly initialized), but this seems to be okay. Maybe add some static assert of the form static_assert(std::ranges::forward_range<BitVector>); to ensure that the rightf range concept is supported?

weiss-i-net commented 2 months ago

I had the same thought and read the same stack overflow thread :D

The static_assert is a good idea. What would be a good place to put it?

tquatmann commented 2 months ago

What would be a good place to put it?

I don't think we have a 'proper' place for these kind of things.
I'd suggest in BitVector.h right after the class declaration?

volkm commented 1 month ago

Looks good! Thanks for your contribution.