On Tue, Dec 10, 2019, 18:21 Barrett Adair wrote:

Dear Boost,

The formal review of Zach Laine's STLInterfaces library begins now, and will run through December 19. Please participate in this review if you can. To submit a review, please reply to this email with the following information: - Your name - Your knowledge of the problem domain - Whether you believe the library should be accepted into Boost (be clear about this)

In addition, you are strongly encouraged to answer the following questions: - What is your evaluation of the library's * Design? * Implementation? * Documentation? * Tests? * Usefulness? - Did you attempt to use the library? If so: * Which compiler(s)? * What was the experience? Any problems? - How much effort did you put into your evaluation of the review?

STLInterfaces is a C++14 library targeting ISO standardization. The following templates are provided, all C++20-friendly:

1. iterator_interface - a modern version of the iterator_facade and iterator_adaptor parts of Boost.Iterator 2. view_interface - a pre-C++20 implementation of C++20's eponymous feature 3. container_interface - a tool to eliminate boilerplate when writing new containers

We would appreciate answers to these library-specific questions: - Do you like the name container_interface? sequence_container_interface is more precise, but seems a bit long. - Would you use something like container_interface, but for associative containers? If so, should it assume a node-based interface, a la std::map and std::set? This assumption would preclude alternative associative container-like types, such as flat_map.

Code: https://github.com/tzlaine/stl_interfaces Docs: https://tzlaine.github.io/stl_interfaces/doc/html/index.html

Only four days are left in this review, and there are no reviews or comments received so far. Please consider reviewing this library if you have any experience in this domain.

-----Original Message----- From: Boost On Behalf Of Barrett Adair via Boost Sent: 11 December 2019 00:21 To: boost@lists.boost.org Cc: Barrett Adair Subject: [boost] [review] [STLInterfaces] STLInterfaces review starts today

Dear Boost,

The formal review of Zach Laine's STLInterfaces library begins now, and will run through December 19. Please participate in this review if you can. To submit a review, please reply to this email with the following information: - Your name - Your knowledge of the problem domain - Whether you believe the library should be accepted into Boost (be clear about this)

In addition, you are strongly encouraged to answer the following questions: - What is your evaluation of the library's * Design? * Implementation? * Documentation? * Tests? * Usefulness? - Did you attempt to use the library? If so: * Which compiler(s)? * What was the experience? Any problems? - How much effort did you put into your evaluation of the review?

STLInterfaces is a C++14 library targeting ISO standardization. The following templates are provided, all C++20-friendly:

1. iterator_interface - a modern version of the iterator_facade and iterator_adaptor parts of Boost.Iterator 2. view_interface - a pre-C++20 implementation of C++20's eponymous feature 3. container_interface - a tool to eliminate boilerplate when writing new containers

We would appreciate answers to these library-specific questions: - Do you like the name container_interface? sequence_container_interface is more precise, but seems a bit long. - Would you use something like container_interface, but for associative containers? If so, should it assume a node-based interface, a la std::map and std::set? This assumption would preclude alternative associative container-like types, such as flat_map.

Code: https://github.com/tzlaine/stl_interfaces Docs: https://tzlaine.github.io/stl_interfaces/doc/html/index.html

This proposal must be worth consideration, but are Boosters suffering from review fatigue after the long discussions on static_string (and about to be distracted by seasonal activities? 😉 ) Extend review period? Paul

On 11.12.19 01:21, Barrett Adair via Boost wrote:

Dear Boost,

The formal review of Zach Laine's STLInterfaces library begins now, and will run through December 19. Please participate in this review if you can. To submit a review, please reply to this email with the following information: - Your name

Rainer Deyke

- Your knowledge of the problem domain

I'm hardly an expert on containers or iterators, but I've written a STL-compatible container or two.

- Whether you believe the library should be accepted into Boost (be clear about this)

Yes, it should be included.

In addition, you are strongly encouraged to answer the following questions: - What is your evaluation of the library's * Design?

