Added reference_proxy and bitset_view

[JohanMabille]

No description provided.

[jjerphan]

A first pass.

[Klaim]

My preference goes to this_type but ok

[Klaim]

This needs to be explicit.

[JohanMabille]

This requires that you explicitly cast each time you want to compare the proxy to a value (for instance), which is paricularly ugly (and does not looks naturaly at all to me):

template <class T>
struct array_impl
{
    using reference = reference_proxy<std::uint8_t>;
    const_reference = const std::uint8_t&;

    using size_type = std::size_t;

    reference operator[](size_type i);
};

array<std::uint8_t> a = ....;
uint8_t expected = 8u;
// With explicit you need to
using const_reference = array<std::uint8_t>::const_reference;
bool b = (const_reference(a[i]) == expected);

// while without, you can simply write:
bool b = a[i] == expected;

How having this operator implicit can be harmful?

[Klaim]

How having this operator implicit can be harmful?

See this guideline for the fundations of why it's a very bad idea. It goes in pair with the guideline about conversion operator that must also be explicit. The cases where this would not work as so rare I, so far, never saw a justifyiable example that isn't improved by explicit.

This requires that you explicitly cast each time you want to compare the proxy to a value (for instance), which is paricularly ugly (and does not looks naturaly at all to me):

That just means that the reference type needs comparison operators with objects of the type being referred to. That conversion is a shortctut that can cause more issues than explicitly stating these operators.

Also note that even like that "ugly" is not a reason to make the code potentially silently incorrect, in particular in a C++ library supposedly trying to make correctness easier than the C api. Worst scenario, static_casts are fine and help clarify where are the potential issues related to conversion. They are noisy for a reason after all.

See as example: https://godbolt.org/z/7jxcvjq3Y

I see no reason to make that operator implicit.

[Klaim]

I forgot a test in my example, here is the udpated version (I only added the test): https://godbolt.org/z/TYx7sGezj

[Klaim]

Could you add some documentation for each one of these types?

[Klaim]

How about begin and end? Is this not a range/container?

[JohanMabille]

yeah they should be, and I want to refactor this hierarchy to be consistent with the dynamic bitset. We can add it in an incoming PR though, to keep this one quite "small" (it is actually already big)

[Klaim]

Consider adding TODOs for these so that we dont forget in further passes.

[Klaim]

const

[Klaim]

I would prefer a separate default constructor which calls the explicit constructor with 1 parameter. That we we dont have an explicit default constructor.

[Klaim]

[jjerphan]

Another pass.

[jjerphan]

Shouldn't this be used instead?

Suggested change

	std::vector<buffer<block_type>> buffers;
	std::vector<array_data> child_data;
	buffer<buffer<block_type>> buffers;
	buffer<array_data> child_data;

[jjerphan]

As an alternative.

Suggested change

	std::fill(data(), data() + size, value);
	std::fill_n(data(), size, value);

[jjerphan]

Alternatively, one can use:

Suggested change

	std::fill(data() + old_size, data() + n, value);
	std::fill_n(data() + old_size, n, value);

[jjerphan]

Should we move it above?

[jjerphan]

Suggested change

	CHECK_EQ(b2.size() , b3.size());
	CHECK_EQ(b2.size(), b3.size());

[jjerphan]

Isn't the comment sufficient?

[JohanMabille]

Converted to draft to avoid accidental merge. This PR will be split into many smaller ones.

[jjerphan]

Can this PR be closed now?