Re: [container] Contributing 'stationary_vector' to Boost

From: Mehrdad Niknami (mniknami_at_[hidden])
Date: 2022-06-13 06:51:15

Hi Andrzej,

Thank you for the feedback. Yes, I think the process is slightly different
for adding to existing libraries. I believe Ion was interested in getting
this reviewed & then merged—he just didn't have the bandwidth.
Unfortunately I'm not sure what the process is at that point—is there
anyone else with the authority to review & merge code into Boost.Container?
I'm not sure whom to reach out to to be honest.

Regarding your notes, I can elaborate a bit on some of the design and try
to clarify some portions of it, though this may be easier to do in more
depth directly with those reviewing the code:

I understand that the library offers different trade-offs to STL containers
> which one might find sometimes useful. But I am not sure when that would
> be. I would expect the docs to tell me when I would prefer this container
> to others already present in the STL.
>

I actually did write up a documentation on this—apologies for neglecting to
commit it to the repo. You can now find this information here
<https://github.com/mehrdadn/libvital/blob/master/doc/cpp/vital/container/stationary_vector.boost.md>
.

The docs say, stationary_vector is "semantically, it is *almost* a drop-in
> replacement for std::vector, except [...]". This is a misleading
> statement. "Almost" says nothing more than that it is actually not a
> drop-in replacement for  std::vector.
>

What "almost" means here is that the container interface is almost exactly
the same as that of std::vector, with the exception of contiguity. There
are numerous practical uses of std::vector that never convert it to a raw
pointer or require contiguity, and this container can be used as a drop-in
replacement in such cases.

THe basic guarantee that std::vector provides is that it stores a
> contiguous array of elements underneath and that I can access it and pass
> it to a C-style function that works on raw arrays:
>
> std::vector<char> vec {'a', 'b', 'c', 'd'};
> c_function_that_takes_pointer_and_length(vec.data(), vec.size());
>
> I don't think stationary_vector can offer that. For that reason, I am not
> sure if "vector" is a good name for the container.
>

Boost.Container already has *stable_vector*
<https://www.boost.org/doc/libs/1_79_0/doc/html/container/non_standard_containers.html>
which
is even more discontiguous than stationary_vector.
Personally, I'm not aware of this having caused any confusion for users,
but if this has happened, I am open to suggestions for better names. (I
have been unable to think of any names that describe the container better
thus far, unfortunately.)

The docs also say, "Exception-safety is intended to be on par with that of
> std::vector (if not better). I have tried to ensure this, but it has
> undergone limited testing." This does not seem comforting.
>

What I meant was, I am not aware of specific benchmarks for this purpose
(hence the wording), but I have tried to test it to the extent that I can.
If you are aware of any particular benchmarks I would be more than happy to
profile the performance of the container on them.

"Exception safety" cannot be equal or better than that of another library.
> It is not even clear what you mean by this.
>

It can definitely be better—for example, this would occur if (for example)
another library only provides a basic exception safety guarantee for some
function, and this library provides a strong exception safety guarantee for
that same function.

It is not libraries that offer exception safety, but functions that offer
> guarantees as to what state the objects are left in when a function that
> modified them reported an error. It also seems to be saying that the author
> does not know if the library functions are exception safe.
>

I am not unaware of them, though I can see how it may give that impression.
I simply have not (yet) documented them.

The best course of action would be to *document* the exception safety of
> the functions in the library and provide automatic tests for demonstrating
> that these guarantees are satisfied. How to test this is described at:
> https://www.boost.org/community/exception_safety.html
>

Thank you for the link! I'd never seen that link before. Currently I have
relied on the tests provided by various standard libraries (in the section
quoted below). I will have to look into that implementation and see how
adaptable it is to this container; hopefully I can integrate it.

Regarding documentation: yes! I will definitely be happy to have all the
details documented.

"*Testing* for single-threaded use is done against test suites for
> std::vector from standard libraries"
> -- how can they pass, given that stationary_vector doesn't guarantee a
> contiguous layout of elements?
>

Of course those tests are excluded. Only applicable tests are run.

If you have any suggestions on whom I may be able to reach out to for the
review process for Boost.Container, I'd appreciate it!

Best,
Mehrdad