Hi,

I recently encountered an interesting issue in which
func(std::move(x)) will actually =93move=94 x even when =91func=92 doesn=92=
t
actually take ownership (e.g. strong guarantee on throw).
The reason for this is that when converting T&& to U&& there might be
an implicit temporary of type U constructed using the U(T&&) move
constructor for conversion (per 8.5.3/5).

Consider the following:
struct B {};
struct D : B {};

list<shared_ptr<B>> aList;
shared_ptr<D> sp = new D;

aList.push_back(move(sp));
</sample code>

One might expect that =91sp=92 will be left untouched when push_back
throws but it actually will always be null.
The reason is that the above call will select
push_back(shared_ptr<B>&&)  hence the caller actually *implicitly*
performs:
aList.push_back(shared_ptr<B>(move(sp));

It might be possible to resolve such cases using generalized push_back
method that will prevent the call-site conversion:
template<typename U> push_back(U&&); // no need for push_back(T
const&)

Such workaround might have other deficiencies (e.g. bad diagnostics)
and ordinary users are certainly not expected use such.

Consider the following user defined function:
// takeover sp only if needed
void TakeOver(shared_ptr<B>&& sp);

// caller try to move ownership
TakeOver(move(spD));

if (!spD) // no-takeover check =96 BUGBUG spD is always be null
regardless of TakeOver=92s implementation

Will the above unconditional transfer of ownership (move) surprise the
callers?
I personally thinks that such move(l-value) is not common enough to
create severe risk but the move notion is young...

Thoughts?

Thanks,
Rani


--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

On Jun 30, 7:31 pm, Rani Sharoni <ranisharon...@gmail.com> wrote:
> I recently encountered an interesting issue in which
> func(std::move(x)) will actually =93move=94 x even when =91func=92 doesn=92=

Sorry for the bad text. This is probably related to pasting from my
text editor.
I will try to give a better version of my concern.

Consider the following code:
struct B {};
struct D : B {};

set<unique_ptr<B>, ...> aSet;

unique_ptr<B> spB = new D;
auto res1 = aSet.insert(move(spB));
assert(res1.second ^ !spB); // take ownership on successful insert

unique_ptr<D> spD = new D;
auto res2 = aSet.insert(move(spD));
assert(res2.second ^ !spD); // BUGBUG - spD is always null
</code>

In most cases func(move(x)) will not modify x unless func modified it
but when implicit conversion is involved then 'x' will be moved
regardless of the implementation of func since it's actually
equivalent to func( U(move(x)) ).

In the above the spD insertion is actually:
aSet.insert(unique_ptr<B>(move(spD))); // take-over and try-insert

Such call-sites are sensitive to whether conversion is needed and that
can surprise the caller.
It might possible to avoid such conversion using template insert:
template<typename U> insert(U&& x); // implementation uses
forward<U>(x)

The above template insert will prevent the conversion but no user is
expected to define such in order to avoid the implicit move conversion
issue.
std::move practically offers a new notation for transfer of ownership
hence such pitfall might be harmful in practice.

Thoughts?

Thanks,
Rani


--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

Am 02.07.2011 09:17, schrieb Rani Sharoni:
>
[..]
> Consider the following code:
> struct B {};
> struct D : B {};
>
> set<unique_ptr<B>, ...>  aSet;
>
> unique_ptr<B>  spB = new D;
> auto res1 = aSet.insert(move(spB));
> assert(res1.second ^ !spB); // take ownership on successful insert
>
> unique_ptr<D>  spD = new D;
> auto res2 = aSet.insert(move(spD));
> assert(res2.second ^ !spD); // BUGBUG - spD is always null
> </code>
>
> In most cases func(move(x)) will not modify x unless func modified it
> but when implicit conversion is involved then 'x' will be moved
> regardless of the implementation of func since it's actually
> equivalent to func( U(move(x)) ).
>
> In the above the spD insertion is actually:
> aSet.insert(unique_ptr<B>(move(spD))); // take-over and try-insert
>
> Such call-sites are sensitive to whether conversion is needed and that
> can surprise the caller.
> It might possible to avoid such conversion using template insert:
> template<typename U>  insert(U&&  x); // implementation uses
> forward<U>(x)
>
> The above template insert will prevent the conversion but no user is
> expected to define such in order to avoid the implicit move conversion
> issue.
> std::move practically offers a new notation for transfer of ownership
> hence such pitfall might be harmful in practice.
>
> Thoughts?

I find your analysis correct and pretty interesting. Nevertheless, I'm
yet not convinced that your scenarios describe gotchas of the real
world, because I believe that user-code would normally not inspect the
argument of std::move in a manner that you describe above. It is true
that library-provided types guarantee that the source of a move
operation is in an "unspecified but valid" state and often specifies
this state exactly, but this guarantee does to extend to any other
types. This was a recent change during the Madrid meeting that leads to
the statement for MoveConstructible and MoveAssignable that the state of
a move source is unspecified. Therefore I would say that people should
better get used to the fact, that inspecting such a move source as a
matter of program flow control is a very bad idea.

