Re: [boost] [Boost.Move] A few notes

Subject: Re: [boost] [Boost.Move] A few notes
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2012-01-12 01:40:34

On Wed, Jan 11, 2012 at 9:29 AM, Dan Ivy <danivy.mail_at_[hidden]> wrote:

> Hi,
>
> I was recently trying out Boost.Move and a few issues worth sharing
> surfaced:
>
> 1. It would be helpful to have configuration macros to force emulation
> mode, even on C++11 compilers, as well as to disable move semantics
> altogether (that is, the conversion operators to boost::rv& should be
> disabled, and move/forward should return lvalue-references. BOOST_RV_REF
> and friends should remain intact, so that overloads remain unique.)
> In many cases, the higher level semantics of a program are expected to be
> identical under all three configurations, so having a quick way to switch
> between them is useful during testing/debugging.
>

It could be useful to force emulation mode, I suppose, but disabling move
semantics altogether could easily break code that uses
moveable-but-noncopyable types. So I question the utility of the latter, at
least.

2. Boost.Move is a little bit too opaque, as it stands. What's really
> missing are Boost.Move-aware type traits. Things like add_rvalue_reference
> are often necessary to calcuate return types of move-aware generic
> functions, and so on. Whether this belongs in Boost.TypeTraits or
> Boost.Move is a separate question. Likewise, there should be type traits to
> calculate the return types of boost::move and, in
> particular, boost::forward. On C++11, the return type of forward<T>
> conincides with add_rvalue_reference<T>, but not so in emulation mode,
> hence the necessity for this trait.
>

+1; perhaps Boost.TypeTraits can update the relevant metafunctions to
account for boost::rv<T>&? I think we definitely should have

add_reference
add_rvalue_reference
add_lvalue_reference
remove_reference
remove_rvalue_reference
remove_lvalue_reference
is_reference
is_rvalue_reference
is_lvalue_reference

where boost::rv<T>& and (probably) boost::rv<T> const & are treated as T&&.

> 3. For some reason, the emulated boost::move is written so it doesn't
> accept temporaries. This doesn't play too nicely with forwarding:
> Unavoidably on C++03, there has to be made a choice whether forwarding
> functions use non-const or const-qualified references (bad wording, but you
> get the point), where the former accepts modifiable lvalues and rejects
> temporaries, and the latter does the opposite (well, sort of). Boost.Move
> chooses the latter. This means that while BOOST_FWD_REF parameters accept
> temporaries,  these temporaries are bound as lvalues, which pretty much
> defeats the purpose of the library (note that these temporaries aren't
> eligible for copy-elision, since they're bound to a reference.)

I wouldn't use such strong language. It's "just" more limited than if true
rvalue references were available. You can still pass explicitly created
emulated rvalue references, and the library will allow you to move them.

While
> there's not much you can do about it on the callee-side, callers could use
> boost::move on the temporaries to force them into rvalue-refs. Only
> that they can't. This creates a somewhat confusing situation where
> forwarding functions are able to take temporaries, but they won't move
> them, while the only things that can actually be moved are lvalues...
>

Okay, when you phrase it that way, it is confusing :)

> Ultimately, this is what I suggest:
> First of all, have boost::move accept temporaries.

Using a T const & parameter? I might prefer to remain safe to prevent
accidentally moving lvalues-to-const.

Secondly, seriously
> consider changing the definition of BOOST_FWD_REF(T) from const T& to T&,
> as this would acheive two desirable, IMO, goals::
> a. Forwarding functions would accept modifiable lvalues (and keep them
> modifiable).
> b. Forwarding functions would REJECT temporaries, UNLESS they're passed
> through boost::move, which assures that they're treated as rvalues.
>

This makes the use of the interface defined using BOOST_FWD_REF more
onerous, *even* if true rvalue references are available! So I would
consider this inferior to the current solution, even if the current
solution is more limited. And, sometimes, you don't even know if the
expression you're using as a parameter to a function of said interface is
an rvalue or an lvalue (and sometimes it could be either, if this is all in
a template).

I sympathize with your goal, though. I've actually played around with
various modifications of Boost.Move during its development process, and I
made some suggestions for additions to Boost.Move which were ultimately
rejected to keep the library simple (which is totally understandable). Two
such suggestions were:

- An additional macro BOOST_FWD2_REF( T ) which expanded to T& (in
emulation mode) and would be used only to capture lvalues or explicitly
created (emulated) rvalue references. This included the result of
boost::forward.
- An additional macro BOOST_MOVE( expr ) which expanded to an expression
nearly functionally identical to expr except it would explicitly cast the
result to an emulated rvalue reference if expr was an rvalue of moveable
type.

> A few notes on the implementation:
>
> 4. There are many one-liner functions floating around that aren't declared
> inline. This is a big-deal for less-capable compilers, such as Sun and
> older versions of GCC. In fact, they simply won't inline boost::move calls
> without it.
>

Sounds like you should file trac tickets and, possibly, patches.

5. boost::rv<T> unconditionally inherits from T, with the assumption
> that it would never get instanciated for non-class types, since it is only
> ever used as a reference. This is a false assumption in general. In the
> context of overload-resolution, the compiler is allowed, though not
> required, to instanciate the types of function parameters, even if the best
> overload can be determined without doing this. Consider this move-aware,
> but not very useful vector class:
>
> #include <boost/move/move.hpp>
>
> template <typename T>
> struct vector {
>    void push_back(const T&) {}
>    void push_back(BOOST_RV_REF(T)) {}
> };
>
> int main() {
>    vector<int> v;
>    v.push_back(123);
> }
>
> The last line of main could potentially instanciate rv<int>, breaking the
> program. And it does. Sun falls right into this trap. It doesn't take much
> to make it break on GCC either, for the same reason.
> Fortunately, the fix is trivial: specialize boost::rv correctly for
> non-class types.
>

Hmmm...again, probably best to file a trac ticket, with patch if possible.

> All comments refer to the 1.48 release. I don't know what's going on on
> trunk.
>

I haven't check recently, either.

- Jeff