- Boost - lists.preview.boost.org

[Formal review] Collection Traits library
by hartmutkaiser＠t-online.de 08 May '04

08 May '04

Hi all, The review of the Collection Traits library, written by Thorsten Ottosen starts today (April 28th, 2004) and runs for 10 days (until May 7th, 2004). The Latest version of the library is available at (yahoo files sections): http://tinyurl.com/ysbaj (72 kB) or in the Boost sandbox. What is it? The Collection Traits library is mainly a tool for writing generic algorithms that work on External Collections. A Collection is a subset of the container requirements; it is so small and essential that it can be made to work with standard containers, iterator-views, built-in arrays and strings by defining a new requirement, the External Collection Concept, where typedefs and functions have been made free-standing instead of members. The library consists of two components: (1) type generators and (2) free-standing functions. The type-generators provide types such as iterators and size_type of external collections whereas the Free-standing functions are for extracting iterators and size of the external collections. Here is a small example: // Example: extracting bounds in generic algorithms template< typename ExternalCollection, typename T > inline typename boost::iterator_of<ExternalCollection>::type find( ExternalCollection& c, const T& value ) { return std::find( boost::begin( c ), boost::end( c ), value´); } template< typename ExternalCollection, typename T > inline typename boost::const_iterator_of<ExternalCollection>::type find( const ExternalCollection& c, const T& value ) { return std::find( boost::begin( c ), boost::end( c ), value´); } // replace first value and return its index template< typename EC, typename T > inline typename boost::size_type_of< EC >::type my_generic_replace( EC& c, const T& value, const T& replacement ) { boost::iterator_of<EC>::type found = find( c, v ); *found = replacement; return std::distance( boost::begin( c ), found ); } // usage std::vector<int> my_vector; typedef vector<int>::iterator iterator; std::pair<iterator,iterator> my_view( my_vector.begin(), my_vector.begin() + N ); char str[] = "a string"; // ... std::cout << my_generic_replace( my_vector, 4, 2 ) << my_generic_replace( my_view, 4, 2 ) << my_generic_replace( str, 'a', 'b' ); As the External Concepts idea is a rather new one, I would like to ask you to comment on this. Please always state in your review, whether you think the library should be accepted as a Boost library! Additionally please consider giving feedback on the following general topics: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? Regards Hartmut Review Manager

20 102

Re:[boost] [serialization] archive_exception bug
by Robert Ramey 08 May '04

08 May '04

