Boost logo

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


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk