- Boost - lists.boost.org

GGL Review
by Phil Endecott 23 Nov '09

23 Nov '09

Dear All, Here is my review of GGL: I have attempted to try the same examples that I did for my Boost.Polygon review. Unfortunately limited time has made it impossible for me to be as thorough as I would have liked. Here is what I have done, and the issues that I encountered along the way. (You may want to look at my Boost.Polygon review for background. It can be seen here: http://thread.gmane.org/gmane.comp.lib.boost.devel/193204/focus=193758 ) My first exercise was a GUI application in which I wanted to merge a large number of small rectangles into a single more complex polygon. Unfortunately I failed at the first step with this: I wanted to teach the library about my existing types (point, box, etc), but how to do this is not properly documented. It seems that there are some macros (with not-Boost-style names) but their only mention in the documentation is in an example. Yes, I could probably have worked out what to do from the example, but I have limited time and furthermore I don't think it's acceptable to expect a library end-user to learn something relatively fundamental like this from an example. (From my brief look at the example, it does look as if this mechanism is significantly less verbose that what I had to do to teach Boost.Polygon about my types, which is a good thing.) So I moved on to my second example, which reads a file containing the borders of the states and provinces of the US and Canada and determines which ones contains points that the user enters. I implemented this using the geometry types provided by the library, so the question of adapting to types did not arise. This all worked quite smoothly, though I did encounter the following issues, mainly with the documentation, which I imagine could be corrected quite easily: - The docs do not say what the C template parameter in point_xy<T,C> is. - The "complete list of members" for ggl::linear_ring is empty. (Presumably the issue here is that the members are all inherited from the std::container; it could at least say that.) - The documentation for ggl::linear_ring says that V is a "container type e.g. std::vector, list, deque"; it should describe this in terms of std concepts (presumably bidirectional container) so that I know if my own custom type will work. Looking at the code I don't see any concept-checking for this; that would be nice. - The args to the within() function are reversed wrt the equivalent function in Boost.Polygon. This is a good example of the issue that I raised in a previous email, where the generic nature of the concisely-named functions can lead to mistakes. - Doxygen doesn't give the full path to the headers, i.e. "point_xy.hpp" rather than "geometries/point_xy.hpp". - There seems to be a dependency on Boost.Math that isn't mentioned on the list of dependent libraries. - The library doesn't seem to be in namespace boost. I guess the intention is to add this later. This is unusual. - I was surprised that the box class doesn't have width and height members. - I got a page full of warnings from my test program; the cause seemed to be unused parameters in within.hpp, lines 118 and 178. Since I have touched only a small fraction of the library, I imagine there are other places with the same issue. As I said, these are minor issues that I imagine could be corrected quickly. However they did give me the impression that the library is still very much a work-in-progress - especially the documentation - rather than a fully-formed finished work. I don't necessarily believe that a library has to be perfect before it can be accepted into Boost, but I think it needs to be a little less rough around the edges than this. That unfortunately was all of the actual testing that I have been able to do. I'll now discuss other issues: Firstly there's the situation concerning Boost.Polygon. My feeling is that the situation that has developed here is very unfortunate. I think it's extraordinary that Boost can even consider having two incompatible, overlapping Geometry libraries reviewed within a few weeks of each other, with authors who are so publicly antagonistic towards each other. How on earth did we get into this situation? (Answer: IMO, the progress towards review flowed to the current situation by following the path of least resistance, and "someone" - a "leader" ? - should have jumped in earlier to steer things in a more constructive direction.) Anyway, Boost.Polygon has now been accepted (within an hour of this review starting! I mean, was that calculated to be antagonistic?) and we have to consider what that implies. One option is to imagine the two libraries in parallel universes. That may be the most stress-free way. But it's not what would deliver the best results for Boost's end users. What I would prefer to see is some work to investigate how more common ground can be introduced. For example: - Common terminology. ("within" vs. "contains", "extents" vs. "envelope", and dozens more. GGL's not-unreasonable adoption of OGC terminology is an issue here.) - Inter-operable types. I would like to hope that the basic geometry traits classes could be shared. Failing that, perhaps a method of forwarding from one set of traits to the other could be used. I am unsure how deeply the choice of tag dispatch vs. SFINAE makes inter-operation more difficult. Maybe someone could comment on that. Let me say a little about performance. There has been a great deal of discussion about this on the list. My feeling is that all libraries should document the worst-case complexity of all of their functions. In the current case we have polygon boolean operations where I believe I learnt as an undergraduate 20 years ago that they should be O(n log n). Here IIUC the implementation is worst-case O(n^2) but typically better. I feel uncomfortable with that, however many benchmarks and years of real-world use have gone in to it. Is it not possible to contrive an introsort-like "fallback" solution that gives both good typical case performance and an optimally-bounded worst case guarantee? The other controversial implementation issue is floating-point robustness. I'm still not clear what the position of this library is on that. Are we just given assurances that it seems to be robust in real-world use, or is it believed that it actually is certainly robust? I expected to see this discussed in the library documentation, but I don't see anything - maybe I've missed it. If it is "believed to be robust in real-world use", it would be helpful to describe the possible failure modes (e.g. segfault, runs forever, or just outputs the wrong answer.) In summary: Congratulations to the authors on getting their library to this point. I'm sure that it contains much useful code. At this time, the library is let down by incomplete documentation, and I feel it should be rejected for that. I believe it is also essential that some effort is done to find common ground (I'm avoiding saying "merge") with Boost.Polygon. If the authors are unable to work together, then I would prefer that Boost keep things simple for its end users, and avoid years of antagonism on the mailing list, but suggesting that one (or other!) library should exist outside of Boost. To answer the usual questions: - What is your evaluation of the potential usefulness of the library? Very useful. - Did you try to use the library? With what compiler? Yes; gcc-4.x on Linux. - How much effort did you put into your evaluation? I've spent about 5 hours in all. - Are you knowledgeable about the problem domain? I'm not a geometry expert, but I have done projects that could make use of this sort of library. Regards, Phil.

16 51

Lots of issues with Boost.Test in Trunk?
by John Maddock 23 Nov '09

23 Nov '09

I'm getting an absolute ton of warnings - of the kind that are normally errors - from Boost.Test while working on Boost.Integer: compile-c-c++ ..\..\..\bin.v2\libs\integer\test\integer_test.test\msvc-9.0\debug \threading-multi\integer_test.obj integer_test.cpp d:\data\boost\trunk\boost/test/impl/execution_monitor.ipp(941) : warning C4535: calling _set_se_translator() requires /EHa d:\data\boost\trunk\boost/test/impl/execution_monitor.ipp(1128) : warning C4273: 'boost::execution_monitor::catch_signals' : inconsistent dll linkage d:\data\boost\trunk\boost/test/execution_monitor.hpp(190) : see previous definition of 'catch_signals' d:\data\boost\trunk\boost/test/impl/execution_monitor.ipp(1199) : warning C4273: 'boost::execution_monitor::execute' : inconsistent dll linkage d:\data\boost\trunk\boost/test/execution_monitor.hpp(173) : see previous definition of 'execute' d:\data\boost\trunk\boost/test/impl/execution_monitor.ipp(1340) : warning C4273: 'boost::execution_exception::execution_exception' : inconsistent dll linkage d:\data\boost\trunk\boost/test/execution_monitor.hpp(125) : see previous definition of '{ctor}' d:\data\boost\trunk\boost/test/impl/execution_monitor.ipp(1348) : warning C4273: 'boost::execution_exception::location::location' : inconsistent dll linkage d:\data\boost\trunk\boost/test/execution_monitor.hpp(117) : see previous definition of '{ctor}' d:\data\boost\trunk\boost/test/impl/debug.ipp(637) : warning C4273: 'boost::debu g::under_debugger' : inconsistent dll linkage and so on. Normally, I would put this down to a problem with Boost.Integer's Jamfile, but in this I see nothing wrong. Any ideas anyone? Thanks, John.

4 6

[serialization] [1.41] serialization of shared_ptr<> fails to compile
by Marco Jez 23 Nov '09

23 Nov '09

Hello, I'm still having troubles with Boost.Serialization in Boost 1.41. The following example code fails to compile with latest Boost release, while it works with 1.39: #include <boost/archive/binary_iarchive.hpp> #include <boost/serialization/shared_ptr.hpp> #include <boost/shared_ptr.hpp> #include <iostream> // dummy struct with no actual serialization code struct Test { template<typename A> void serialize(A &, unsigned) {} }; int main() { boost::archive::binary_iarchive ia(std::cin); boost::shared_ptr<Test> x; ia & x; return 0; } The compiler (MSVC 9) gives me this error: error C2440: 'return' : cannot convert from 'const boost::serialization::extended_type_info_typeid<T>' to 'const boost::serialization::extended_type_info *'c:\boost\include\boost-1_41\boost\archive\shared_ptr_helper.hppIt seems that an address-of (&) operator is required in shared_ptr_helper.hpp at line 111, that is: ... return boost::serialization::singleton< ... should read: ... return &boost::serialization::singleton< ... This change makes my code compile, but I'm not sure whether it's an appropriate fix or not, as I don't know Serialization's internals. Cheers, Marco

2 1

[test] Boost.Test broken in Trunk
by John Maddock 23 Nov '09

23 Nov '09

Folks, Boost.Test is currently completely broken in Trunk for msvc: compile-c-c++ ..\..\..\bin.v2\libs\test\build\msvc-9.0\debug\asynch-exceptions-on\threading-multi\exception_safety.obj exception_safety.cpp d:\data\boost\trunk\boost/test/detail/unit_test_parameters.hpp(54) : error C2059 : syntax error : '__declspec(dllexport)' d:\data\boost\trunk\boost/test/detail/unit_test_parameters.hpp(55) : error C2059 : syntax error : '__declspec(dllexport)' and: d:\data\boost\trunk\boost/test/impl/progress_monitor.ipp(44) : error C2039: 'log_sink' : is not a member of 'boost::unit_test::runtime_config' d:\data\boost\trunk\boost/test/impl/progress_monitor.ipp(44) : error C3861: 'log_sink': identifier not found Obviously this completely breaks regression testing for msvc as well :-( Strangely, I can't see how any of the recent fixes could have caused this either? John.

2 2

Does anyone use the IBM pSeries?
by Marshall Clow 23 Nov '09

23 Nov '09

We've got a really old bug (#583) with patches to support V8 of this compiler. Looking for someone to apply these patches to a local boost install, and report back on the results.... https://svn.boost.org/trac/boost/ticket/583 -- Marshall

1 0

Using units prefix dynamically to get the appropriate prefix for the size of the value?
by Paul A. Bristow 23 Nov '09

23 Nov '09

I'd like to use the units library prefix (k, M, G ...) system, but I'd really like to determine the prefix dynamically (automatically) depending on the value. For example, if I have a length of 12300 m I'd like an output of "12.3 km" but if my length is 0.0123 m, I'd like an output of "12.3 mm". I can't see how to do this (and including the correct length unit of m or meter of course). Suggestions? Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow(a)hetp.u-net.com

2 3

Possible regression in Boost.Test
by Ian Emmons 23 Nov '09

23 Nov '09

A previously reported issue in Boost.Test, which the bug tracking system indicates as fixed, is still present in the 1.41.0 release. The issue I am speaking of is entitled "Boost:Test pointer error with g++ 64-bit on Mac OS X 10.6 Snow Leopard", discussed here: https://svn.boost.org/trac/boost/ticket/3432 It looks to me like one of two things happened here. Either the bug was fixed on a branch but the branch was never merged into the release, or a merge conflict overwrote the change before it was ever released. In 1.38.0 and 1.39.0 the method in question looked like this: void clear() { while( !m_test_units.empty() ) { test_unit_store::value_type const& tu = *m_test_units.begin(); // the delete will erase this element from map if( test_id_2_unit_type( tu.second->p_id ) == tut_suite ) delete (test_suite const*)tu.second; else delete (test_case const*)tu.second; } } The ticket claims to have fixed the bug like so: void clear() { while( !m_test_units.empty() ) { test_unit_store::value_type const& tu = *m_test_units.begin(); // the delete will erase this element from map if( test_id_2_unit_type( tu.second->p_id ) == tut_suite ) { test_suite const* p = (test_suite const*)tu.second; delete p; } else { test_case const* p = (test_case const*)tu.second; delete p; } } } But in 1.40.0 and 1.41.0 the method looks like this: void clear() { while( !m_test_units.empty() ) { test_unit_store::value_type const& tu = *m_test_units.begin(); // the delete will erase this element from map if( ut_detail::test_id_2_unit_type( tu.second->p_id ) == tut_suite ) delete static_cast<test_suite const*>(tu.second); else delete static_cast<test_case const*>(tu.second); } } Perhaps whoever switched the C-style cast to static_cast removed the local pointer variable and reintroduced the bug? In any case, the bug needs to be re-fixed, in the next release, and I would suggest adding a comment so that a future maintainer doesn't eliminate the apparently pointless local pointer variable, like so: void clear() { while( !m_test_units.empty() ) { test_unit_store::value_type const& tu = *m_test_units.begin(); // the delete will erase this element from map // Important: The pointer MUST be copied to local // storage in order to avoid the issue described in // https://svn.boost.org/trac/boost/ticket/3432 if( ut_detail::test_id_2_unit_type( tu.second->p_id ) == tut_suite ) { test_suite const* p = static_cast<test_suite const*>(tu.second); delete p; } else { test_case const* p = static_cast<test_case const*>(tu.second); delete p; } } }

2 1

[thread] unlock on multiple lockables
by Vicente Botet Escriba 23 Nov '09

23 Nov '09

Hi, I don't know if I have already requested that but ... Is there any deep reason to don't provide the equivalent of the free functions lock, try_lock Non-member function lock(Lockable1,Lockable2,...) Non-member function lock(begin,end) Non-member function try_lock(Lockable1,Lockable2,...) Non-member function try_lock(begin,end) to unlock multiple lockables? Non-member function unlock(Lockable1,Lockable2,...) Non-member function unlock(begin,end) Thanks, Vicente -- View this message in context: http://old.nabble.com/-thread--unlock-on-multiple-lockables-tp26471789p2647… Sent from the Boost - Dev mailing list archive at Nabble.com.

5 5

Re: [boost] Meta State Machine library review starts Monday
by Christophe Henry 23 Nov '09

23 Nov '09

>Is there any direct link for an online documentation? Yes, in the Sandbox, which will also be more up to date than the one in the Vault: http://svn.boost.org/svn/boost/sandbox/msm/libs/msm/doc/index.htm

1 0

[1.41.0] [integer] integer.hpp does not compile in MSVC 8.0 with /Za option on
by Adam Badura 23 Nov '09

23 Nov '09

An empty CPP file containing only inclusion of <boost/integer.hpp>: --------------- #include <boost/integer.hpp> --------------- does not compile in MS Visual Studio 8.0 Standard when /Za option ("Disable Language Extensions") is used. The output is following: --------------- D:\Libraries\boost_1_41_0\boost/integer_fwd.hpp(70) : error C2039: 'long_long_type' : is not a member of 'boost' D:\Libraries\boost_1_41_0\boost/integer_fwd.hpp(70) : error C2065: 'long_long_type' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer_fwd.hpp(70) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_fwd.hpp(73) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(67) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(73) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(79) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(85) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(119) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(126) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(132) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(138) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(144) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(150) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer_traits.hpp(156) : error C2913: explicit specialization; 'boost::integer_traits' is not a specialization of a class template D:\Libraries\boost_1_41_0\boost/integer.hpp(88) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(94) : see reference to class template instantiation 'boost::int_max_value_t<MaxValue>' being compiled D:\Libraries\boost_1_41_0\boost/integer.hpp(89) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(90) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(91) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(101) : error C2065: 'const_min' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(107) : see reference to class template instantiation 'boost::int_min_value_t<MinValue>' being compiled D:\Libraries\boost_1_41_0\boost/integer.hpp(102) : error C2065: 'const_min' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(103) : error C2065: 'const_min' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(104) : error C2065: 'const_min' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(116) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(122) : see reference to class template instantiation 'boost::uint_value_t<Value>' being compiled D:\Libraries\boost_1_41_0\boost/integer.hpp(117) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(118) : error C2065: 'const_max' : undeclared identifier D:\Libraries\boost_1_41_0\boost/integer.hpp(119) : error C2065: 'const_max' : undeclared identifier --------------- I suppose this happens to other editions as well. Also it seems that the same happens with 2005 version. 1.40 version had the same problem and I reported it but it seems no one cared about it. Adam Badura

6 13