- Boost - lists.boost.org

[review] Convert library
by Jeroen Habraken 12 Jun '14

12 Jun '14

Hi, As promised my review of the boost::convert library. Note that the questions were answered in a seemingly random order, thus I hope it all comes together. Secondly, being the author of the boost::coerce library [2][3] I may be a little biased at times, please forgive me. -- What is your evaluation of the design? There are two parts to this, the design of the backend and the design of the interface that the user predominantly sees. I have little to say on the first, the notion of multiple backend converters is a big step forwards and their interface is sane. I'm far less convinced when it comes to the user-facing interface as I believe the simple tasks should be simple and intuitive, whereas the harder tasks may be harder. The most trivial use of boost::convert, and boost::lexical_cast and boost::coerce simply don't compare in my opinion. boost::cstringstream_converter cnv; int i = boost::convert<int>::from("23", cnv).value(); versus int i = boost::lexical_cast<int>("23"); int j = boost::coerce::as<int>("23"); When the convert library makes it into boost I'd want to mentally deprecate lexical_cast as a library I should use, and always use convert instead. This means convert should have a default converter, which may well be backed by lexical_cast with an interface such as: int i = boost::convert<int>::from("23").value(); This still doesn't completely satisfy me as I'd like to write int i = boost::convert<int>::from("23"); but it'd be an improvement in my opinion. Now I'm aware lexical_cast doesn't have a non-throwing interface, and it probably never will, but I've solved the problem as follows in boost::coerce: int i = boost::coerce::as_default<int>("XXX"); int j = boost::coerce::as_default<int>("XXX", tag::hex()); int k = boost::coerce::as_default<int>("XXX", 23); int l = boost::coerce::as_default<int>("XXX", tag::hex(), 23); The former two returning the default-constructed int, 0, the latter two returning 23. That said, I'll be the first to admit the coerce interface isn't perfect, the fact that I need SFINAE to distinguish between as_default calls with two parameters, the second either being a tag (somewhat similar to your converters but not quite) or the default value goes to show just that. However I'd prefer int i = boost::convert<int>::from("23", cnv); int i = boost::convert<int>::from_default("23", cnv, 0); to the current interface using value_or(0). This is subjective though, and quite possibly a matter of taste. In summary, before this turns into too long a rant, I think the current user-facing interface can be improved substantially, but I'm well aware this is subjective. -- What is your evaluation of the implementation? I've only had a quick glance at the code and it still looks fine. The gripe I had two years ago regarding the use of reserved names (include guards prefixed with "__") has been fixed. A few comments though: In the boost::lexical_cast based converter there is a try { } catch (...) { } which I believe to be an overly broad catch as lexical_cast explicitly throws a boost::bad_lexical_cast. If in an unlikely event a std::bad_alloc were to be thrown for example I'd rather not have boost::convert hide it from me. In the std::strtol based converter errno isn't checked, and it must be noted that strtol accepts leading whitespace which may or may not be wanted. I've implemented what I believe to be a correct converter using strtol which checks for the leading whitespace in [0], but it comes with no guarantees as it's quite tricky indeed. The new C++11 wrapper std::stol and friends in [1] may be of use here, but unfortunately they only have a throwing interface. All converters are added to the main boost namespace which is crowded enough as it is -- hi boost::_1 --, and libraries should add their functionality to their own namespaces in my opinion. With the current design it's not possible to add them in the boost::convert namespace but I believe they should be in their own namespace, may I suggest boost::converter. It must be said that the first two comments are minor at best, the last regarding the namespace is more important but potentially quite trivial to fix. -- What is your evaluation of the documentation? I've found it to be very hard to write documentation as the library author as at some point you're so intimately familiar with your library that it's hard to get a feel for what the user needs. Whilst there are some rough edges I think the current documentation is decent enough for it not to pose a problem. Again a few comments: The motivation should probably be addressed first, not near the end. The printf and std::strtol based converters aren't documented, and the converters should probably be grouped into a sub-section along with an explanation of how to write your own. Last of all I'd like to see an examples directory showing the examples from the documentation, you already have a test/encryption.hpp which probably belongs there. -- What is your evaluation of the potential usefulness of the library? In the previous review I've argued the library its functionality should be added to boost::lexical_cast. Whilst I still hold that position for the previous version of the library I believe this is no longer the case for the present version, largely due to the added support for multiple backends. There would be no practical way for lexical_cast to add this, and I believe it to be very useful indeed. -- Did you try to use the library? With what compiler? Did you have any problems? I've tried the library with Clang 3.2 and 3.4, and GCC 4.6 and 4.8 on Mac OS X 10.9.3 and the examples from the documentation using the sstream converter compile and run fine. When I tried the printf_converter something as simple as: #include <boost/convert/api.hpp> #include <boost/convert/converter/printf.hpp> int main(int, char *[]) { return boost::convert<int>::from("23", boost::printf_converter()).value(); } failed to compile. It's missing includes as adding the following to converter/printf.hpp fixes the problem: #include <boost/mpl/find.hpp> #include <boost/mpl/vector.hpp> This probably indicates that you're trying to test too much in a single file, and had the includes included via a different boost library. In the previous review I noticed that boost::convert<int>::from('1', cnv).value() spew a rather long error message. This has since been fixed, the error message is now considerably shorter making it easier -- will it ever be easy with C++ -- to figure out what's going on. I must note here that the error message the latest versions of GCC produced is actually easier to figure out than the one Clang produced. -- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Something between a quick reading and an in-depth study, now probably leaning towards the latter. -- Are you knowledgeable about the problem domain? As the author of boost::coerce, which I've admittedly ignored for far too long, I'm familiar with the conversion domain. Consequently I've seen the pitfalls and endured the hardships in designing and implementing such a library, my utmost respect for having it reviewed not once, but twice! And finally: -- Do you think the library should be accepted as a Boost library? In my previous review this was a straight no, stating that the functionality should be added to boost::lexical_cast. With the current design supporting multiple backends this no longer holds true as I've stated in the evaluation of the potential usefulness of the library. Again, the library is quite useful, but, I so strongly disagree with the current user-facing interface that I can't straight-out accept it. Once the library is accepted this interface is set in stone and like marriage it's "speak now or forever hold your peace". As such I'm voting YES, CONDITIONALLY on the interface being simplified for the seemingly simple cases as most of my other comments were minor or quite trivial to fix. With that out of the way I'd like to propose the following if you're interested. Let's have a look at whether it may be possible to merge the functionally of boost::convert and boost::coerce, as they seem to have grown towards each other with the possibility to support multiple backends. Two years ago I'd never have proposed this, but I've mellowed a bit since then. You seem to have focussed on backends primarily backed by std::stringstream and friends whereas I have focussed on a backend backed by boost::spirit. These may end up complementing leading to a single library capable of supporting both, being more generic and powerful to the user when done right. Regardless of whether we end up going this way I'd love to have a discussion on the interface, either on the mailing list or offline as this is currently a major stumbling block for me. Jeroen [0] https://github.com/VeXocide/coerce/blob/master/libs/coerce/example/backend.… [1] http://en.cppreference.com/w/cpp/string/basic_string/stol [2] https://github.com/VeXocide/coerce/ [3] http://vexocide.org/coerce/

