On Monday 14 July 2014 10:20:41 Peter Dimov wrote:
Agustín K-ballo Bergé wrote:
As mentioned in the pull request, cases like the following: int i= 0; boost::reference_wrapper<int> r = boost::ref(boost::ref(i));
were performing "accidental collapsing", as it triggers an implicit conversion followed by a (possibly elided) copy construction.
Thanks.
This works because of another boost::ref 'feature', it returns a const reference_wrapper which allows the outer boost::ref to bind to it. That const return is probably a mistake, now that I think of it, but it's too late to change it now.
In fact, of the four ref/cref combinations, three happen to work as if reference collapsing overloads are present.
What makes this hard is that I think that Eric is right and that the reference collapsing overloads should never have been added to std::ref. It should be obvious that nobody would write code like the above on purpose; there's absolutely no point in calling boost::ref twice when one call would do. It's similar to something like
int x = std::cref( 5 );
which is also something that used to work and is broken now, and also something that nobody would write except by mistake.
The motivation for the reference collapsing overloads must come from examples taken from real code in which ref somehow gets applied to something that already is a reference_wrapper but not to ref(x) directly, and the code needs the result to be a flattened reference_wrapper for some reason. So far I've been unable to think of such an example. I wonder if anyone has ever encountered it. This feature looks like a typical committee invention to me.
I think Boost.Proto is one of the examples where undesired reference recursion happened. I say 'undesired' because I cannot see why somebody would want a reference to a reference. If not because of performance, conceptually there are no references to references in C++, and std::ref attempts to reflect that. Now that Boost.Proto does reference collapsing, reverting the change will break it again (I think). Do you think we could settle with a trait I proposed earlier and a prominent note in 1.56 release notes?