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.