Greetings from Bremen,

Daniel Kr   gler



--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

Rani Sharoni wrote:
> On Jun 30, 7:31 pm, Rani Sharoni <ranisharon...@gmail.com> wrote:
>> I recently encountered an interesting issue in which
>> func(std::move(x)) will actually =93move=94 x even when =91func=92
>> doesn=92=
>
> Sorry for the bad text. This is probably related to pasting from my
> text editor.
> I will try to give a better version of my concern.
>
> Consider the following code:
> struct B {};
> struct D : B {};
>
> set<unique_ptr<B>, ...> aSet;
>
> unique_ptr<B> spB = new D;
> auto res1 = aSet.insert(move(spB));
> assert(res1.second ^ !spB); // take ownership on successful insert
>
> unique_ptr<D> spD = new D;
> auto res2 = aSet.insert(move(spD));
> assert(res2.second ^ !spD); // BUGBUG - spD is always null
> </code>
>

I don't think this is any worse than that the language lets you divide
by zero or index an array out of range.

By using std::move(spD) you say "take it, I don't care about it
anymore". Then on the next line you DO care!


Isn't it a rare condition to try to insert into a set, but still
wanting to keep the duplicate? Why not use a multiset in that case?



Bo Persson



--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

> > auto res1 = aSet.insert(move(spB));
> > assert(res1.second ^ !spB); // take ownership on successful insert
>
> > auto res2 = aSet.insert(move(spD));
> > assert(res2.second ^ !spD); // BUGBUG - spD is always null
>
> I find your analysis correct and pretty interesting. Nevertheless, I'm
> yet not convinced that your scenarios describe gotchas of the real
> world, because I believe that user-code would normally not inspect the
> argument of std::move in a manner that you describe above. It is true
> that library-provided types guarantee that the source of a move
> operation is in an "unspecified but valid" state

Interesting. I can't find this in the N3242 draft though it make
sense.
OTOH, the VC2010 move-constructor/assignment of containers explicitly
clears the source argument though there is theoretical opportunity for
storage recycling in case that move-assignment will simply use swap
without clear:
vector<string> aVec1, aVec2;
aVec1 = move(aVec2);  // aVec1.swap(aVec2);
aVec2.assign(aList.begin(), aList.end()); // recycle aVec1 storage

I guess that users can simple use swap() to recycle storage but move-
assignment is now a standard notation for transfers of data (unlike
swap() that is merely a convention).

What do you think that is better usage-wise - to clear or not to clear
(post move)?

For standard types like unique_ptr I don't see any other option beside
of nullifying the source after the move. There is practically no gain
in leaving such unspecified when all implementations will have the
same behavior (nullify source) hence making this a de-facto standard.

> This was a recent change during the Madrid meeting that leads to
> the statement for MoveConstructible and MoveAssignable that the state of
> a move source is unspecified. Therefore I would say that people should
> better get used to the fact, that inspecting such a move source as a
> matter of program flow control is a very bad idea.

I think that the "unspecified state" hold only if the move actually
took place so, for example, in case of failed std::set insertion it's
guarantee that the source will be left untouched.

Thanks,
Rani


--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

On 2011-07-04 17:14, Rani Sharoni wrote:
>
>>> auto res1 = aSet.insert(move(spB));
>>> assert(res1.second ^ !spB); // take ownership on successful insert
>>
>>> auto res2 = aSet.insert(move(spD));
>>> assert(res2.second ^ !spD); // BUGBUG - spD is always null
>>
>> I find your analysis correct and pretty interesting. Nevertheless, I'm
>> yet not convinced that your scenarios describe gotchas of the real
>> world, because I believe that user-code would normally not inspect the
>> argument of std::move in a manner that you describe above. It is true
>> that library-provided types guarantee that the source of a move
>> operation is in an "unspecified but valid" state
>
> Interesting. I can't find this in the N3242 draft though it make
> sense.

It has been added late to the N3090 document. The relevant additions are:

a) 17.3.26 [defns.valid]:

"valid but unspecified state

an object state that is not specified except that the object   s
invariants are met and operations on the object behave as specified
for its type [ Example: If an object x of type std::vector<int> is in
a valid but unspecified state, x.empty() can be called
unconditionally, and x.front() can be called only if x.empty() returns
false.    end example ]"

b) 17.6.5.15 Moved-from state of library types [lib.types.movedfrom]:

"Objects of types defined in the C++ standard library may be moved
from (12.8). Move operations may be explicitly specified or implicitly
generated. Unless otherwise specified, such moved-from objects shall
be
placed in a valid but unspecified state."

> OTOH, the VC2010 move-constructor/assignment of containers explicitly
> clears the source argument though there is theoretical opportunity for
> storage recycling in case that move-assignment will simply use swap
> without clear:
> vector<string>  aVec1, aVec2;
> aVec1 = move(aVec2);  // aVec1.swap(aVec2);
> aVec2.assign(aList.begin(), aList.end()); // recycle aVec1 storage
>
> I guess that users can simple use swap() to recycle storage but move-
> assignment is now a standard notation for transfers of data (unlike
> swap() that is merely a convention).
>
> What do you think that is better usage-wise - to clear or not to clear
> (post move)?

I tend to argue in favor for clearing to ensure that memory capacities
at least from the hold values are freed. Clearing a vector alone won't
reduce it's capacity though, so there might be further arguments for
freeing the memory storage as well (via idioms like swap).

> For standard types like unique_ptr I don't see any other option beside
> of nullifying the source after the move. There is practically no gain
> in leaving such unspecified when all implementations will have the
> same behavior (nullify source) hence making this a de-facto standard.

For some particular types, especially std::unique_ptr, nullifying the
pointer is required to ensure that class invariants hold.

>> This was a recent change during the Madrid meeting that leads to
>> the statement for MoveConstructible and MoveAssignable that the state of
>> a move source is unspecified. Therefore I would say that people should
>> better get used to the fact, that inspecting such a move source as a
>> matter of program flow control is a very bad idea.
>
> I think that the "unspecified state" hold only if the move actually
> took place so, for example, in case of failed std::set insertion it's
> guarantee that the source will be left untouched.

Why?

Greetings from Bremen,

Daniel Kr   gler





--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

> > auto res1 = aSet.insert(move(spB));
> > assert(res1.second ^ !spB); // take ownership on successful insert
>
> > auto res2 = aSet.insert(move(spD));
> > assert(res2.second ^ !spD); // BUGBUG - spD is always null
> > </code>
>
> I don't think this is any worse than that the language lets you divide
> by zero or index an array out of range.

I think that the programmer is responsible to avoid such bugs while in
the above move issue the language/library can be of help with
balancing the safety of such constructs.

> By using std::move(spD) you say "take it, I don't care about it
> anymore". Then on the next line you DO care!

I actually wondered if std::move should be thought of as "allow move"
or "always move".
Syntactically speaking std::move only enables transfer of ownership
but doesn't force it so it's up to the programmer.
I personally got used that transfer of ownership is optional to allow
things like strong failure guarantee (e.g. no side effects if
std::insert failed).

> Isn't it a rare condition to try to insert into a set, but still
std::insert is just an example.

> wanting to keep the duplicate? Why not use a multiset in that case?
Maybe for retry on some transient state or insert to another
collection.
(I often use use maps but never the "multi" ones, i.e. map of lists).

In general users can write functions that uses rvalue-ref and need to
assure the strong guarantee.
I think that std::move(l-value) is expected to obey the "no side
effects" strong guarantee in general and the above unique_ptr case
seems cornered enough to cause much problems.

Rani


--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

On 2 Jul., 09:17, Rani Sharoni wrote:
>
> Consider the following code:
> struct B {};
> struct D : B {};
>
> set<unique_ptr<B>, ...> aSet;
>
> unique_ptr<B> spB = new D;

This kind of initialization is neither supported by unique_ptr nor by
shared_ptr. You need to use the direct initialization syntax. If your
standard library implementation accepts that, implementers apparently
forgot to make the corresponding constructors `explicit`.

> auto res1 = aSet.insert(move(spB));
> assert(res1.second ^ !spB); // take ownership on successful insert
>
> unique_ptr<D> spD = new D;
> auto res2 = aSet.insert(move(spD));
> assert(res2.second ^ !spD); // BUGBUG - spD is always null
> </code>
>
> In most cases func(move(x)) will not modify x unless func modified it
> but when implicit conversion is involved then 'x' will be moved
> regardless of the implementation of func since it's actually
> equivalent to func( U(move(x)) ).
>
> In the above the spD insertion is actually:
> aSet.insert(unique_ptr<B>(move(spD))); // take-over and try-insert
>
> Such call-sites are sensitive to whether conversion is needed and that
> can surprise the caller.
>
> It might possible to avoid such conversion using template insert:
> template<typename U> insert(U&& x); // implementation uses
> forward<U>(x)
>
> [...]
> Thoughts?

Interesting case. The problem here is that move() basically means "I
don't care anymore about this" but you actually do care in certain
cases. On the other hand I understand the desire to replace the
following code

 if (aSet.find(spD)==aSet.end()) {
   auto r = aSet.insert(move(spD));
   assert(r.second);
 } else {
   // keep using spD somehow
 }

with something that performs just a single search (instead of two) and
a _conditional_ move in one step. I believe `emplace` is what you
want:

 auto r = aSet.emplace(move(spD));
 if (!r.second) {
   // emplace did not construct anything, so
   // spD should still be valid here
 }

Cheers!
SG


--
[ comp.std.c++ is moderated.  To submit articles, try posting with your ]
[ newsreader.  If that fails, use mailto:std-cpp-submit@vandevoorde.com ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]