Fredrik Blomqvist wrote: > The switch-case in class archive_exception lacks a 'break' after the > 'stream> error' case which leads to incorrect what() message. OK > I also note that the error code "invalid_class_name" doesn't have a > message > equivalent. OK > Furthermore I'm confused about this (which was the reason I found the > other > bug) : > (VC7.1 + Dinkumware) > > int main() > { > try > { > // save > { > std::ofstream ofs("serializer_test.txt"); > boost::archive::text_oarchive oa(ofs); > > oa << 'x'; > // ofs.close(); // uncommenting this throws "stream_error" > (altough > currently with wrong what() message) > oa << 'y'; > } > > // load > { > std::ifstream ifs("serializer_test.txt", std::ios::binary); > boost::archive::text_iarchive ia(ifs); > > char x, y; > ia >> x; > // ifs.close(); // uncommenting this _doesn't_ throw? > ia >> y; > > assert(x == 'x' && y == 'y'); > } > } > catch(std::exception const& e) > { > std::cout << e.what(); > } > > return 0; > } > > > Shouldn't reading throw also? The archives check is/os.good() to throw. > Or is this a bug in my std.lib (or my understanding of it? ;) > #1 looks correct to me. An attempt to read a closed stream should be an error. #2 looks to me like it should throw as well. One think, I'm checking os.good BEFORE trying to write. I don't check it afterwards. Maybe that's the issue. The reason I did this is that after reading eof - os.good() returns false - not really what we want. But at the point where we can check, we don't know that there isn't anything more to be read. Robert Ramey

1 0

[serialization] archive_exception bug
by Fredrik Blomqvist 08 May '04

08 May '04

Hi, The switch-case in class archive_exception lacks a 'break' after the 'stream error' case which leads to incorrect what() message. I also note that the error code "invalid_class_name" doesn't have a message equivalent. Furthermore I'm confused about this (which was the reason I found the other bug) : (VC7.1 + Dinkumware) int main() { try { // save { std::ofstream ofs("serializer_test.txt"); boost::archive::text_oarchive oa(ofs); oa << 'x'; // ofs.close(); // uncommenting this throws "stream_error" (altough currently with wrong what() message) oa << 'y'; } // load { std::ifstream ifs("serializer_test.txt", std::ios::binary); boost::archive::text_iarchive ia(ifs); char x, y; ia >> x; // ifs.close(); // uncommenting this _doesn't_ throw? ia >> y; assert(x == 'x' && y == 'y'); } } catch(std::exception const& e) { std::cout << e.what(); } return 0; } Shouldn't reading throw also? The archives check is/os.good() to throw. Or is this a bug in my std.lib (or my understanding of it? ;) // Fredrik Blomqvist

1 0

BUG: graphviz_write (boost_1_31_0)
by Mark Purtill 08 May '04

08 May '04

The graphviz_write function is inconsistent in how it writes vertices. When it write the vertex itself, it uses get(vertex_index, ...), but when it writes the edges, it just writes the vertex descriptor. When using adjacency_list< listS, listS, bidirectionalS,... >, this produces bad output (the edges list their endpoints by hexadecimal address values). Here's a patch: *** graphviz.hpp.orig 2004-05-07 12:00:21.000000000 -0700 --- graphviz.hpp 2004-05-07 11:34:11.000000000 -0700 *************** *** 266,272 **** } typename graph_traits<Graph>::edge_iterator ei, edge_end; for(tie(ei, edge_end) = edges(g); ei != edge_end; ++ei) { ! out << source(*ei, g) << Traits::delimiter() << target(*ei, g) << " "; epw(out, *ei); //print edge attributes out << ";" << std::endl; } --- 266,272 ---- } typename graph_traits<Graph>::edge_iterator ei, edge_end; for(tie(ei, edge_end) = edges(g); ei != edge_end; ++ei) { ! out << get(vertex_index, source(*ei, g)) << Traits::delimiter() << get(vertex_index, target(*ei, g)) << " "; epw(out, *ei); //print edge attributes out << ";" << std::endl; } -- ^.-.^ Mark Purtill <mpurtill(a)softwarerevolution.com> (425)284-2770 ((")) Software Revolution 11332 NE 122nd Way Kirkland WA 98034-6931

2 1

RE: [boost] Re: License : Graph library incompatw/Boost (was incompatw/GPL)
by Jon Kalb 08 May '04

08 May '04

> -----Original Message----- > From: boost-bounces(a)lists.boost.org > [mailto:boost-bounces@lists.boost.org] On Behalf Of David Abrahams > Sent: Friday, May 07, 2004 4:15 AM > To: boost(a)lists.boost.org > Cc: Andrew Lumsdaine; Lie-Quan Lee; Jeremy Siek > Subject: [boost] Re: License Conversion Update: Graph library > incompatw/GPL > Maybe this is all a distraction. I think it may be a much > bigger problem that the graph library's license, IIUC, seems > to be incompatible with the Boost license requirements. > Isn't anyone a little alarmed about that? > > I guess if there's a chance we can relicense it under the > Boost license, that'd solve things, but I'm not sure it's > possible. Can any of the copyright holders comment? Yes I'm alarmed about that. I looked at the graph library docs on the web site and they don't say anything at all about copyright or license (that I could find). I think we are (unintentionally) misleading people. People have a right to assume that any library that we have accepted has met the submission requirements which include the license requirements from http://boost.org/more/lib_guide.htm#License. We simply can't include a library that doesn't meet these requirements. I think we need to pull the graph library from the current release and put a note on the website explaining the situation until we get it resolved. I realize that this is a pretty drastic move, but I feel this situation calls Boost's integrity into question and that calls for swift and decisive action. How was this not caught during the review? -- Jon Kalb

4 3

Lambda Doc Bug
by Stephan T. Lavavej 08 May '04

08 May '04

http://boost.org/libs/lambda/doc/ar01s04.html#sect:storing_bound_arguments > [1] Strictly taken, the C++ standard defines for_each as a > non-modifying sequence operation, and the function object > passed to for_each should not modify its argument. The > requirements for the arguments of for_each are unnecessary > strict, since as long as the iterators are mutable, for_each > accepts a function object that can have side-effects on their > argument. Nevertheless, it is straightforward to provide > another function template with the functionality > of std::for_each but more fine-grained requirements for its > arguments. This is incorrect, as well as having a typo. for_each is described as "non-modifying" in the Standard because it does not inherently modify sequences (contrast copy() and friends). However, the functor it takes can mutate its argument. This footnote must be removed. Stephan T. Lavavej http://nuwen.net

1 0

looping construct - testers wanted ;)
by John Torjo 08 May '04

08 May '04

Dear boosters, In the "looping construct" thread, a while ago David Abrahams said that we should do a test, to compare all looping solutions. I've made such a test. It tests against: - keeping internal functions in crange<> - virtual functions - BOOST_FOREACH comparing to using raw iterators. The tests are pretty heavy. I've tested on a Win2000 box/512 Mb of RAM/1.8Ghz Intel processor (laptop), VC7.1 with all optimizations on. It seems that Dave, you were right about the virtual functions - the vtable can be cached. But it seems to happen only when the loop contents are pretty trivial (if within the loop there are some not-so-simple instructions, the vtable aren't cached - see the 'set_to_az' test). Please if any of you can spare about 15 minutes to run the tests, it would be great! Also, any improvement to the tests is welcome. I've attached my results and the code to test. Conclusions (mine :D): - BOOST_FOREACH rocks big time - vtables can be cached, in case the instructions within the loop are not too compilcated - the overhead is pretty large for both vtables and internal functions (at least for vc7.1) I'm eager to see how the tests run on different platforms. Best, John -- John Torjo Freelancer -- john(a)torjo.com -- http://www.torjo.com/logview/ - viewing/filtering logs is just too easy! ***************************************************************************** ** ** ** WARNING: This email contains an attachment of a very suspicious type. ** ** You are urged NOT to open this attachment unless you are absolutely ** ** sure it is legitmate. Opening this attachment may cause irreparable ** ** damage to your computer and your files. If you have any questions ** ** about the validity of this message, PLEASE SEEK HELP BEFORE OPENING IT. ** ** ** *****************************************************************************

7 15

[serialization] bug in basic_iarchive.cpp
by Fredrik Blomqvist 07 May '04

07 May '04

I noticed a typo in the assignment operator for the cobject_id class in libs/serialization/src/basic_iarchive.cpp class cobject_id { public: cobject_id & operator=(const cobject_id & rhs){ bis_ptr = rhs.bis_ptr; bisp_ptr = rhs.bisp_ptr; file_version = rhs.file_version; tracking_level = rhs.tracking_level; ==> initialized = initialized; // [line 101] return *this; } ......... }; This could perhaps be part of the bug/problems Martin Ecker had(?). // Fredrik Blomqvist

1 0

Re:Re:[serialization] cyclic references
by Martin Ecker 07 May '04

07 May '04

Robert Ramey wrote: > Francois Fath wrote: > > > I have difficulties when serializing two objects pointing to each other. > > The generated archive is fine (I can see both references in it), but when > > deserializing, only one is restored. In the attached example, I have a > > collection of nodes, each one is pointing to the previous and to the next > > node. When deserializing, only the ->next references are restored, all the > > ->previous are NULL. > > Hmmm - I would have expect to get a "pointer_conflict" exception when the > archive is being created - see test_cyclic_ptrs. Hm, this is unfortunate. MFC's serialization mechanism (via CArchive) supports these kinds of cyclic pointer relationships. I believe, it should be possible to extend boost::serialization to support this as well, especially since the written out archives seem to contain all that is needed to reconstruct the data structure upon deserialization. See my previous mail on this subject on what I think needs to be addressed to achieve this. Am I missing something here or are there other complications that maybe MFC's CArchive cannot deal with or simply disregards with respect to cyclic pointer structures? Best Regards, Martin Ecker

1 0

tuple - pair interoperability
by Fredrik Blomqvist 07 May '04

07 May '04

Hi, I think it would be convenient if one could treat std::pair as a boost two-tuple. Both tuples::tie and tuple assignment already work with std::pair but I've noticed that the get<> accessor doesn't. So, I propose a simple extension to tuples::get<> with the obvious semantics: boost::get<0>(p) == p.first boost::get<1>(p) == p.second The only reference I found in the archives was a post by Tomas Witt early 2002. "Tuple extension proposal": http://lists.boost.org/MailArchives/boost/msg25582.php Should I submit the code I've got or does something like this already exist somewhere? (I've tested the code using VC7.1, it doesn't rely on PTS although member-templates) Regards // Fredrik Blomqvist

4 6