On Wed, Aug 03, 2011 at 11:53:45AM +0100, John Maddock wrote:

Documentation from the library may be viewed online here: file:///M:/data/boost/sandbox/move/libs/container/doc/html/index.html

For you who do not have John's M: drive, the docs exist at: https://svn.boost.org/svn/boost/sandbox/move/libs/container/doc/html/index.h... -- Lars Viklund | zao@acc.umu.se

Hi, I think that this library is really necessary. I would like to use move and emplace_back also C++03. My small review is follow: - It seems that the default allocator type is not written. Default is std::allocator? Does not Boost.Container provide allocator type? - There are 2 patterns allocator type name. `A` and `Alloc`. It will be better to unite into either. - basic_string has not shrink_to_fit() member function. C++0x's basic_string has shrink_to_fit(). Thanks, Akira.

...

======================== Akira Takahashi mailto:faithandbrave@gmail.com http://d.hatena.ne.jp/faith_and_brave/

On 03/08/2011 12:53, John Maddock wrote:

Documentation from the library may be viewed online here: file:///M:/data/boost/sandbox/move/libs/container/doc/html/index.html

You probably meant <http://svn.boost.org/svn/boost/sandbox/move/libs/container/doc/html/index.html>

Review comments might like to answer the following questions:

* What is your evaluation of the design?

It's the same as the STL, with some very welcome enhancements from C++0x, all of which also work with a C++03 compiler.

* What is your evaluation of the implementation?

It's based on SGI STL code and Boost.Intrusive, both being very good. The move emulation seems a bit fragile, but Boost.Move was accepted, so I guess that is ok. I really like the fact that it avoids reinventing the wheel and reuses other libraries / implementations.

* What is your evaluation of the documentation?

Pretty good, I wonder if it couldn't become my new STL reference documentation.

* What is your evaluation of the potential usefulness of the library?

Move-enabled STL containers are extremely important. Whether it's useful to have them in Boost or not is of some debate, some might say it's better to rely on your standard library. I think it might be good for the people not quite ready to hop onto the C++0x bandwagon yet. Plus sometimes it's good to be able to rely on a single portable reference implementation that actually works with stateful allocators etc., while it is implementation-defined whether the standard one does.

* Did you try to use the library? With what compiler? Did you have any problems?

I ran the tests against trunk with GCC on linux without problems. I did not, however, try to use the library.

* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

A quick reading of the documentation and implementation.

* Are you knowledgeable about the problem domain?

I've been using the containers of the standard library extensively for a very long time, and I have followed a bit the C++0x developments.

And finally, every review should answer this question:

* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.

Yes. Those containers already exist as part of Boost.Interprocess, and I don't see any problem with them being promoted to their own library.

Hi, My review is here.

* What is your evaluation of the design?

I think it is important, emulating move semantics and emplace inserter.

* What is your evaluation of the implementation?

Some requirements added in C++0x (see 23.2.1 in n3290). Does Boost.Container satisfy, partial or not? (just question) I anxious about 23.2.1/8 especially. And associated to the requirements, do you have any plans to provide allocator_traits?

* What is your evaluation of the documentation?

1) There are no documentation about free functions. (operators, swap, ... 2) The documentation of deque::push_[back|front] are missing. 3) Some member functions are not documented about Returns/Effects, Complexity or Throws. (ex: after No.23 in basic_string) Please check it.

* What is your evaluation of the potential usefulness of the

library? Very good! It can replace STL container easily.

* Did you try to use the library? With what compiler? Did you have

any

problems? No, I only read documents and source code.

* How much effort did you put into your evaluation? A glance? A

quick

reading? In-depth study? A quick reading for C++03 developer. And a glance for C++0x developer.

* Are you knowledgeable about the problem domain?

I am using STL containers (including C++0x's one), but not expert about implementations and algorithms.

And finally, every review should answer this question:

* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. I think should be accepted it with improved documentations.

Thanks, -- TAKAHASHI Kohei College of Information Science, University of Tsukuba private mail: flast@flast.jp others: flast@ac-room.org s0911476@coins.tsukuba.ac.jp

On 8/3/2011 3:53 AM, John Maddock wrote:

o slist: the classic pre-standard singly linked list container."

In the forthcoming C++11 standard, this is called forward_list. -- Eric Niebler BoostPro Computing http://www.boostpro.com

2011/8/3 John Maddock <boost.regex@virgin.net>:

  *  What is your evaluation of the design?

Design is really good. Includes new functions from upcoming standard (like cend(), cbegin()). Library also contains all the required free functions (like getline(), swap()) and comparison operators. Really enjoyed the move semantics and placement insertion.

  * What is your evaluation of the implementation?

High quality implementation.

  * What is your evaluation of the documentation?

Documentation is full, however it is hard to read the reference section... I think that the design must be changed: 1) Functions names does not catch an eye. May be the should be bold? 2) Vector description requires info about vector<bool>. Does this implementation uses bool optimization? 3) Descriptions of containers must be on a separate pages and must be accessible through the table of contests. One click to get the info, instead of two clicks and a lot of scrolling.

  * What is your evaluation of the potential usefulness of the library?

