On Fri, Feb 24, 2012 at 2:51 AM, Rob Desbois <rob.desbois@gmail.com> wrote:
On 23 February 2012 14:39, Robert Jones <robertgbjones@gmail.com> wrote:
> I favoured the type traits magic approach, and publish the resulting value
> type as the
> value type of the iterator...
>
> typedef typename if_<is_const<NODE>, const typename NODE::value_type, typename NODE::value_type>::type value_type;

Thanks for the input Rob - see below for the solution I settled on.

On 23 February 2012 18:58, Jeffrey Lee Hellrung, Jr.
<jeffrey.hellrung@gmail.com> wrote:
> To reiterate Robert Jones' reply, yes, AFAIK, a const iterator should pass a
> const-qualified value_type to iterator_facade/iterator_adaptor, as implied
> by the constant node_iterator example in the docs:
>
> http://www.boost.org/doc/libs/1_48_0/libs/iterator/doc/iterator_facade.html#a-constant-node-iterator

Actually iterator_facade/iterator_adaptor's value type in the
tutorials is the _node_ type, not the node's value_type.

I never said anything to the contrary, AFAIK...

> On a related note, you should specify your reference type you pass to
> iterator_adaptor explicitly as the return type of your dereference member
> function. Otherwise I think you'll end up returning a reference to a
> temporary within operator*. I believe that's *actually* what the quoted
> error is trying to tell you.

Thanks - I've updated to use
node_iterator::iterator_adaptor_::reference as the return type from
dereference(). The docs show that this defaults to Value&.
The error message still stands though, since const
node_type::value_type is still not const.

Huh? I don't understand. "const node_type::value_type" *is* const-qualified.

IIRC, in your original post, your dereference member function returned an rvalue (based on the member function definition, not the actual return type). This suggests that the reference type you give to iterator_facade should be Value, rather than accepting the default of Value&.
 
I settled on implementing the boost::mpl::if_ method as shown by Rob,
which turned out to be a viable solution to the problem I faced, but
then after some more thinking and coding realised that I didn't
actually want the mutable iterator type to enable modification of the
data in the referenced node. Since this is an ordered tree, modifying
the data of a node is clearly a Bad Thing!
Feeling somewhat silly that I didn't recognise this before leaping
down the rabbit hole...what I now have is that Value is *always*
const:
  template< class NODE >
  node_iterator: public boost::iterator_adaptor<
     node_iterator<NODE>, // Derived
     NODE*, // Base
     const typename NODE::value_type // Value
  >

The reference type is then const typename NODE::value_type& as
expected and everything works fine. It was only after discovering that
std::set and friends' iterators can be the same type as the
const_iterator that this finally clicked!

Thanks for the guidance, nice to know I wasn't going completely off my
rocker with the mpl approach :-D
--rob

Okay, good to know you got things working, I just wanted to give you a head's up on double-checking to ensure you're not returning an lvalue reference bound to a temporary :)

- Jeff