[boost] Re: Iterator adaptors <-> boost::bind interaction

From: Aleksey Gurtovoy (agurtovoy_at_[hidden])
Date: 2005-05-24 17:37:56

David Abrahams writes:
> Aleksey Gurtovoy <agurtovoy_at_[hidden]> writes:
>
>> It became way too easy to write
>> an innocently looking transformation that in fact invokes undefined
>> behavior -- and the latter is not even diagnosed by the most compilers
>> (neither at compile nor at run time). Consider, for instance, this:
>>
>>     #include <boost/iterator/transform_iterator.hpp>
>>     #include <boost/bind.hpp>
>>     #include <algorithm>
>>
>>     struct X
>>     {
>>         X( int i = 7 ) : i_( i ) {}
>>         ~X() { this->i_ = -1; }
>>         int const& get_i() const { return this->i_; }
>>  
>>      private:
>>         int i_;
>>     };
>>
>>     struct Y { X x; };
>>     X make_x( int i ) { return X( i ); }
>>
>>     int main()
>>     {
>>         int i[] = { 8 };
>>         Y y[1];
>>
>>         // undefined behavior #1 (returning reference to a temporary)
>>         int r = *boost::make_transform_iterator(
>>               boost::make_transform_iterator( i, std::ptr_fun( &make_x ) )
>>             , std::mem_fun_ref( &X::get_i )
>>             );
>>
>>         assert( r == 8 );
>>
>>         // undefined behavior #2 (returning reference to a temporary)
>>         r = *boost::make_transform_iterator(
>>               boost::make_transform_iterator( y, boost::bind( &Y::x, _1 ) )
>>             , std::mem_fun_ref( &X::get_i )
>>             );
>>
>>         assert( r == 7 );
>>     }
>>
>>
>> IMO this is way too subtle and error-prone to leave it as
>> is. Opinions?
>
> Well I'm afraid there's too much going on in that code for me to see
> where the problems are without some narrative.  

Understandable.

> You could take that as an argument that it's too subtle,

Yes, that's my line of argument. That, and the fact that the code like
this used to work as expected until the blend between transform and
projection iterators.

> but personally I would never write
> something that involved and "just expect it to work," so I don't see
> it that way.  

It's only "involved" because it's a minimal test case stripped of the
higher-level layers that make it usable in real life. Non-stripped
version would be something like:

    transform_view( 
          transform_view( y, boost::bind( &Y::x, _1 )
        , std::mem_fun_ref( &X::get_i )
        )

Is it more realistically looking for you, use case-wise?

> Maybe you could add some commentary?

Sure. Both of the marked lines above invoke undefined behavior on
dereference of the corresponding composite transform iterator because
its 'operator*' by the iterator is basically equivalent to this:

       int const& operator*() const
    // ^^^^^^^^^^
      {
          // returning a reference to a member of a temporary object 
          // that will be destroyed at the end of the function block scope
          return make_x( *base() ).get_i(); // #1
    //    return X( base()->x ).get_i(); // #2
      }

-- 
Aleksey Gurtovoy
MetaCommunications Engineering