flat_set and slist containers are very required. I'd love to use them in my projects. Boost.Interprocess will benefit from this library (Interprocess's STL containers and move implementation can be replaced with Boost.Containers - less amount of duplicate code will be maintained ) BGL can take advantage of this library

  * Did you try to use the library? With what compiler? Did you have any problems?

Tried to use it with gcc-4.5. Works good. Tested it with lexical_cast - works. No problems.

  * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

Looked through the sources and documentation for a few hours. Made a few tests.

  * Are you knowledgeable about the problem domain?

Yes.

  * Do you think the library should be accepted as a Boost library?

Yes. Boost.Interprocess and Boost.Graph libraries should take advantages of Boost.Containers. Many other projects will benefit from this library. Best regards, Antony Polukhin

On 8/3/2011 6:53 AM, John Maddock wrote:

The review of the containers library by Ion Gaztanaga starts today, to summarize the library:

"Boost.Container library implements several well-known containers, including STL containers. The aim of the library is to offers advanced features not present in standard containers or to offer the latest standard draft features for compilers that comply with C++03.

In short, what does Boost.Container offer?

* Move semantics are implemented, including move emulation for pre-C++0x compilers. * New advanced features (e.g. placement insertion, recursive containers) are present. * Containers support stateful allocators and are compatible with Boost.Interprocess (they can be safely placed in shared memory). *

The library offers new useful containers: o flat_map, flat_set, flat_multiset and flat_multiset: drop-in replacements for standard associative containers but more memory friendly and with faster searches. o stable_vector: a std::list and std::vector hybrid with random-access iterators that offers iterator stability in insertions and erasures. o slist: the classic pre-standard singly linked list container."

Documentation from the library may be viewed online here: file:///M:/data/boost/sandbox/move/libs/container/doc/html/index.html

The source may be accessed from the "move" directory of the sandbox SVN or downloaded from http://www.drivehq.com/web/igaztanaga/boost_container.zip. Note that this download contains a copy of the accepted, but not yet release, Boost.Move library - extract the zip over a copy of Boost-1.47 to get a full working copy.

This is a very late review of the Containers library. My review is based on reading the documentation rather than trying the library. First off I vote that the library should be accepted into Boost.

Review comments might like to answer the following questions:

* What is your evaluation of the design?

The design is straightforward.

* What is your evaluation of the implementation?

Did not look at the implementation.

* What is your evaluation of the documentation?

I think the documentation is good, but it is hard to find overviews of the separate containers. They are buried in the reference whereas I would like to see them as separate topics in the TOC. For the example in the "Emplace: Placement insertion" topic there is a 'non_copy_movable ncm;' in the main function but it is never used, which is confusing. I did get the idea of emplacement from the example and understand what it is. There is mention in the Introduction that "Containers support stateful allocators" but that appears to be all that is written about that topic. I would like to know what that is about. The "Recursive Containers" topic gives a nice example but does not explain what a recursive container entails and why a recursive container is not guaranteed by the standard. Some further information should be given in this topic. For all of the containers which have the same names as the std:: containers, it should be pointed out what their benefits are. if it is a matter of all those containers having move semantics, placement insertion, recursive containers, and stateful allocators, that should be mentioned. One would like to know why one should use the Containers library containers rather than the std:: containers without having to read the description of each container.

* What is your evaluation of the potential usefulness of the library?

The new containers are definitely useful. The containers which have the same names as the std:: containers need to make a case for their usefulness better than they currently do in the doc.

* Did you try to use the library? With what compiler? Did you have any problems?

No, I did not try it. My trust in Ion Gaztanaga's programming from his other work in Boost is pretty high. I will try out various containers when I have the chance to experiment with them.

* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

A moderate reading of the doc.

* Are you knowledgeable about the problem domain?

Yes. I have used and programmed some C++ like conatiners in various programming environments, even when I was not using C++, and consider the C++ standard library conatainers, iterators, algorithms superior to any other similar library in other languages I use ( Python, C++/CLI and C# .Net, Java ).

And finally, every review should answer this question:

* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.

Answered already. The library should be accepted into Boost. The new containers are very useful. I apologize for the late review and the fact that I did not get a chance to really use the library, but I will do so when I am not as busy as I have been on other programming chores. Eddie Diener