boost::dereferenceable<>::operator->() problem
Recently two of our developers encountered the same problem with boost::dereferenceable<>::operator->(). There were an iterator which returned value_type from its operator*, not value_type&: value_type operator*() const; And it was inherited from boost::random_access_iterator_helper, which in turn is indirectly inherited from boost::dereferenceable. Since our iterator class template did not have its own operator->, one from boost::dereferenceable<> was inherited. And it is defined as follows: P operator->() const { return &*static_cast<const T&>(*this); } where P is pointer to value_type of corresponding iterator. Both of the compilers which we use (MSVC 6.0 and CodeWarrior) compile this code without any errors (though at least one of them did gave a warning). That lead to operator-> returning addres of a temporary object. What do you think about modifying operator-> so that it would be permissive to iterators for which operator* gives rvalue? E.g. this way: struct address_of_proxy { value_type value; address_of_proxy( value_type const& v ) : value( v ) { } operator value_type const*() const { return &value; } value_type const* operator->() const { return &value; } }; address_of_proxy operator->() const { return address_of_proxy( *static_cast<const T&>(*this) ); } where value_type is value_type of corresponding iterator. And of course it is possible to avoid creating proxy object when iterator's operator* returns reference, not value... -- Pavel Kuznetsov MetaCommunications Engineering http://www.meta-comm.com/engineering
"Pavel Kuznetsov" <pavel@despammed.com> writes:
Recently two of our developers encountered the same problem with boost::dereferenceable<>::operator->().
There were an iterator which returned value_type from its operator*, not value_type&:
value_type operator*() const;
And it was inherited from boost::random_access_iterator_helper,
A standard-conforming random access iterator must return value_type& from its operator*.
which in turn is indirectly inherited from boost::dereferenceable. Since our iterator class template did not have its own operator->, one from boost::dereferenceable<> was inherited. And it is defined as follows:
P operator->() const { return &*static_cast<const T&>(*this); }
where P is pointer to value_type of corresponding iterator.
Both of the compilers which we use (MSVC 6.0 and CodeWarrior) compile this code without any errors (though at least one of them did gave a warning). That lead to operator-> returning addres of a temporary object.
What do you think about modifying operator-> so that it would be permissive to iterators for which operator* gives rvalue?
E.g. this way:
struct address_of_proxy { value_type value; address_of_proxy( value_type const& v ) : value( v ) { } operator value_type const*() const { return &value; } value_type const* operator->() const { return &value; } };
address_of_proxy operator->() const { return address_of_proxy( *static_cast<const T&>(*this) ); }
I don't think we should pump up these old helper templates any more. Can you say "iterator_facade?" -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams:
A standard-conforming random access iterator must return value_type& from its operator*.
Yes, I know that. It does not relevant here, since boost::input_iterator_helper uses the same boost::dereferenceable implementation, and input iterators are not required to return references, so the problem is still there...
I don't think we should pump up these old helper templates any more. Can you say "iterator_facade?"
Ok, I got your point. But maybe then it might be a good idea to add some kind of compile-time assert to boost::dereferenceable<>::operator->()? -- Pavel Kuznetsov MetaCommunications Engineering http://www.meta-comm.com/engineering
"Pavel Kuznetsov" <pavel@despammed.com> writes:
David Abrahams:
A standard-conforming random access iterator must return value_type& from its operator*.
Yes, I know that. It does not relevant here, since boost::input_iterator_helper uses the same boost::dereferenceable implementation, and input iterators are not required to return references, so the problem is still there...
I don't think we should pump up these old helper templates any more. Can you say "iterator_facade?"
Ok, I got your point. But maybe then it might be a good idea to add some kind of compile-time assert to boost::dereferenceable<>::operator->()?
I'd be happy to evaluate your patch containing an assert. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David,
Ok, I got your point. But maybe then it might be a good idea to add some kind of compile-time assert to boost::dereferenceable<>::operator->()?
I'd be happy to evaluate your patch containing an assert.
Here it is. First, we should add: #include <boost/type_traits/detail/yes_no_type.hpp> #include <boost/static_assert.hpp> to the top of <boost/operators.hpp>. Then dereferenceable class template definition (around 300 line of the same file): template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { return &*static_cast<const T&>(*this); } }; should be changed as follows: namespace detail { template<class T> struct has_lvalue_const_dereference_ { private: template<class U> static ::boost::type_traits::yes_type discriminator_(U&(T::*)()const); template<class V> static ::boost::type_traits::no_type discriminator_(V(T::*)()const); static ::boost::type_traits::no_type discriminator_(...); public: BOOST_STATIC_CONSTANT(bool, value = sizeof( discriminator_( &T::operator* ) ) == sizeof( ::boost::type_traits::yes_type ) ); }; struct empty_base { }; } // namespace detail template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { BOOST_STATIC_ASSERT( ::boost::detail::has_lvalue_const_dereference_<T>::value ); return &*static_cast<const T&>(*this); } }; That's it, I think. -- Pavel Kuznetsov MetaCommunications Engineering http://www.meta-comm.com/engineering
"Pavel Kuznetsov" <pavel@despammed.com> writes:
David,
Ok, I got your point. But maybe then it might be a good idea to add some kind of compile-time assert to boost::dereferenceable<>::operator->()?
I'd be happy to evaluate your patch containing an assert.
Here it is.
Looks way too heavy to me for what it's accomplishing. How about: namespace error { template <class T, class Value> int dereferenceable_requires_lvalue_dereference(Value&); } template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { enum { assertion = sizeof( error::dereferenceable_requires_lvalue_dereference<T>( *static_cast<const T&>(*this) )) }; return &*static_cast<const T&>(*this); } }; ?? Does that work? -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David,
Here it is.
Looks way too heavy to me for what it's accomplishing. How about: namespace error { template <class T, class Value> int dereferenceable_requires_lvalue_dereference(Value&); } template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { enum { assertion = sizeof( error::dereferenceable_requires_lvalue_dereference<T>( *static_cast<const T&>(*this) )) }; return &*static_cast<const T&>(*this); } }; ?? Does that work?
Alas, that does not, since at least MSVC allows binding rvalue to non-const reference. But we can simplify original approach along these lines: namespace error { template <class T, class Value> int dereferenceable_requires_lvalue_dereference( Value& (T::*)()const); } template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { enum { assertion = sizeof( error::dereferenceable_requires_lvalue_dereference<T>( &T::operator* )) }; return &*static_cast<const T&>(*this); } }; -- Pavel Kuznetsov MetaCommunications Engineering http://www.meta-comm.com/engineering
"Pavel Kuznetsov" <pavel@despammed.com> writes:
Does that work?
Alas, that does not, since at least MSVC allows binding rvalue to non-const reference. But we can simplify original approach along these lines:
namespace error { template <class T, class Value> int dereferenceable_requires_lvalue_dereference( Value& (T::*)()const); }
template <class T, class P, class B = ::boost::detail::empty_base> struct dereferenceable : B { P operator->() const { enum { assertion = sizeof( error::dereferenceable_requires_lvalue_dereference<T>( &T::operator* )) }; return &*static_cast<const T&>(*this); } };
No good, I'm afraid. You can define a unary non-member operator*: struct X {}; int operator*(X const&); -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
participants (2)
-
David Abrahams -
Pavel Kuznetsov