15 78

Even newer dependency report
by Peter Dimov 12 Jun '14

12 Jun '14

Shows significant progress. http://www.pdimov.com/tmp/report-d6c23cf/module-overview.html http://www.pdimov.com/tmp/report-d6c23cf/module-levels.html

2 3

a new library Global Object Manager submission interest check
by George Georgiev 11 Jun '14

11 Jun '14

Hi, I have code that deals with problems related to initialization of global objects (singletons). 1. Multi-threading - problems occurring in some singleton patterns based on the fact that local for a function or method static variables are not initialized in a thread safe manner 2. Initialization order - there is no multi-platform/multi-compiler standard way to control the global objects initialization order 3. Static linking of global objects code without need - some patterns result in global objects to be linked in the final executable, even if they are provided from static libraries and the code that uses them is not in use 4. Capability for easy replacement of a global implementation object with a mock object I was struggling with shutdown issues in complex c++ application for years in several different projects. With the solution I come up a few years ago I have no issues in any project I have used it. Please let me know if this is something of interest to the boost community. Note that the boost singleton pattern used for path_locale (boost filesystem path) is having the multi-thread issue I mentioned in point 1. This is a small test that demonstrates the problem: https://github.com/ggeorgiev/boost-filesystem-singletons/blob/master/filesy… Thanks, George

3 10

[range] why not adding conversion operator std::pair<FTI.FTI> to boost iterator_range
by Andreas Fabri 11 Jun '14

11 Jun '14

Wouldn't it be a good idea to add a conversion operator to the class iterator_range? This would allow to write functions returning a range, instead of a pair of iterators, but boost::tie would still work with it. Best regards, andreas -- Andreas Fabri, PhD Chief Officer, GeometryFactory Editor, The CGAL Project phone: +33.492.954.912 skype: andreas.fabri

3 4

[BGL]
by Pablo Madoery 11 Jun '14

11 Jun '14

Hello, I Have 2 graphs: graph1 and graph2, and two sets of edge descriptors. One set is associated to edges of graph1 and another set associated to edges of graph2. I don't understand why is it possible to access information about edges of graph2 by using graph1 and the edge descriptors of graph2. I realized of that because of an own mistake when using graphs. What I would like to know is why is that possible to do and if it should be an exception that can handle this situation. I send the code I was using to test this: #include <iostream> #include <boost/graph/adjacency_list.hpp> #include <boost/graph/graphviz.hpp> #include <fstream> #include "GraphInfo.h" using namespace std; using namespace boost; typedef adjacency_list<setS, vecS, bidirectionalS, VertexInfo, EdgeInfo> Graph; void printGraph(Graph graph, string file) { ofstream ofs(file.c_str()); dynamic_properties dp; dp.property("node_id", get(vertex_index, graph)); dp.property("label", get(&EdgeInfo::weight, graph)); write_graphviz_dp(ofs, graph, dp); } int main() { Graph graph1; Graph::vertex_descriptor a1 = add_vertex(graph1); Graph::vertex_descriptor b1 = add_vertex(graph1); Graph::vertex_descriptor c1 = add_vertex(graph1); Graph::edge_descriptor e1 = (add_edge(a1, b1, graph1)).first; Graph::edge_descriptor e2 = (add_edge(b1, a1, graph1)).first; graph1[a1].id = 10; graph1[b1].id = 20; graph1[c1].id = 30; graph1[e1].weight = 1; graph1[e2].weight = 2; Graph graph2; Graph::vertex_descriptor a2 = add_vertex(graph2); Graph::vertex_descriptor b2 = add_vertex(graph2); Graph::vertex_descriptor c2 = add_vertex(graph2); Graph::vertex_descriptor d2 = add_vertex(graph2); Graph::edge_descriptor e4 = (add_edge(c2, b2, graph2)).first; Graph::edge_descriptor e5 = (add_edge(b2, c2, graph2)).first; Graph::edge_descriptor e6 = (add_edge(d2, a2, graph2)).first; Graph::edge_descriptor e7 = (add_edge(a2, d2, graph2)).first; graph2[a2].id = 10; graph2[b2].id = 20; graph2[c2].id = 30; graph2[e4].weight = 13; graph2[e5].weight = 14; graph2[e6].weight = 15; graph2[e7].weight = 16; // WHY IS THIS POSSIBLE TO DO // IT IS ASKING FOR AN EDGE OF GRAPH2 BUT USING GRAPH1 ??? double weight = graph1[e7].weight; cout<<"weight: "<<weight<<endl; printGraph(graph1, "graph1.dot"); printGraph(graph2, "graph2.dot"); cout << "end" << endl; return 0; } Thank you very much

1 0

[pimpl] No documentation for pointer semantics
by Edward Diener 10 Jun '14

10 Jun '14

The documentation for "Pimpl with Value Semantics" has: "and was deployed in the previous chapter for the Pimpls with pointer semantics..." but there is no previous chapter about Pimpl with pointer semantics. From that point on the documentation of "Pimpl with Value Semantics makes no sense to me and I do not know what is being explained.

8 24

[Modularization] new Memory module
by Vicente J. Botet Escriba 10 Jun '14

10 Jun '14

|What about creating a Memory module containing| most of the things the standard defines in <memory> except maybe what is already in SmartPptr, e.g. ||* boost/interprocess/allocators/*|| |* boost/intrusive/pointer_traits.hpp Vicente |

5 22

Tick: Trait instrospection library for C++14
by paul Fultz 10 Jun '14

10 Jun '14

Hi, I developed a trait introspection library for C++14 based on Eric Niebler's concept checking, here: https://github.com/pfultz2/Tick So you can build a simple `is_incrementable` trait like this: TICK_TRAIT(is_incrementable) { template<class T> auto requires(T&& x) -> TICK_VALID( x++, ++x ); }; Then use it like this: static_assert(is_incrementable<T>(), "Not incrementable"); Also, you can validate the return of an expressions like this: TICK_TRAIT(is_equality_comparable) { template<class T, class U> auto requires(T&& x, U&& y) -> TICK_VALID( returns<bool>(x == y), returns<bool>(x != y) ); }; You can also match against another trait using placeholder expressions: TICK_TRAIT(is_incrementable) { template<class T> auto requires(T&& x) -> TICK_VALID( return<std::is_integral<_>>(x++), return<std::is_integral<_>>(++x) ); }; Any feedback would be appreciated. Also, I don't knw if there would be any interest in incorporating this into boost. Thanks, Paul

7 13

[config] RTTI detection on Intel C++
by Peter Dimov 10 Jun '14

10 Jun '14

I was going to prepare a simple pull request that basically added #if !defined(__RTTI) && !defined(__INTEL_RTTI__) && !defined(__GXX_RTTI) && !defined(_CPPRTTI) #define BOOST_NO_RTTI #define BOOST_NO_TYPEID #endif to intel.hpp, but then I saw that this file doesn't contain any logic for BOOST_NO_EXCEPTIONS, relying instead on common_edg.hpp to check __EXCEPTIONS. So I wonder whether the preferred approach wouldn't be to check __RTTI in common_edg.hpp instead. Opinions? There's also a slight complication here because I suspect that when Intel is in Microsoft mode, when RTTI is off it will still support typeid, as MSVC does; so just BOOST_NO_RTTI would need to be defined, and not BOOST_NO_TYPEID.

2 2

quickbook equivalent of <simplesect>?
by Peter Dimov 10 Jun '14

10 Jun '14

Does Quickbook support <simplesect> or an equivalent? This is useful for the 'Authors' sections in Core, which don't need to clutter the ToC.

2 9