All - I'm posting this review (with permission of the author) that was not posted on the list during the review. This review was considered as part of the library acceptance. Anyway, the review may be of interest to other variant users as it is quite detailed and implements a novel use of variants...
Jeff -----Original Message----- Sent: Sunday, March 02, 2003 11:00 AM Dear boosters, just my two pence on the variant library. To evaluate the variant library, I implemented a variant_iterator type with the help of boost::iterator_adaptor and boost::variant. The main aspect here is that all iterator operators, such as dereference, increment, etc., must be implemented such that they work for the variant. For simplicity, I restricted myself to *two* iterator types; the general case with n types might be (much!) more complicated. For details, see the attached file. Environment: Win XP, MSVC++ 7.0, STLPort 4.5.3, Boost 1.29.0 + everything from Boost sandbox Issues I came across in no particular order: + I regard it a bad practice to put all the files either straight into the boost directory, or into boost/detail/variant. Since variant is no "simple library implemented within the library header", boost/variant and boost/variant/detail would be conforming to the Boost directory structure guidelines. For the same reason, I think everything should be put into namespace boost::variant. Further, I'd like to include everything in one line: #include "boost/variant/variant.hpp" + The equality operator is missing. I guess it occurs pretty often that someone has to test two variant<T0,...,Tn>'s for equality, i.e. that they are of same type and same value, and I think this should be fixed before being submitted to Boost. + So I had to write operator==() myself, and stumbled over another issue. Two variants are equal if the have same type and same value, so a hand-written version might look like: bool operator()( const boost::variant<T0,T1>& v0, const boost::variant<T0,T1>& v1 ) const { if( v0.which() != v1.which() ) return false; // same type? try { switch( v0.which() ) { case 0: // same value? return( boost::extract<T0>(v0)() == boost::extract<T0>(v1)() ); // etc... } } catch( boost::bad_extract& exc ) { /* etc... */ } } This implementation, however, throwed bad_extract's in any case. Changing the header to pass-by-value instead pass-by-reference, i.e. bool operator()( boost::variant<T0,T1> v0, boost::variant<T0,T1> v1 ) const solved the problem. Weird. + Even in cases when the two iterator types are different, derived types might be equal; for instance, std::vector<int>::iterator is different from std::list<int>::iterator, but std::vector<int>::value_type and std::list<int>::value_type are the same Since variant<T0,T1> requires T0 and T1 to be different, I got boost::variant<...>::preprocessor_list_initializer::initialize( void*,.. ): member function already defined or declared with ... a) Is it possible to issue an error message that is better understandable? b) Since I expect this issue to appear quite frequently, my wish would be to have a helper construct at hand which, for any type list T0,...,Tn, calculates a typle list S0,...,Sk were all duplicate types are removed. If all T0,...,Tn are the same, this might even boil down to the common type T. I had to write such a variant_helper for n=2, and I'm aware that the general case is more complicated. OK, that's only a wish, it might as well be delayed for any Boost 1.3x release. + Various typos in the docs; from the tutorial: "the closet possible type" ... "Naturally, If there" ... "no convesion is applied" A spell checker might be helpful. Conclusion: Provided that the equality operator for variant is added to the library, I vote "yes": variant should be accepted into Boost. After all, it seems stable enough (and definitely useful!), so I'll immediatetely start using it. - Roland
#include <iostream> #include <vector> #include <list> #include <boost/type_traits.hpp> #include <boost/iterator_adaptors.hpp> #include "boost/variant.hpp" #include "boost/apply_visitor.hpp" #include "boost/static_visitor.hpp" #include "boost/extract.hpp" struct do_increment: boost::static_visitor<void> { template<typename T> void operator()( T& t ) const { ++t; } }; struct do_decrement: boost::static_visitor<void> { template<typename T> void operator()( T& t ) const { --t; } }; template< typename Difference > struct do_advance: boost::static_visitor<void> { do_advance( const Difference& theN ): n ( theN ) { } template<typename T> void operator()( T& t ) const { t += n; } private: Difference n; }; template< typename Reference > struct do_dereference: boost::static_visitor<Reference> { template<typename T> Reference operator()( const T& t ) const { return *t; } }; template< typename VisitorT, typename T0, typename T1 > struct visit_helper { typedef typename VisitorT::result_type result_type; visit_helper( const VisitorT& theVisit = VisitorT() ) : visit( theVisit ) { } result_type operator()( T0& t0 ) { return visit( t0 ); } result_type operator()( const T0& t0 ) const { return visit( t0 ); } result_type operator()( boost::variant< T0, T1 >& v ) { return boost::apply_visitor( visit, v ); } result_type operator()( const boost::variant< T0, T1 >& v ) const { return boost::apply_visitor( visit, v ); } private: VisitorT visit; }; template< typename T0, typename T1 > struct equality_helper { bool operator()( const T0& t0, const T0& t1 ) const { return t0 == t1; } //### Using references caused check() to fail / a bad_extract() exception?! //bool operator()( const boost::variant<T0,T1>& v0, const boost::variant<T0,T1>& v1 ) const bool operator()( boost::variant<T0,T1> v0, boost::variant<T0,T1> v1 ) const { if( v0.which() != v1.which() ) return false; try { switch( v0.which() ) { case 0: { boost::extract<T0> extT0v0( v0 ); boost::extract<T0> extT0v1( v1 ); if( !extT0v0.check() || !extT0v1.check() ) { return false; } return( extT0v0() == extT0v1() ); } break; case 1: return( boost::extract<T1>( v0 )() == boost::extract<T1>( v1 )() ); break; default: return false; } } catch( boost::bad_extract& exc ) { std::cerr << exc.what(); return false; } } }; template< typename Iter0, typename Iter1 > class variant_iterator_policy: public boost::default_iterator_policies { public: // Constructors variant_iterator_policy() { } template< class AdaptedT > typename AdaptedT::reference dereference( const AdaptedT& x ) const { typedef do_dereference< typename AdaptedT::reference > do_dereference_type; return visit_helper< do_dereference_type, Iter0, Iter1 >()( x.base() ); } template< typename AdaptedT > void increment( AdaptedT& x ) { //boost::apply_visitor( do_increment(), x.base() ); visit_helper< do_increment, Iter0, Iter1 >()( x.base() ); } template< typename AdaptedT > void decrement( AdaptedT& x ) { //boost::apply_visitor( do_decrement(), x.base() ); visit_helper< do_decrement, Iter0, Iter1 >()( x.base() ); } template< typename AdaptedT > void advance( AdaptedT& x, typename AdaptedT::difference_type n ) { typedef do_advance< typename AdaptedT::difference_type > do_advance_type; visit_helper< do_advance_type, Iter0, Iter1 >( do_advance_type(n) )( x.base() ); } template<class AdaptedT1, class AdaptedT2 > bool equal( const AdaptedT1& x, const AdaptedT2& y ) const { return equality_helper< Iter0, Iter1 >()( x.base(), y.base() ); } private: }; template< class T0, class T1 > struct variant_helper { template< bool B > struct select { typedef boost::variant< T0, T1 > ret; }; template<> struct select< true > { typedef T0 ret; }; typedef select< boost::is_same< T0, T1 >::value >::ret type; }; template< typename Iter0, typename Iter1, typename Value0 = boost::detail::iterator_traits<Iter0>::value_type, typename Value1 = boost::detail::iterator_traits<Iter1>::value_type, typename Reference0 = boost::detail::iterator_defaults<Iter0,Value0>::reference, typename Reference1 = boost::detail::iterator_defaults<Iter1,Value1>::reference, typename Pointer0 = boost::detail::iterator_defaults<Iter0,Value0>::pointer, typename Pointer1 = boost::detail::iterator_defaults<Iter1,Value1>::pointer, typename Category0 = boost::detail::iterator_traits<Iter0>::iterator_category, typename Category1 = boost::detail::iterator_traits<Iter1>::iterator_category, typename Distance0 = boost::detail::iterator_traits<Iter0>::difference_type, typename Distance1 = boost::detail::iterator_traits<Iter1>::difference_type > struct variant_iterator_generator { typedef variant_helper< Iter0, Iter1 >::type base_type; typedef variant_iterator_policy< Iter0, Iter1 > policies_type; typedef variant_helper< Value0, Value1 >::type value_type; //### variant does NOT accept references //typedef boost::variant< Reference0, Reference1 > reference; typedef variant_helper< Value0, Value1 >::type reference; typedef variant_helper< Pointer0, Pointer1 >::type pointer; typedef variant_helper< Category0, Category1 >::type category; typedef variant_helper< Distance0, Distance1 >::type distance; typedef boost::iterator_adaptor< base_type, policies_type, value_type, reference, pointer, category, distance > type; }; int main( void ) { typedef std::list<int> U; typedef std::vector<int> V; U u; for( int i = 42; i < 51; ++i ) { u.push_back( i ); } V v; for( int i = 0; i < 10; ++i ) { v.push_back( i ); } typedef variant_iterator_generator< typename U::iterator, typename V::iterator, typename U::value_type, typename V::value_type, typename U::reference, typename V::reference, typename U::pointer, typename V::pointer, boost::detail::iterator_traits< typename U::iterator >::iterator_category, boost::detail::iterator_traits< typename V::iterator >::iterator_category, typename U::difference_type, typename V::difference_type >::type variant_iter_type; variant_iter_type it( u.begin() ); variant_iter_type e( u.end() ); for( ; it != e; ++it ) { std::cout << *it << " "; } std::cout << std::endl; it = variant_iter_type( v.begin() ); e = variant_iter_type( v.end() ); for( ; it != e; ++it ) { std::cout << *it << " "; } std::cout << std::endl; return 0; }
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost