Boost logo

Boost :

Subject: Re: [boost] [move][container] Review Request (new versions of Boost.Move and Boost.Container in sandbox and vault)
From: Jeffrey Hellrung (jhellrung_at_[hidden])
Date: 2009-08-23 04:04:55


Ion Gaztañaga wrote:
> ...new versions of libraries Boost.Move and Boost.Container.

Few comments on proposed Boost.Move:

- Make all metafunctions true MPL metafunctions (as mentioned in
previous postings).

- BOOST_COPYABLE_AND_MOVABLE definition scopes relative to the global
scope, while other macros seems to scope relative to the local scope
(e.g., the difference between ::boost::rv<TYPE> and boost::rv<TYPE>).
I'm a stickler for consistency, and would probably consider
::boost::rv<TYPE> et al to be safer.

- What is the rationale for placing BOOST_COPYABLE_AND_MOVABLE and
BOOST_MOVABLE_BUT_NOT_COPYABLE in the private section of a class
definition? IIRC, BOOST_ENABLE_MOVE_EMULATION was suppose to go in the
public section, and I'm generally wondering what the motivations are for
specifying public or private.

- Even though this can be gleaned from the code, I'd like to see a "How
it works" section, e.g., how exactly does the library coerce rvalues
into binding to the BOOST_RV_REF overload of operator= rather than the
BOOST_COPY_ASSIGN_REF overload? Perhaps more generally, I think some
rationale for why this particular emulation strategy was chosen over
others would be nice (e.g., the previous proposed Boost.Move used a
by-value operator= to assign from rvalues efficiently; why the change?
How does this compare to Adobe's move emulation?). In most other
respects, I think the documentation is very good and complete.

- I would've renamed BOOST_COPY_REF_N_TEMPL_ARGS to
BOOST_COPY_ASSIGN_REF_N_TEMPL_ARGS just to be consistent with
BOOST_RV_REF and BOOST_RV_REF_N_TEMPL_ARGS.

- Can you comment at all on the stability of the names of the "class
declaration" macros (e.g., BOOST_MOVABLE_BUT_NOT_COPYABLE)? I recall a
previous post that suggested alternative names. I have no opinion
either way, I was just curious on the status of that suggestion.

- In the clone_ptr example, would it be more correct to check for
self-assignment in the move assignment operator (as is done in the copy
assignment operator) rather than not?

- Overall, great job. If there's anything I can do to help the
transition, let me know.

- Jeff


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