Seems fine. I have a few nits to pick, but nothing that would hold up the inclusion of the library in Boost. Nit 1: I don't like 'bool Contiguous = discontiguous'. If a symbolic name like discontiguous is provided, then it should be part of an enum and not a plain boolean. This prevents the name from being misused in other contexts. Nit 2: It seems to me that, with a bit of effort, container_interface could do much more to make containers easier to write. For example: - X::iterator can be inferred from begin(). - X::reference is just X::value_type &. - operators < and == can be provided, since the requirements of these functions already suggest a default implementation. The user can still override the provided implementation if a more efficient implementation is possible. If it is for some reason desirable to create a container without operator <, an additional policy argument could be added to container_interface. If there's a good reason for container_interface not to provide these, then this reason should be documented.

* Implementation?

Didn't look at it. I assume that it's fine, or that any issues with it are too subtle for me to find.

* Documentation?

There are two types of tables with different columns in the container_interface documentation: those that document user requirements and those that document the functionality provided by container_interface. This can make reading difficult. If I just want to write a container I am only interested in the user requirements, so I have to skip every other table. If I am searching for specific functions (using the text search functionality of my browser) to see if container_interface provides it, I have to mentally switch between the two table types. It might be better to either merge the two table types, or to group all tables of the same type together. I think it should be clarified that although container_interface provides no help in making containers allocator-aware, it can still be used for writing allocator-aware containers. It is simply up to the user to provide all allocator-related functionality. (At least I assume that's the case?)

* Tests?

Didn't look.

* Usefulness?

Fairly useful. I don't see myself using it often, it would definitely be useful for the few times when I do need to write a custom container or a custom iterator.

- Did you attempt to use the library?

No.

- Do you like the name container_interface? sequence_container_interface is more precise, but seems a bit long.

Since I'm only going to be writing out the full name of the template once per container, I think I prefer the longer, more precise name.

- Would you use something like container_interface, but for associative containers? If so, should it assume a node-based interface, a la std::map and std::set? This assumption would preclude alternative associative container-like types, such as flat_map.

I can see the usefulness of such a template, although I have no immediate use for it. I would prefer that it not assume a node-based interface. -- Rainer Deyke (rainerd@eldwood.com)

On Tue, Dec 10, 2019 at 4:21 PM Barrett Adair via Boost wrote:

The formal review of Zach Laine's STLInterfaces library begins now

I've asked a couple of folks I know to submit reviews, hopefully we will see some new reviews shortly.

My mini-review will focus on the container_interface. On Tue, Dec 10, 2019 at 7:21 PM Barrett Adair via Boost < boost@lists.boost.org> wrote:

- Your name

Krystian Stasiowski

- Your knowledge of the problem domain

I enjoy implementing generic containers, and dabble in the subject.

- Whether you believe the library should be accepted into Boost (be clear about this)

I vote to ACCEPT.

- What is your evaluation of the library's * Design?

Overall, its pretty solid. One notable thing that comes to mind is that this should be made SFINAE friendly, ie disable certain overloads when the required functions are not defined in the derived class. Additionally, providing a destructor is a big no-no in my opinion, and could lead to elements having their destructor called twice. Infact, the documentation says that the user must provide a destructor that destroys all the elements of the container, but the destructor provided by the container_interface clears the container anyways, which leads to UB.

* Implementation?

This implementation is pretty solid.

* Documentation?

The documentation is _alright_. One thing I noticed is there is no cohesive list of which functions are provided by the interface when others are implemented by the user. It makes it quite a pain to navigate and even confusing.

* Usefulness?

I see this being useful for implementations of containers that don't have to be extremely optimized. For example, the implementation of `clear` provided by the implementation is not optimal for trivial types

- Did you attempt to use the library? If so: * Which compiler(s)?

MSVC.

* What was the experience? Any problems?

I would say that my experience with the library was quite good. To test it out, I opted to rewrite Vinnie's boost::json::array container using container_interface to see how much it would really simplify writing containers. It mostly consisted of removing code as expected.

- How much effort did you put into your evaluation of the review?

I spent about an hour and a half messing with the implementation.

Hello, this is my mini review of STLInterfaces.

- Your name

Daniela Engert

- Your knowledge of the problem domain

I've written some iterators and containers in the past in our companies in-house production code base. Currently I am about to write another container with very particular features where this library would be very handy by saving me from writing much boilerplate code.

- Whether you believe the library should be accepted into Boost (be clear about this)

I vote to *CONDITIONALLY* ACCEPT the library after fixing the problem that I will point out below

In addition, you are strongly encouraged to answer the following questions: - What is your evaluation of the library's * Design? It looks solid and pretty much matches my expectations in this regard * Implementation?

 - I'm not too fond of the BOOST_STL_INTERFACES_DOXYGEN clutter, but I can live with it - there are two instances of '#if 0' in container_interface.hpp which should be removed - I like it pretty much. In particular I want to point out that this is one of the few Boost libraries that caused no compiler warnings in msvc 14.1 at warning level 4 from the get-go.

* Documentation?

It's ok. If one is familiar with may other Boost library documentations she will eventually find what one is looking for. The uninitated might find it hard to get into this stuff and dismiss the library as too mysterious to him to be useful.

* Tests? The library is lacking the integration into Boost's test framework. Therefore I didn't run any tests. Sorry, no CMake on this computer. * Usefulness? High. I really want to use it. - Did you attempt to use the library? If so: * Which compiler(s)? VS2019 with msvc 19.2.4,  msvc 19.1.9, msvc 19.0.3 * What was the experience? Any problems?

I have been playing with the static_vector example as a starter because it engages most parts of the library. It doesn't compile with VS2015 (msvc 19.0.3) because of problems with the 'noexcept()' operator related to function overloading. Even without this problem it wouldn't compile because of the unconditional use of __has_include which is not part of C++ 14! ---- Condition 1 for acceptance: make use of __has_include conditional. There are conforming compilers that don't implement it. Or consider the removal of the optional dependency on 3rd-party non-Boost libraries. ---- I had no problems with VS2017 (msvc 19.1.9)  in both debug and release mode. I didn't check older versions of msvc 19.1.x With the current version of VS2019 (i.e. 19.4) I experience the following problems:  - the static_vector example doesn't compile in C++ 14 mode because of the missing include of <tuple> (std::tie in line 222 is undefined). Alas, when compiled in C++ 17 mode all is fine.  - the same source also compiles in C++ 2a mode when the compiler is instructed not to reveal the true value of __cplusplus. But when it does (by means of /Zc:__cplusplus) hell breaks lose. In this case the compiler defines __cplusplus to 201705L and __cpp_lib_concepts to 201806L. This enables the compilation of the section of lines 557-695 in iterator_interface.hpp and the section of lines 698-996 in container_interface.hpp, thereby exposing  -- the syntax error in line  731 of container_interface.hpp  -- the use of entities from the empty namespace 'v2_dtl' in various places in container_interface.hpp (first occurence in line 750)  -- the compilation failure in iterator_interface lines 618-621, most likely due to the problem that Andrzej has already pointed out in https://github.com/tzlaine/stl_interfaces/issues/13 It doesn't matter if the v1 or the v2 interface is used here (for obvious reasons). ---- Condition 2 for acceptance: address the syntactical problems Condition 3 for acceptance: address the missing entities from namespace 'v2_dtl' Condition 4 for acceptance: fix the compilation failures when C++ 20 concepts are enabled (like with msvc19.2.4 in C++ 2a mode and full conformance turned on) and the respective code paths are engaged. ----

- How much effort did you put into your evaluation of the review?

About 5 hours studying the documentation and implementation, and playing with various versions of msvc to figure out all of the problems mentioned above. Ciao    Dani