Boost logo

Boost :

From: Peter Dimov (pdimov_at_[hidden])
Date: 2004-12-06 17:02:50


Sean Parent wrote:
> On Dec 6, 2004, at 12:10 PM, Peter Dimov wrote:
>>
>> This is not a bug. Bind was deliberately changed to return by value
>> because of a bug report similar to the following:
>>
>> pair_t make_pair( int, int );
>>
>> bind( &pair_t::first, bind( make_pair, 10, 20 ) )();
>>
>> The inner bind returns by value (because make_pair returns a value)
>> and the outer bind returns a reference to a temporary.
>>
>> A similar, but rare, situation can arise when bind is invoked on an
>> rvalue:
>>
>> pair_t const make_pair();
>>
>> bind( &pair_t::first, _1 )( make_pair() );
>>
>> I'm more interested in the actual code that was broken by this
>> change, because the assert() above was never required to pass.
>> By-value or by-reference was (and probably still is) an open
>> question. Of course I should have gotten it right in the first place.
>
> The documentation explicitly states that it will return a const
> reference:
> -----
>> bind(&X::f, args)
>>
>> is equivalent to
>>
>> bind<R>(mem_fn(&X::f), args)
>>
>> where R is the return type of X::f (for member functions) or a const
>> reference to the type of the member (for data members.)
> -----

Yes, you are right. I missed that.

> The actual failure was in conjunction with boost::function() where it
> fails silently because of a cast to result type inside boost function.
> Here is a snippet that was my first attempt at isolating the problem:
>
> ----
> #include <boost/bind.hpp>
> #include <boost/function.hpp>
> #include <utility>
> #include <cassert>
>
> int main()
> {
> typedef std::pair<int, int> pair_t;
>
> pair_t pair(10, 20);
>
> boost::function<const int&(const pair_t&)>
> function(boost::bind(&pair_t::first, _1));
>
> const int& x = function(pair);
>
> assert(&pair.first == &x);
>
> return 0;
> }

I get warning C4172 from VC++ 7.1, "returning address of local variable or
temporary", on this example (in function_template.hpp:111).

The example can be further simplified to:

#include <boost/function.hpp>

int f()
{
    return 5;
}

int main()
{
    boost::function<int const & ()> g( f );
    int const & r = g();
}

which produces the same warning.

A static_assert in boost::function would probably help on compilers that
don't emit a warning.

[...]

>> The explicit version:
>>
>> const int& x = boost::bind<int&>(&pair_t::first, _1)(pair);
>>
>> should work on conforming compilers.
>
> Good to know - I will review my code and try to be explicit where
> possible - however, I use bind() a lot within templates and rely on it
> not making an additional (often expensive) copy of my members. I don't
> think there there is a correct answer for the default behavior
> (between returning a const& and a value) but the docs need to be
> consistent - and a change of this nature needs to be called out in
> the release notes.

Yes, you are right. I underestimated the impact of this change; the new
behavior was supposed to eliminate this sort of bug, not introduce it.
Breaking existing code is never pleasant. The change has been in the CVS for
some time, but unfortunately most real-world testing occurs on the actual
releases.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk