Discussion:
[Review] Formal Review: Boost.Move
(too old to reply)
OvermindDL1
2010-05-07 03:36:55 UTC
Permalink
Greetings Boost Developers and Users,

It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts May 10th and lasts until May 24th, 2010, unless an
extension occurs.

What is it?
===========

The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.

The most used function it would add would be:

val_b = boost::move( val_a );

Which would move val_a to val_b; val_a should not be used after this
line. This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use.

Other patterns emulated are && rvalue references for functions/members
and constructors. There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.


Getting the library
===================

The latest version of this library may be downloaded from

SVN: http://svn.boost.org/svn/boost/sandbox/move/

and the docs may be viewed here

Docs: http://igaztanaga.drivehq.com/libs/move/doc/html/index.html

and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.


Writing a review
================

If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.

Here are some questions you might want to answer in your review:

- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?

And finally, every review should answer this question:

- Do you think the library should be accepted as a Boost library?

Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.


Special considerations
======================

If you want or need a primer on the subject, please look at the
working draft for C++1x. This linked site has also been shown to be
quite useful in describing move usefulness:

Want Speed? Pass by Value.:
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
Move It With Rvalue References:
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/

And there are further links in that series, but those two will get you
started in the domain.


Best regards,

OvermindDL1, Review Manager (Boost.Move)
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/
OvermindDL1
2010-05-10 22:24:28 UTC
Permalink
Greetings Boost Developers and Users,

It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts today, May 10th and lasts until May 24th, 2010, unless
an extension occurs.

What is it?
===========

The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.

The most used function it would add would be:

val_b = boost::move( val_a );

Which would move val_a to val_b; val_a should not be used after this
line. This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use. boost:move would be used in place of
std:move in all areas.

Other patterns emulated are && rvalue references for functions/members
and constructors. There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.


Getting the library
===================

The latest version of this library may be downloaded from

SVN: http://svn.boost.org/svn/boost/sandbox/move/

and the docs may be viewed here

Docs: http://igaztanaga.drivehq.com/libs/move/doc/html/index.html

and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.

Complete download: http://igaztanaga.drivehq.com/move_review.zip


Writing a review
================

If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.

Here are some questions you might want to answer in your review:

- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have
any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?

And finally, every review should answer this question:

- Do you think the library should be accepted as a Boost library?

Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.


Special considerations
======================

If you want or need a primer on the subject, please look at the
working draft for C++1x. This linked site has also been shown to be
quite useful in describing move usefulness:

Want Speed? Pass by Value.:
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/

Move It With Rvalue References:
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/

And there are further links in that series, but those two will get you
started in the domain.

I personally ask if as many people could look into this as possible.
This is an important library that could end up being used in just
about every other Boost sub-library and many places elsewhere, and if
there are any problems, they need to be found out sooner, rather than
later.


Best regards,

OvermindDL1, Review Manager (Boost.Move)
John Dlugosz
2010-05-11 18:48:54 UTC
Permalink
Post by OvermindDL1
If you want or need a primer on the subject, please look at the
working draft for C++1x. This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
This article suggests passing by value and letting the compiler make (or omit) the copy. It also has specific advice on return value behavior: return a local variable, but not a formal parameter (even if you have to swap them).

But, I recall reading that Boost has conditional code with a lengthy explanation about what optimizes for one compiler is bad for another, and vice versa, and concerned passing const ref (or not). Is this specific advice more up-to-date with current compilers, or "just his"?

--John

TradeStation Group, Inc. is a publicly-traded holding company (NASDAQ GS: TRAD) of three operating subsidiaries, TradeStation Securities, Inc. (Member NYSE, FINRA, SIPC and NFA), TradeStation Technologies, Inc., a trading software and subscription company, and TradeStation Europe Limited, a United Kingdom, FSA-authorized introducing brokerage firm. None of these companies provides trading or investment advice, recommendations or endorsements of any kind. The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited.
If you received this in error, please contact the sender and delete the material from any computer.
OvermindDL1
2010-05-12 07:47:43 UTC
Permalink
On Tue, May 11, 2010 at 12:48 PM, John Dlugosz
Post by OvermindDL1
If you want or need a primer on the subject, please look at the
working draft for C++1x.  This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
This article suggests passing by value and letting the compiler make (or omit) the copy.  It also has specific advice on return value behavior: return a local variable, but not a formal parameter (even if you have to swap them).
But, I recall reading that Boost has conditional code with a lengthy explanation about what optimizes for one compiler is bad for another, and vice versa, and concerned passing const ref (or not).  Is this specific advice more up-to-date with current compilers, or "just his"?
I am pretty sure that is rather up-to-date with GCC/VS (even slightly
older versions), unsure about others. Boost.Move is more general
though, used for construction of objects, moving values (excellent for
sorting as the articles demonstrate) and others. RVO and Move'ing
combined provides for quite a few efficiencies, and for compilers that
do not support RVO, more liberal use of Boost.Move could emulate it
rather well.
OvermindDL1
2010-05-20 23:20:17 UTC
Permalink
Post by OvermindDL1
Greetings Boost Developers and Users,
It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts May 10th and lasts until May 24th, 2010, unless an
extension occurs.
What is it?
===========
The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.
 val_b = boost::move( val_a );
Which would move val_a to val_b; val_a should not be used after this
line.  This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use.
Other patterns emulated are && rvalue references for functions/members
and constructors.  There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.
Getting the library
===================
The latest version of this library may be downloaded from
 SVN: http://svn.boost.org/svn/boost/sandbox/move/
and the docs may be viewed here
 Docs:  http://igaztanaga.drivehq.com/libs/move/doc/html/index.html
and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.
Writing a review
================
If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.
- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?
- Do you think the library should be accepted as a Boost library?
Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.
Special considerations
======================
If you want or need a primer on the subject, please look at the
working draft for C++1x.  This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
And there are further links in that series, but those two will get you
started in the domain.
Best regards,
OvermindDL1, Review Manager (Boost.Move)
We are approaching the final days of the review and there have been no
definitive reviews as of yet, and a lot of talk about design and what
to include or not include. If this continues without any actual
reviews and none in sight then I will have to end the review period,
else if we do start to get some reviews then we can issue an extension
if the Review Wizards agree.
Steven Watanabe
2010-05-20 23:43:19 UTC
Permalink
AMDG
Post by OvermindDL1
We are approaching the final days of the review and there have been no
definitive reviews as of yet, and a lot of talk about design and what
to include or not include. If this continues without any actual
reviews and none in sight then I will have to end the review period,
else if we do start to get some reviews then we can issue an extension
if the Review Wizards agree.
I intend to submit a review.

In Christ,
Steven Watanabe
Neil Groves
2010-05-21 07:37:28 UTC
Permalink
Post by OvermindDL1
We are approaching the final days of the review and there have been no
definitive reviews as of yet, and a lot of talk about design and what
to include or not include. If this continues without any actual
reviews and none in sight then I will have to end the review period,
else if we do start to get some reviews then we can issue an extension
if the Review Wizards agree.
Heck, I intend to submit a review, but I'm having some terrible schedule
problems.

I really want to see a move library in Boost. From looking at the
implementation I like the design, however I have noticed a problem that as
yet, I have not thought of a solution for.

In the move emulation:

#define BOOST_COPYABLE_AND_MOVABLE(TYPE)\
public:\
TYPE& operator=(TYPE &t)\
{ this->operator=(static_cast<const ::BOOST_MOVE_NAMESPACE::rv<TYPE>
&>(const_cast<const TYPE &>(t))); return *this;}\
public:\
operator ::BOOST_MOVE_NAMESPACE::rv<TYPE>&() \
{ return *reinterpret_cast< ::BOOST_MOVE_NAMESPACE::rv<TYPE>* >(this);
}\
operator const ::BOOST_MOVE_NAMESPACE::rv<TYPE>&() const \
{ return *reinterpret_cast<const ::BOOST_MOVE_NAMESPACE::rv<TYPE>*
Post by OvermindDL1
(this); }\
private:\
//

#else //#ifdef BOOST_MOVE_OPTIMIZED_EMULATION

#define BOOST_COPYABLE_AND_MOVABLE(TYPE)\
public:\
operator ::BOOST_MOVE_NAMESPACE::rv<TYPE>&() \
{ return *reinterpret_cast< ::BOOST_MOVE_NAMESPACE::rv<TYPE>* >(this);
}\
operator const ::BOOST_MOVE_NAMESPACE::rv<TYPE>&() const \
{ return *reinterpret_cast<const ::BOOST_MOVE_NAMESPACE::rv<TYPE>*
Post by OvermindDL1
(this); }\
private:\
//


These reinterpret_cast operations violate the standard aliasing rules
AFAICT. Indeed while most compilers work happily with this arrangement. I
have reproduced defects with GCC 4.4 when optimizations are enabled.
Additionally if one enables the strict aliasing warnings these lines are
indicated as being incorrect.

My concern here is that there is not an obvious simple and efficient
standard solution and yet, a change to correct this would almost certainly
significantly affect the interface.

I'll try and make some time so that I can put something more constructive
together. I figured it would be useful to put my concern out and see if
anyone else has a solution.

Regards,
Neil Groves
Ion Gaztañaga
2010-05-21 08:32:02 UTC
Permalink
Post by Neil Groves
These reinterpret_cast operations violate the standard aliasing rules
AFAICT. Indeed while most compilers work happily with this arrangement.
I have reproduced defects with GCC 4.4 when optimizations are enabled.
Additionally if one enables the strict aliasing warnings these lines are
indicated as being incorrect.
What if we use static_cast? Does this solve any problem? I don't know
much about aliasing rules so can you shed some light on this?

Best,

Ion
Neil Groves
2010-05-21 11:35:01 UTC
Permalink
Post by Neil Groves
These reinterpret_cast operations violate the standard aliasing rules
AFAICT. Indeed while most compilers work happily with this arrangement.
I have reproduced defects with GCC 4.4 when optimizations are enabled.
Additionally if one enables the strict aliasing warnings these lines are
indicated as being incorrect.
What if we use static_cast? Does this solve any problem? I don't know much
about aliasing rules so can you shed some light on this?
There was a bug logged against GCC 4.4 using a Boost.Move inspired snippet -
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44186

This bug was closed as invalid because the C99 aliasing rules are violated
by the casting. However by changing all of the reinterpret and c style casts
to static_casts technically renders this compliant due to Section 5.2.9 para
2. Therefore I believe static_casts provide a route to a performant and
standard compliant version.

Sadly I'm still having errors with my version on GCC 4.4 despite having
fixed the aliasing problems. The relevant GCC warnings have all disappeared
indicating that the aliasing is now compliant. I am continuing to
investigate why the generated code appears to be incorrect.

I'll perform a full review as soon as I possibly can.
Best,
Ion
I hope this helps,
Neil Groves
Frank Mori Hess
2010-05-21 13:06:47 UTC
Permalink
Post by Neil Groves
Sadly I'm still having errors with my version on GCC 4.4 despite having
fixed the aliasing problems. The relevant GCC warnings have all disappeared
indicating that the aliasing is now compliant.
You should be aware that gcc has historically been quite poor at generating
strict-aliasing violation warnings. Just because it doesn't warn doesn't
mean there aren't any problems. Maybe one of those static_cast<void*>() are
leading to problems?
Steven Watanabe
2010-05-22 15:49:24 UTC
Permalink
AMDG
Post by Frank Mori Hess
Post by Neil Groves
Sadly I'm still having errors with my version on GCC 4.4 despite having
fixed the aliasing problems. The relevant GCC warnings have all disappeared
indicating that the aliasing is now compliant.
You should be aware that gcc has historically been quite poor at generating
strict-aliasing violation warnings. Just because it doesn't warn doesn't
mean there aren't any problems. Maybe one of those static_cast<void*>() are
leading to problems?
There shouldn't be a cast through void*. Since
boost::rv<T> inherits from T, it should just use
a static_cast. The cast back to T is effectively
a static_cast, and the two conversions need
to match.

In Christ,
Steven Watanabe
Ion Gaztañaga
2010-05-22 19:05:21 UTC
Permalink
There shouldn't be a cast through void*. Since
boost::rv<T> inherits from T, it should just use
a static_cast. The cast back to T is effectively
a static_cast, and the two conversions need
to match.
There's no conversion through void* but gcc still crashes with
static_cast. C++ aliasing rules allow accesing a type T by through a
base class, but we're accessing it through a non-existing derived type
(rv<T>) so, I think this breaks aliasing rules. Adding
__attribute__((__may_alias__)) to rv to tell the compiler that rv could
alias any type seems to fix the problem (although is overkill IMHO). See:

https://svn.boost.org/trac/boost/ticket/3950

Other compilers don't seem to have this problem and hopefully this
emulation is temporary until all compilers implement rvalue references.

Best,

Ion
Neil Groves
2010-05-23 09:15:13 UTC
Permalink
There's no conversion through void* but gcc still crashes with static_cast.
C++ aliasing rules allow accesing a type T by through a base class, but
we're accessing it through a non-existing derived type (rv<T>) so, I think
this breaks aliasing rules. Adding __attribute__((__may_alias__)) to rv to
tell the compiler that rv could alias any type seems to fix the problem
https://svn.boost.org/trac/boost/ticket/3950
Sadly while it fixes the problems in unit test. I have an application where
I did added the may_alias attribute, and now I get passed unit tests for
move and compiler defects in function lookup for classes that use
RVALUE_REF. This isn't a complete fix for GCC 4.4. I'm still working on a
better solution, and a proper review.

I convinced myself that with static_casts do not break strict aliasing rules
since the conversion from rv<T>* to T* is valid. I believe that section
5.2.9 para 2 applies:

"An lvalue of type “cv1 B,” where B is a class type, can be cast to type
“reference to cv2 D,” where D is a class
derived (Clause 10) from B, if a valid standard conversion from “pointer to
D” to “pointer to B” exists (4.10),
cv2 is the same cv-qualification as, or greater cv-qualification than, cv1,
and B is neither a virtual base class
of D nor a base class of a virtual base class of D. The result has type “cv2
D.” An rvalue of type “cv1 B” may
be cast to type “rvalue reference to cv2 D” with the same constraints as for
an lvalue of type “cv1 B.” If the
object of type “cv1 B” is actually a subobject of an object of type D, the
result refers to the enclosing object
of type D. Otherwise, the result of the cast is undefined."

I'll get back to you after a little more experimentation.

Regards,
Neil Groves
Ion Gaztañaga
2010-05-23 12:06:44 UTC
Permalink
Post by Neil Groves
I convinced myself that with static_casts do not break strict aliasing
rules since the conversion from rv<T>* to T* is valid. I believe that
The problem is not downcasting, but the fact that T is not rv<T> (the
dynamic type of T is T, not rv<T>):

"If the object of type “cv1 B” is actually a subobject of an object of
type D, the result refers to the enclosing object of type D. Otherwise,
the result of the cast is undefined."

When we use static_cast<rv<T>>, the object T is not actually a subobject
of rv<T>, so I think we're in undefined behaviour. Imagine this:

class B
{
int filler;
};

class D : public B
{ int filler2;}

B b;
D *dp = static_cast<D*>(&b);

//dp is now pointing to garbage

Best,

Ion
Neil Groves
2010-05-23 12:38:03 UTC
Permalink
Post by Neil Groves
I convinced myself that with static_casts do not break strict aliasing
Post by Neil Groves
rules since the conversion from rv<T>* to T* is valid. I believe that
The problem is not downcasting, but the fact that T is not rv<T> (the
"If the object of type “cv1 B” is actually a subobject of an object of type
D, the result refers to the enclosing object of type D. Otherwise, the
result of the cast is undefined."
Yes I concur. I expected it to be undefined, but when I started looking I
failed to find the relevant part of the standard.

So am I correct in stating that the move emulation is therefore
fundamentally relying upon undefined behaviour with no performant
alternative that uses the same interface?

Is the impact of this issue such that to be compliant we would need to use a
move_sink like the adobe design? I imagine this would make substitution with
proper rvalue references more difficult.

Do you have a plan, because I cannot see a simple solution despite having
spent quite some time exploring the options.
Post by Neil Groves
Best,
Ion
_______________________________________________
Regards,
Neil Groves
Ion Gaztañaga
2010-05-23 18:48:30 UTC
Permalink
Post by Neil Groves
So am I correct in stating that the move emulation is therefore
fundamentally relying upon undefined behaviour with no performant
alternative that uses the same interface?
It seems so, but I'm not sure. We'll need to confirm this with a
language expert. If the problem is GCC, we can workaround it. Hopefully
move emulation works nearly for all compilers and in the future all of
them will have rvalus references, so I don't think we have a big issue here.
Post by Neil Groves
Is the impact of this issue such that to be compliant we would need to
use a move_sink like the adobe design? I imagine this would make
substitution with proper rvalue references more difficult.
Yes, it will make it quite difficult. I think we can solve GCC issues,
(either if it's undefined behaviour or not), more now that GCC offers
rvalue references. I hope -std=c++0x will be activated by default soon.

Best,

Ion
Steven Watanabe
2010-05-23 19:09:32 UTC
Permalink
AMDG
Post by Ion Gaztañaga
Post by Neil Groves
So am I correct in stating that the move emulation is therefore
fundamentally relying upon undefined behaviour with no performant
alternative that uses the same interface?
It seems so, but I'm not sure. We'll need to confirm this with a
language expert. If the problem is GCC, we can workaround it.
Hopefully move emulation works nearly for all compilers and in the
future all of them will have rvalus references, so I don't think we
have a big issue here.
I'm wondering whether it would be better to
use something like

template<class T>
struct rv {
T* impl;
operator const T&() const;
};

template<class T>
T& unwrap_rv(rv<T>&);

I know the interface isn't quite as nice, and
there will be more cases where overload resolution
needs a hand, but it seems safer.

In Christ,
Steven Watanabe
Ion Gaztañaga
2010-05-23 19:26:49 UTC
Permalink
Post by Steven Watanabe
I'm wondering whether it would be better to
use something like
template<class T>
struct rv {
T* impl;
operator const T&() const;
};
template<class T>
T& unwrap_rv(rv<T>&);
I know the interface isn't quite as nice, and
there will be more cases where overload resolution
needs a hand, but it seems safer.
We'll need to experiment a bit, that's why exposing rv<T> was a bit
dangerous IMHO. I don't know if this circular conversion between rv<->T
will be harmful, but we could try it.

Best,

Ion
Neil Groves
2010-05-23 21:55:38 UTC
Permalink
Post by Steven Watanabe
I'm wondering whether it would be better to
use something like
template<class T>
struct rv {
T* impl;
operator const T&() const;
};
template<class T>
T& unwrap_rv(rv<T>&);
I know the interface isn't quite as nice, and
there will be more cases where overload resolution
needs a hand, but it seems safer.
I might be being a bit slow, but doesn't replacing the inheritance of rv<T>
from T with the contained pointer change the lifetime and ownership of the T
object in problematic ways?

Anyhow here is my review:
1. What is your evaluation of the implementation?
In general it is a good well executed modern C++ implementation. There are a
few compiler portability issues such as the is_convertible implementation
and the accessability of the boost::rv destructor.

I have found the macro idiom to work very well and it does not obfuscate
code. I like the macros since they are able to guarantee a completely
optimal upgrade path to proper rvalue references where these are supported
by the compiler.

For me the most serious implementation issue is the non-compliant aliasing
when move emulation is selected. For the forseeable future most of the
compilers that we support will use the emulation support. While the tested
behaviour on many compilers has not shown defects, it is my opinion that
this is not an acceptable approach for Boost. We must not have core
libraries that depend on undefined behaviour. While I appreciate that GCC
4.4 has been the first to show defect symptoms due to the aliasing, it is
quite conceivable that problems would arise after compiler revisions or
patches that we would not be able to fix. This is especially important
considering the huge usefulness of this library.

2. What is your evaluation of the documentation?

There are a couple of small spelling mistakes that have already been pointed
out on the list, but I found the documentation entirely adequate to use the
library confidently.

3. What is your evaluation of the potential usefulness of the library?

Clearly Boost.Move will be enormously useful in client code and as a tool
for other Boost libraries. There are plenty of benchmarks that show
substantial performance benefits with rvalue references, and from my timings
a good emulation achieves very similar results. I am very keen to introduce
move support into Boost.Range.

4. Did you try to use the library? With what compiler? Did you have any
problems?

I tried to use the library with my own unit tests, and integrated into a
large-scale application. I used Intel C++ 10.0, 10.1, 11.0, GCC 4.3, 4.4.3,
MSVC 8, MSVC 9, MSVC10.

I did have a few problems. The rv destructor needs to be public for VC.

The I had to replace the is_convertible implementation with the one from
type_traits.

The violating of the strict aliasing rules was causing problems with
correctness on GCC 4.4, and I tried a couple of modifications:

The first modification was to replace the reinterpret_casts with
static_casts.

The second modification was to add the __attribute__ may_alias to the GCC
4.4 move emulation. This has the very strange effect of causing all of my
unit tests to pass, but causes subsequent compilation failures in regions of
code that use the move support. The failure appears to be that a function of
the movable class cannot be found, and the compiler lists the available
functions, one of which is exactly the function signature being looked for!
Hence I believe that may_alias replaces on set of problems for another less
obvious set of problems.

5. How much effort did you put into your evaluation?

I have put in approximately 40 hours of evaluation and experimentation with
the compiler set mentioned. I placed much of my time examining the generated
code for performance issues, and exploring solutions to the aliasing rule
violation.

6. Are you knowledgeable about the problem domain?

I am familiar with the advantages of move support, and the relevant language
specifications.

7. Do you think the library should be accepted as a Boost library.

Yes, if and only if a fully compliant move emulation mechanism is produced.
Absence of evidence of error on compilers is not sufficient in my opinion.

I would gladly accept a more complex interface for a defined implementation.

Thank you Ion for putting this together, and please let me know if I can
assist in any way.

Regards,
Neil Groves
Ion Gaztañaga
2010-05-24 06:58:31 UTC
Permalink
Post by Steven Watanabe
I'm wondering whether it would be better to
use something like
template<class T>
struct rv {
T* impl;
operator const T&() const;
};
template<class T>
T& unwrap_rv(rv<T>&);
This does not seem to work:

template<class T>
struct rv_ref
{
rv_ref(T &t){ ptr = &t; }
rv_ref(const T &t){ ptr = &t; }

operator const T& () const { return this->get(); }
T& get() { return *const_cast<T*>(ptr); }
const T& get() const { return *ptr; }

private:
const T *ptr;
};


template<class T>
inline rv_ref<T> move(T &t)
{ return static_cast< rv_ref<T> >(t); }

class movable
{
movable(movable&);
movable & operator=(movable&);
public:
operator rv_ref<movable>()
{ return rv_ref<movable>(*this); }
operator const rv_ref<movable>() const
{ return rv_ref<movable>(*this); }

movable() : i(0) {}

movable(rv_ref<movable> rv)
{
movable &t = rv.get();
i = t.i;
t.i = 0;
}

movable& operator=(rv_ref<movable> rv)
{
movable &t = rv.get();
i = t.i;
t.i = 0;
return *this;
}
int i;
};

movable sm_rvalue()
{
movable m;
return move(m);
}

int main()
{
movable m, m2(move(m));
m = move(m2);
//MSVC 7.1
//error C2679: binary '=' : no operator found which
//takes a right-hand operand of type 'movable' (or
//there is no acceptable conversion)
//GCC
//error: no matching function for call to 'movable::movable(movable)'
//note: candidates are: movable::movable(rv_ref<movable>)
//note: movable::movable(movable&)
m = sm_rvalue();
return 0;
}
Ilya Sokolov
2010-05-24 09:18:36 UTC
Permalink
[snip]
movable sm_rvalue()
{
movable m;
return move(m);
}
int main()
{
movable m, m2(move(m));
m = move(m2);
//MSVC 7.1
//error C2679: binary '=' : no operator found which
//takes a right-hand operand of type 'movable' (or
//there is no acceptable conversion)
//GCC
//error: no matching function for call to 'movable::movable(movable)'
//note: candidates are: movable::movable(rv_ref<movable>)
//note: movable::movable(movable&)
m = sm_rvalue();
return 0;
}
You can solve it by applying move() as you did in sm_rvalue().
Steven Watanabe
2010-05-24 15:16:58 UTC
Permalink
AMDG
Post by Steven Watanabe
template<class T>
struct rv_ref
{
rv_ref(T &t){ ptr = &t; }
rv_ref(const T &t){ ptr = &t; }
These constructors need to be explicit.

Your code compiles with VC 2010 with this change.

In Christ,
Steven Watanabe
Ion Gaztañaga
2010-05-24 21:09:58 UTC
Permalink
Post by Steven Watanabe
AMDG
Post by Steven Watanabe
template<class T>
struct rv_ref
{
rv_ref(T &t){ ptr = &t; }
rv_ref(const T &t){ ptr = &t; }
These constructors need to be explicit.
Your code compiles with VC 2010 with this change.
But not on my GCC:

error: no matching function for call to 'movable::movable(movable)'
note: candidates are: movable::movable(rv_ref<movable>)
note: movable::movable(movable&)
error: initializing temporary from result of
'movable::movable(rv_ref<movable>)'

But after some strange experiments, and fighting against the elements
here's my last try, compiles in MSVC 7.1, 8.0, 9.0, 10.0, gcc 4.3, and
Intel 10.0 (edg based). Keep an eye on the catch by non-const reference,
plus enable_if trick, to avoid conversion ambiguities:

#include <boost/utility/enable_if.hpp>
#include <boost/type_traits/is_same.hpp>

#define BOOST_COPYABLE_AND_MOVABLE(TYPE)\
public:\
TYPE& operator=(TYPE &t)\
{\
this->operator=(const_cast<const TYPE &>(t));\
return *this; \
}\
public:\
operator rv_ref<TYPE>(){ return rv_ref<TYPE>(*this); }\
private:\
//

#define BOOST_COPY_ASSIGN_REF(TYPE)\
const_rv_ref< TYPE > \
//


#define BOOST_MOVABLE_BUT_NOT_COPYABLE(TYPE)\
private:\
TYPE(TYPE &);\
TYPE& operator=(TYPE &);\
public:\
operator rv_ref<TYPE>(){ return rv_ref<TYPE>(*this); }\
private:\
//


#define BOOST_RV_REF(TYPE)\
rv_ref<TYPE> \
//


template<class T>
struct rv_ref
{
explicit rv_ref(T &t){ ptr = &t; }

operator T& () { return *ptr; }

private:
T *ptr;
};


template<class T>
struct const_rv_ref
{
const_rv_ref(rv_ref<T> t){ ptr = t.ptr; }
template<class U>
const_rv_ref(U &t, typename boost::enable_if_c
<boost::is_same<const T, U>::value>::type* = 0)
{ ptr = &t; }
operator const T& () const { return this->get(); }

private:
const T *ptr;
};

template<class T>
inline rv_ref<T> move(T &t)
{ return static_cast< rv_ref<T> >(t); }


class copiable_and_movable
{
public:
BOOST_COPYABLE_AND_MOVABLE(copiable_and_movable)
public:
copiable_and_movable(){}

copiable_and_movable(const copiable_and_movable &){}
copiable_and_movable(BOOST_RV_REF(copiable_and_movable)){}

copiable_and_movable &operator =
(BOOST_COPY_ASSIGN_REF(copiable_and_movable)){ return *this; }
copiable_and_movable &operator =
(BOOST_RV_REF(copiable_and_movable)){ return *this; }
};


static copiable_and_movable scm;

copiable_and_movable sc_rvalue()
{ return copiable_and_movable(); }

const copiable_and_movable sc_crvalue()
{
typedef const copiable_and_movable ctype;
return ctype();
}

const copiable_and_movable &sc_clvalue()
{ return scm; }

copiable_and_movable &sc_lvalue()
{ return scm; }




class movable
{
BOOST_MOVABLE_BUT_NOT_COPYABLE(movable)

public:
movable() : i(0) {}

movable(BOOST_RV_REF(movable) rv)
{
movable &t = rv;
i = t.i;
t.i = 0;
}

movable& operator=(BOOST_RV_REF(movable) rv)
{
movable &t = rv;
i = t.i;
t.i = 0;
return *this;
}
int i;
};

movable sm_rvalue()
{
movable m;
//This fails in several compilers (GCC for instance)
// error: no matching function for call to 'movable::movable(movable)'
// note: candidates are: movable::movable(rv_ref<movable>)
// note: movable::movable(movable&)
// initializing temporary from result of 'movable::movable(rv_ref<movable>)'
//
// --> return move(m);

//This works (in-place construction of movable)
return movable(move(m));
}

const int rvalue_catched = 1;
const int lvalue_catched = 2;
const int const_rlvalue_catched = 3;

int catch_it(BOOST_RV_REF(copiable_and_movable))
{ return rvalue_catched; }

int catch_it(copiable_and_movable &)
{ return lvalue_catched; }

int catch_it(BOOST_COPY_ASSIGN_REF(copiable_and_movable))
{ return const_rlvalue_catched; }

int main()
{
{
movable m, m2(move(m));
m = move(m2);
m = sm_rvalue();
}
{
copiable_and_movable c, c2(move(c));
c = move(c2);
c = sc_rvalue();
c = sc_clvalue();
c = sc_lvalue();
}

if(rvalue_catched != catch_it(sc_rvalue()))
return 1;
if(lvalue_catched != catch_it(sc_lvalue()))
return 1;
if(const_rlvalue_catched != catch_it(sc_clvalue()))
return 1;
if(const_rlvalue_catched != catch_it(sc_crvalue()))
return 1;

return 0;
}
Ion Gaztañaga
2010-05-24 21:37:27 UTC
Permalink
Post by Ion Gaztañaga
But after some strange experiments, and fighting against the elements
here's my last try, compiles in MSVC 7.1, 8.0, 9.0, 10.0, gcc 4.3, and
Intel 10.0 (edg based). Keep an eye on the catch by non-const reference,
Maybe const_rv_ref should be called const_rlv_ref, because catches both
rv_ref and const T.

Best,

Ion
John Dlugosz
2010-05-24 17:59:23 UTC
Permalink
Post by Ion Gaztañaga
The problem is not downcasting, but the fact that T is not rv<T> (the
Reading the referenced article on C99 aliasing rules, perhaps what is needed is a alias_cast template that does it right (e.g. pass it through a union or use proper compiler-specific decorations).

TradeStation Group, Inc. is a publicly-traded holding company (NASDAQ GS: TRAD) of three operating subsidiaries, TradeStation Securities, Inc. (Member NYSE, FINRA, SIPC and NFA), TradeStation Technologies, Inc., a trading software and subscription company, and TradeStation Europe Limited, a United Kingdom, FSA-authorized introducing brokerage firm. None of these companies provides trading or investment advice, recommendations or endorsements of any kind. The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited.
If you received this in error, please contact the sender and delete the material from any computer.
Neil Groves
2010-05-24 18:14:08 UTC
Permalink
Post by John Dlugosz
Post by Ion Gaztañaga
The problem is not downcasting, but the fact that T is not rv<T> (the
Reading the referenced article on C99 aliasing rules, perhaps what is
needed is a alias_cast template that does it right (e.g. pass it through a
union or use proper compiler-specific decorations).
That was my first thought too, but as far as I know all correct uses of a
union would mandate a fully copy of the T instance. I think that the
containment by pointer that was suggested by Steven Watanabe makes for a
nice solution.

If you have a novel solution that uses a union while avoiding assignment to
a new T instance I would be very interested.

Regards,
Neil Groves
John Dlugosz
2010-05-24 20:03:38 UTC
Permalink
I think that the point of the article (example 1 vs example 3) is that you are not allowed to cast or unionize pointers. You must store and fetch from a location that's declared as a union.

Now what if you cast, not from T1* to T2*, but from T1* to U*? Does the union itself have to be defined as a union, or can you cast the original typed pointer to the union pointer, perhaps as the parameter to a function call so that only the U* pointer is in scope and being dereferenced while T1* in the caller is dormant and assumed to be "changed" by virtue of the call.

Perhaps a close reading of the spec will indicate that a "barrier" is possible where two pointers are not used at the same time. I'm certain that works in practice though: upon calling foo(p);, the compiler assumes that *p has been changed. Within the body of foo, p is not used at all but q is used, which has the same address and different type. If p is not used, you don't have to worry about it being assumed not to change just because q is used. However, inlining complicates things.

I suppose to be ratified, a portable solution is needed. But individual compiler-specific solutions would implement the alias_cast by saying "yes this will alias" in a compiler-specific manner. The general solution is only for formality and bootstrapping a port to a different compiler/version, so it doesn't need to be that efficient.



From: boost-users-***@lists.boost.org [mailto:boost-users-***@lists.boost.org] On Behalf Of Neil Groves
Sent: Monday, May 24, 2010 1:14 PM
To: boost-***@lists.boost.org
Subject: Re: [Boost-users] [Review] Formal Review: Boost.Move
Post by Ion Gaztañaga
The problem is not downcasting, but the fact that T is not rv<T> (the
Reading the referenced article on C99 aliasing rules, perhaps what is needed is a alias_cast template that does it right (e.g. pass it through a union or use proper compiler-specific decorations).

That was my first thought too, but as far as I know all correct uses of a union would mandate a fully copy of the T instance. I think that the containment by pointer that was suggested by Steven Watanabe makes for a nice solution.

If you have a novel solution that uses a union while avoiding assignment to a new T instance I would be very interested.

Regards,
Neil Groves


TradeStation Group, Inc. is a publicly-traded holding company (NASDAQ GS: TRAD) of three operating subsidiaries, TradeStation Securities, Inc. (Member NYSE, FINRA, SIPC and NFA), TradeStation Technologies, Inc., a trading software and subscription company, and TradeStation Europe Limited, a United Kingdom, FSA-authorized introducing brokerage firm. None of these companies provides trading or investment advice, recommendations or endorsements of any kind. The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited. If you received this in error, please contact the sender and delete the material from any computer.
Ravi
2010-05-23 01:51:42 UTC
Permalink
Post by Ion Gaztañaga
What if we use static_cast? Does this solve any problem? I don't know
much about aliasing rules so can you shed some light on this?
The following is the (only?) best explanation of gcc aliasing rules that I
have seen:

http://cellperformance.beyond3d.com/articles/2006/06/understanding-strict-
aliasing.html

Regards,
Ravi
OvermindDL1
2010-05-23 22:15:50 UTC
Permalink
Post by OvermindDL1
Greetings Boost Developers and Users,
It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts May 10th and lasts until May 24th, 2010, unless an
extension occurs.
What is it?
===========
The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.
 val_b = boost::move( val_a );
Which would move val_a to val_b; val_a should not be used after this
line.  This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use.
Other patterns emulated are && rvalue references for functions/members
and constructors.  There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.
Getting the library
===================
The latest version of this library may be downloaded from
 SVN: http://svn.boost.org/svn/boost/sandbox/move/
and the docs may be viewed here
 Docs:  http://igaztanaga.drivehq.com/libs/move/doc/html/index.html
and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.
Writing a review
================
If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.
- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?
- Do you think the library should be accepted as a Boost library?
Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.
Special considerations
======================
If you want or need a primer on the subject, please look at the
working draft for C++1x.  This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
And there are further links in that series, but those two will get you
started in the domain.
Best regards,
OvermindDL1, Review Manager (Boost.Move)
There have been a couple of review submissions so far, so at this
point I will keep the review on track for its normal ending period,
thus it ends tomorrow night. I will go through the reviews and create
an overall summary shortly thereafter (I already have time set aside
for Tuesday for this).

Do note, thus far there have been a few issues brought up about this
library, I will see how these continue, but you may expect me to ask
for feedback on them as there seems to be a possible conditional
acceptance for this library, and at least one (gcc related) bug-fix
due to how this library implements something that is not well defined
in the standard.

If at all possible, please finish up discussion soon, but do not feel
rushed, I can delay the final decision, this is an important library
for older compilers and it needs to be very well fleshed out.
Ilya Sokolov
2010-05-24 10:20:52 UTC
Permalink
[snip]
one (gcc related) bug-fix
due to how this library implements something that is not well defined
in the standard.
It is defined pretty clear (for me) in 3.8/6 of 14882:2003:

_quote_
Similarly, before the lifetime of an object has started but after the
storage which the object will occupy has
been allocated or, after the lifetime of an object has ended and before
the storage which the object occupied
is reused or released, any lvalue which refers to the original object
may be used but only in limited ways.
Such an lvalue refers to allocated storage (3.7.3.2), and using the
properties of the lvalue which do not
depend on its value is well-defined. If an lvalue-to-rvalue conversion
(4.1) is applied to such an lvalue, the
program has undefined behavior; if the original object will be or was of
a non-POD class type, the program
has undefined behavior if:
— the lvalue is used to access a non-static data member or call a
non-static member function of the object,
or
— the lvalue is implicitly converted (4.10) to a reference to a base
class type, or
— the lvalue is used as the operand of a static_cast (5.2.9) (except
when the conversion is ultimately
to char& or unsigned char&), or
— the lvalue is used as the operand of a dynamic_cast (5.2.7) or as the
operand of typeid.
_end_of_quote
[snip]
OvermindDL1
2010-05-25 09:31:12 UTC
Permalink
Post by OvermindDL1
Greetings Boost Developers and Users,
It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts May 10th and lasts until May 24th, 2010, unless an
extension occurs.
What is it?
===========
The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.
 val_b = boost::move( val_a );
Which would move val_a to val_b; val_a should not be used after this
line.  This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use.
Other patterns emulated are && rvalue references for functions/members
and constructors.  There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.
Getting the library
===================
The latest version of this library may be downloaded from
 SVN: http://svn.boost.org/svn/boost/sandbox/move/
and the docs may be viewed here
 Docs:  http://igaztanaga.drivehq.com/libs/move/doc/html/index.html
and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.
Writing a review
================
If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.
- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?
- Do you think the library should be accepted as a Boost library?
Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.
Special considerations
======================
If you want or need a primer on the subject, please look at the
working draft for C++1x.  This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
And there are further links in that series, but those two will get you
started in the domain.
Best regards,
OvermindDL1, Review Manager (Boost.Move)
The review is over, I shall still be looking at newly posted reviews
and reports over the next day or so as I get time to combine all the
data. So this is last call!
OvermindDL1
2010-05-26 19:10:34 UTC
Permalink
Post by OvermindDL1
Greetings Boost Developers and Users,
It's my pleasure to announce that the review of Ion Gaztañagas' Move
library starts May 10th and lasts until May 24th, 2010, unless an
extension occurs.
What is it?
===========
The Boost.Move library would supply an emulated C++1x Move semantics
interface, aka RValue References, allowing for a wide variety of easy
to use and easy to add in optimization abilities.
 val_b = boost::move( val_a );
Which would move val_a to val_b; val_a should not be used after this
line.  This semantic allows for a vast amount of optimizations,
especially in regards to temporaries, see the documentation for
further examples and use.
Other patterns emulated are && rvalue references for functions/members
and constructors.  There are also a number of STL containers in the
sandbox that support move semantics using this library for higher
efficiency.
Getting the library
===================
The latest version of this library may be downloaded from
 SVN: http://svn.boost.org/svn/boost/sandbox/move/
and the docs may be viewed here
 Docs:  http://igaztanaga.drivehq.com/libs/move/doc/html/index.html
and do note, this library is header-only and is designed to be
absolutely generic to be useful in all areas of C++ programming.
Writing a review
================
If you feel this is an interesting library, then please submit your
review to the developer list (preferably), or to the review manager.
- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With what compiler? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick
- reading? In-depth study?
- Are you knowledgeable about the problem domain?
- Do you think the library should be accepted as a Boost library?
Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.
Special considerations
======================
If you want or need a primer on the subject, please look at the
working draft for C++1x.  This linked site has also been shown to be
http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
http://cpp-next.com/archive/2009/09/move-it-with-rvalue-references/
And there are further links in that series, but those two will get you
started in the domain.
Best regards,
OvermindDL1, Review Manager (Boost.Move)
The review is over, I am still following the conversation thread for
more input however. I should have the final summary and overall vote
of all the reviews here within the next few days, in the mean time
please keep up the ongoing conversation as it is important overall.
_______________________________________________
Unsubscribe & other changes: ht

Continue reading on narkive:
Loading...