[Boost-users] Re: iterator_adaptor: disguise iterator as const_iterator?

From: David Abrahams (dave_at_[hidden])
Date: 2004-12-01 11:44:44

Martin Wartens wrote:
> Hi all,
> I am trying to disguise a std container non-const iterator as a
> const_iterator. My guess is that iterator_adaptor could be useful for this,
> but I can't figure it out myself.
> 
> I am trying to solve an old problem with the standard library, which is
> that you can't convert a const_iterator into an iterator. [There are two
> "solutions" to that problem, one uses distance and is expensive, the other
> one does a brutal conversion on bitlevel which breaks for several std
> implementations.]

That's not the problem, and "fixing" it actually causes new 
const-correctness problems.  The actual problem is that the standard 
library doesn't allow const_iterators to be used as position indicators 
in mutating container operations.  The constness of the container should 
be enough to control whether you can insert.

That said, you may have to do this as a workaround for that interface 
deficiency.

> My use case: I have written a wrapper class mymap for std::map. I only want
> to give map::const_iterators to the users of this class. If the user wants
> to erase an element from the map, he must pass a const_iterator back to the
> wrapper class and call "mymap::erase(const_iterator)". The problem is that
> mymap has no way to convert the const_iterator into an iterator to call
> "map::erase(iterator)" in turn [there is no "map::erase(const_iterator)"].
> 
> So my idea was to write a iterator_adaptor for iterator that lets it behave
> like a const_iterator. Unfortunately I didn't get very far. I have to say
> that the documentation was not of much help to me. There are lots of
> complicated implementation details but only small examples. 

You're not kidding?  Did you go through the tutorial starting with 
iterator_facade (http://tinyurl.com/2gzpz#tutorial-example) which 
continues on to the iterator adaptor tutorial 
(http://tinyurl.com/58e2p#tutorial-example)?  Gee, it looks from the 
below like you did!

> So any help is appreciated. How can I solve my problem? 
> 
> Side note: In the following code sample, iterator_adaptor accidentally
> turns a const_iterator into a non-const iterator. (Seemingly, some
> bit-level casting is done somewhere). Is this a bug? 
> 
> #include <boost/iterator/iterator_adaptor.hpp>
> #include <map>
> template <class Value>
> class node_iter
> :public boost::iterator_adaptor<node_iter<Value>, Value>
                                                    ^^^^^
This thing needs to be an iterator; naming it Value is a bit confusing 
since that's normally used for the iterator's value_type.
> {
> public:
>     node_iter()
>       : node_iter::iterator_adaptor_(0) {}
> 
>     node_iter(Value p)
>       : node_iter::iterator_adaptor_(p) {}
> };  
 >
> int main()
> {
> 	typedef std::map<int, int> mymap_type;
> 	mymap_type testmap;
> 	node_iter<mymap_type::const_iterator> ni(testmap.begin());
> 	ni->second = 42; //this works, not a const_iterator anymore
> }

Wow, thanks for finding that case!  We're being bitten by the 
constructor/cast ambiguity.  In short,

    return T(a);

is fine when T is not a built-in type, but when it is, all sorts of 
nasty violations can occur.

So I've attached a partial fix (iterator_facade.patch, committed to 
CVS).  The reason it's only partial is that the "pointer" type of your 
iterator still comes out wrong.  So ending your program with

   return ni->second;

also fails to compile.  To get around that you can add 
-DBOOST_ITERATOR_REF_CONSTNESS_KILLS_WRITABILITY to your compiler's 
command line.

The question I have for Jeremy and Thomas is, what would we be giving up 
by enabling that always?  I think it means that some obscure corner 
cases would fail to work.

I think the minimal change needed to get it going is to change:

         typedef typename mpl::eval_if<
             detail::iterator_writability_disabled<ValueParam,Reference>
           , add_pointer<typename add_const<value_type>::type>
           , add_pointer<value_type>
         >::type pointer;

to:

       typedef typename mpl::eval_if<
           mpl::or_<
              detail::iterator_writability_disabled<ValueParam,Reference>
            , indirect_traits::is_reference_to_const<Reference>
           >
         , add_pointer<typename add_const<value_type>::type>
         , add_pointer<value_type>
       >::type pointer;

Though I am inclined to think we should just do the simple thing and 
turn on the #define always.  Thoughts?

-- 
Dave Abrahams
Boost Consulting
http://www.boost-consulting.com

--- iterator_facade.hpp.~1.30.~	2004-09-02 16:07:58.613356800 -0400
+++ iterator_facade.hpp	2004-12-01 11:18:43.026366400 -0500
@@ -7,8 +7,6 @@
 #ifndef BOOST_ITERATOR_FACADE_23022003THW_HPP
 #define BOOST_ITERATOR_FACADE_23022003THW_HPP
 
-#include <boost/static_assert.hpp>
-
 #include <boost/iterator.hpp>
 #include <boost/iterator/interoperable.hpp>
 #include <boost/iterator/iterator_traits.hpp>
@@ -16,6 +14,9 @@
 #include <boost/iterator/detail/facade_iterator_category.hpp>
 #include <boost/iterator/detail/enable_if.hpp>
 
+#include <boost/implicit_cast.hpp>
+#include <boost/static_assert.hpp>
+
 #include <boost/type_traits/is_same.hpp>
 #include <boost/type_traits/add_const.hpp>
 #include <boost/type_traits/add_pointer.hpp>
@@ -322,7 +323,7 @@
 
         static type make(Reference x)
         {
-            return type(&x);
+            return implicit_cast<type>(&x);
         }
     };