[variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

classic Classic list List threaded Threaded
140 messages Options
1234 ... 7
Reply | Threaded
Open this post in threaded view
|

[variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Antony Polukhin
Current implementation of recursive_wrapper move constructor is not optimal:

recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
    : p_(new T(std::move(operand.get()) ))
{ }

During descussion were made following proposals:

I: Leave it as is
  - bad performance
  - can not provide noexcept guarantee

II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
  + good performance
  + provides noexcept guarantee for move constructor
  + optimization will be used even if varinat has no type with trivial
default constructor
  + easy to implement
  - triggers an assert when user tries to reuse moved object
  - adds an empty state to the recursive_wrapper

III: Make recursive_wrapper and variant cooperate, enable move for
varinat in the presence of recursive_wrappers only when there is at
least one type that is nothrow-default-constructible, regardless of
whether it's current. It is easyer to understand by example:
        typedef variant<int, recursive_wrapper<foo>> V;
        V v1( std::move(v2) );
        This move-constructs v1 from v2 and leaves int() into v2.
  + good performance
  + provides noexcept guarantee for rcursive_wrapper
  + does not adds an empty state
  - optimization won't trigger if varinat has no type with trivial
default constructor
  - hard to implement
  - user may be obscured by the fact, that v2 from the example now contains int

IV: After move away, delay construction of type held by
recursive_wrapper till it will be be required.
  +/- good performance? but more checks
  + provides noexcept guarantee for rcursive_wrapper
  + does not adds an explicit empty state
  + optimization will be used even if varinat has no type with trivial
default constructor
  +/- not hard to implement
  --- additional requirement for type T of recursive_wrapper: it must
be default constructible
  - less obvious behavior for user (for example get() function would
construct values, allocate memory and can possibly throw)


Please, vote for solution or propose a better one.

P.S.:
As I know Joel de Guzman votes for solution II (and totally agains
solution I, because it is too slow).
I wanted to leave as is (solution I) or use solution IV, but Joel
talked me to solution II.

P.P.S.:
More info can be obtained from ticket #7718
https://svn.boost.org/trac/boost/ticket/7718
 and previous series of letters.

--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Andrey Semashev-2
On Mon, Jan 21, 2013 at 5:39 PM, Antony Polukhin <[hidden email]> wrote:

[snip]

> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>   + good performance
>   + provides noexcept guarantee for move constructor
>   + optimization will be used even if varinat has no type with trivial
> default constructor
>   + easy to implement
>   - triggers an assert when user tries to reuse moved object
>   - adds an empty state to the recursive_wrapper

I'd prefer this option. IMHO, one of the major points of move
semantics is optimization and moved-from objects are as good as dead
anyway; I wouldn't even try to revive a moved-from object with
assignment. But this is a breaking change (is it?), so it should have
a prominent note in release notes.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Krzysztof Czainski
In reply to this post by Antony Polukhin
2013/1/21 Antony Polukhin <[hidden email]>

> Current implementation of recursive_wrapper move constructor is not
> optimal:
>
> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
>     : p_(new T(std::move(operand.get()) ))
> { }
>
> During descussion were made following proposals:
>
[...]

II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>

I vote for II, because:

  + good performance
>   + provides noexcept guarantee for move constructor
>   + optimization will be used even if varinat has no type with trivial
> default constructor
>   + easy to implement
>

And the above 4 points mean to me that it is a simple solution, which does
the job.

  - triggers an assert when user tries to reuse moved object
>

Yes, using the moved object in other cases then assigning something to it
is UB, so that's the right thing to do ;-)

  - adds an empty state to the recursive_wrapper


This empty state can only be achieved by moving from the object, right?
That's fine with me.

Regards,
Kris

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Joel de Guzman
In reply to this post by Andrey Semashev-2
On 1/21/13 10:29 PM, Andrey Semashev wrote:

> On Mon, Jan 21, 2013 at 5:39 PM, Antony Polukhin <[hidden email]> wrote:
>
> [snip]
>
>> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>>    + good performance
>>    + provides noexcept guarantee for move constructor
>>    + optimization will be used even if varinat has no type with trivial
>> default constructor
>>    + easy to implement
>>    - triggers an assert when user tries to reuse moved object
>>    - adds an empty state to the recursive_wrapper
>
> I'd prefer this option. IMHO, one of the major points of move
> semantics is optimization and moved-from objects are as good as dead
> anyway; I wouldn't even try to revive a moved-from object with
> assignment. But this is a breaking change (is it?), so it should have
> a prominent note in release notes.

No it's not (a breaking change). We don't have move prior to this patch.
The only issue is the one Paul Smith notes regarding conservative move
and that is only relevant in the extremely rare case when someone tries
to do something to a moved-from object. This notion of a "valid" state
is not clear to begin with anyway and many say it depends on the library
to define what "valid" means. The case of NaN is a very clear example
of a perfectly "valid" state similar to what we have here. To require
that an object not have such preconditions on operations after move will
have a negative impact on optimizations such as this.

Me? You know my vote. But Peter's suggestion (III) is also an acceptable
route.

Regards,
--
Joel de Guzman
http://www.boostpro.com
http://boost-spirit.com



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Paul Smith
In reply to this post by Antony Polukhin
On Mon, Jan 21, 2013 at 3:39 PM, Antony Polukhin <[hidden email]> wrote:
> Current implementation of recursive_wrapper move constructor is not optimal:
>
> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
>     : p_(new T(std::move(operand.get()) ))
> { }

First of all, let's make a distinction here: there's
recursive_wrapper, and there's a variant that contains a
recursive_wrapper. Peter's solution, for example, addresses the
latter, not the former.

>
> During descussion were made following proposals:
>
> I: Leave it as is
>   - bad performance
>   - can not provide noexcept guarantee

I take it as leave variant as is. I think we're already past that.

>
> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>   + good performance
>   + provides noexcept guarantee for move constructor

recursive_wrapper's move constructor. Not necessarily variant's.

>   + optimization will be used even if varinat has no type with trivial
> default constructor

And with a standalone recursive_wrapper.

>   + easy to implement
>   - triggers an assert when user tries to reuse moved object
>   - adds an empty state to the recursive_wrapper

Does it? Either recursive_wrapper has a documented, valid empty state,
or a "gee I hope noone ever steps here" pseudo-"state".

>
> III: Make recursive_wrapper and variant cooperate, enable move for
> varinat in the presence of recursive_wrappers only when there is at
> least one type that is nothrow-default-constructible, regardless of
> whether it's current.

We can enable move construction regardless. The presence of a cheap
type simply enables this optimization.

> It is easyer to understand by example:
>         typedef variant<int, recursive_wrapper<foo>> V;
>         V v1( std::move(v2) );
>         This move-constructs v1 from v2 and leaves int() into v2.
>   + good performance
>   + provides noexcept guarantee for rcursive_wrapper

No, not for recursive_wrapper.
recursive_wrapper's move constructor remains as-is. It is only when
recursive_wrapper is hosted inside a variant that we can do this. And
even then, if any of the other types has a throwing move/copy-ctor,
variant can't have a noexcept move-ctor (which is true for the other
solutions as well).

>   + does not adds an empty state
>   - optimization won't trigger if varinat has no type with trivial
> default constructor

And not with a standalone recursive_wrapper.

>   - hard to implement

Well, hardER. Variant already does something similar.

>   - user may be obscured by the fact, that v2 from the example now contains int

Not really. That's what a move constructor does. If the user wants
copy semantics he shouldn't move the variant.

>
> IV: After move away, delay construction of type held by
> recursive_wrapper till it will be be required.
>   +/- good performance? but more checks

More checks, dynamic allocation and possibly throwing in 'get'. Not a
very good thing for a previously trivial inline function. Not sure at
all that this is an overall win performance-wise.

>   + provides noexcept guarantee for rcursive_wrapper

And provides yesexcept for 'get' :-)

>   + does not adds an explicit empty state
>   + optimization will be used even if varinat has no type with trivial
> default constructor
>   +/- not hard to implement
>   --- additional requirement for type T of recursive_wrapper: it must
> be default constructible

+1

>   - less obvious behavior for user (for example get() function would
> construct values, allocate memory and can possibly throw)
>
>
> Please, vote for solution or propose a better one.

Solution 2, and any of its spinoffs, is just passing responsibility
from one point to the other. It's definitely not a "correct" solution.
If you feel confident enough to use it regardless, fine, but let's not
make this the default. For example, we can add an
optional_recursive_wrapper type. (I wouldn't suggest using a macro to
trigger this behavior since it can lead to ODR violations).

Solution 4 is probably worse than keeping variant as it is. Other than
performance implications, adding throwability to a currently trivial
function is something that should be avoided.

Solution 3 is the only one that plays by the rules. It's not a
universal solution, but it addresses enough cases and at least for now
it's good enoguh.
If you're put off by the intrusiveness of it - I understand. To
paraphrase this solution in a more generic way: when possible, the
conservative move-ctor of variant can destructively move the contained
value and construct a cheap type to cover it up. If we had a generic
way to do a destructive move, then variant wouldn't need to tinker
with recursive_wrapper's internals and we could apply the same
optimization for user types. I'd start with just recursive_wrapper
though.


I'd go with 3.

--
Paul Smith

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

ldionne
In reply to this post by Antony Polukhin
> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>   + good performance
>   + provides noexcept guarantee for move constructor
>   + optimization will be used even if varinat has no type with trivial
> default constructor
>   + easy to implement
>   - triggers an assert when user tries to reuse moved object
>   - adds an empty state to the recursive_wrapper

My vote goes to II.

    - Using moved-from objects should rarely happen, if ever.
    - It is not even a breaking change since move from variants was not
      supported in the past.

Moreover, I am explicitly against proposition III:

    - The state of the moved-from variant is not explicitly set by the
      programmer. I tend to prefer explicit to implicit.
    - The type held in the moved-from variant depends on the types in the
      variant, which could make it harder to use variants in generic code.
    - It is simpler to have a single, well-documented behavior. It introduces
      less complexity.

Question:
Would it be possible to assign to moved-from variant?
If so, proposition III is equivalent to proposition II with the following,
unless I am mistaken:

    typedef variant<int, recursive_wrapper<foo> > V;
    V v1(std::move(v2)); // using v2 now triggers an assertion
                         // (except when trying to set its content)
    v2 = int(); // this is explicit and equivalent to III

Just my 2 cents.

Louis Dionne



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Paul Smith
In reply to this post by Joel de Guzman
On Mon, Jan 21, 2013 at 6:12 PM, Joel de Guzman <[hidden email]> wrote:
> This notion of a "valid" state
> is not clear to begin with anyway and many say it depends on the library
> to define what "valid" means. The case of NaN is a very clear example
> of a perfectly "valid" state similar to what we have here.

I'm not sure what the NaN example supposed to convey? I think Nevin
brought it up to show that std::less<double> is not a weak partial
order. And guess what, it technically isn't! Try to put NaNs inside an
associative container and watch the world collapse. The big difference
is that no operation on non-NaNs that the container performs ever
produces a NaN out of thin air.

--
Paul Smith

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

cppljevans
In reply to this post by Antony Polukhin
On 01/21/13 07:39, Antony Polukhin wrote:
> Current implementation of recursive_wrapper move constructor is not
optimal:
>
> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
>     : p_(new T(std::move(operand.get()) ))
> { }
>
> During descussion were made following proposals:
[snip]

> III: Make recursive_wrapper and variant cooperate, enable move for
> varinat in the presence of recursive_wrappers only when there is at
> least one type that is nothrow-default-constructible, regardless of
> whether it's current. It is easyer to understand by example:
>         typedef variant<int, recursive_wrapper<foo>> V;
>         V v1( std::move(v2) );
>         This move-constructs v1 from v2 and leaves int() into v2.
>   + good performance
>   + provides noexcept guarantee for rcursive_wrapper
>   + does not adds an empty state
>   - optimization won't trigger if varinat has no type with trivial
>     default constructor
>   - hard to implement
>   - user may be obscured by the fact, that v2 from the example now
contains int
>
[snip]

What about III except the moved-from variant's which() method returns
-2 (or some other "signal" value) signalling the object has been moved
from?  The moved-from variant's destructor and assign operators would
check the which() value (as, I assume, they now do) and behave
accordingly (the destructor would simply do nothing except recover the
buffer memory).  The user, using the example you provide above, would
be, I believe, *less* obscured by this than v2 containing an int,
because now v2 would contain some "signal" value, such as boost::blank
which the user would realize meant something unusual had happened.
Implementation, AFAICT, would be very simple and there would be no
need to check if the variant had no type with trivial default
constructor because boost::blank *does* have a trivial default
constructor.

The one downside I see is this might break existing code.
Of course I'm probably missing something, and any enlightenment
would be appreciated.

-regards,
Larry




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Andrey Semashev-2
In reply to this post by Paul Smith
On Monday 21 January 2013 19:06:32 Paul Smith wrote:

> On Mon, Jan 21, 2013 at 6:12 PM, Joel de Guzman <[hidden email]> wrote:
> > This notion of a "valid" state
> > is not clear to begin with anyway and many say it depends on the library
> > to define what "valid" means. The case of NaN is a very clear example
> > of a perfectly "valid" state similar to what we have here.
>
> I'm not sure what the NaN example supposed to convey? I think Nevin
> brought it up to show that std::less<double> is not a weak partial
> order. And guess what, it technically isn't! Try to put NaNs inside an
> associative container and watch the world collapse. The big difference
> is that no operation on non-NaNs that the container performs ever
> produces a NaN out of thin air.

Why would a container try to order moved-from elements?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Peter Dimov-2
In reply to this post by Antony Polukhin
Antony Polukhin wrote:
> III: Make recursive_wrapper and variant cooperate, enable move for variant
> in the presence of recursive_wrappers only when there is at least one type
> that is nothrow-default-constructible, regardless of whether it's current.

That's not quite correct. Move for variant would be always enabled, it just
won't result in a NULL reference_wrapper. For example, if a variant<int,
string, reference_wrapper<string2>> contains int(5), it will continue to do
so after move; and if it contains string("..."), it will hold string() after
move. Only if it had a recursive_wrapper( string2("...") ) would it be int()
after move, and if there was no suitable int in the list, it would either
contain recursive_wrapper( string2("...") ), or recursive_wrapper(
string2() ), depending on how the move constructor of recursive_wrapper is
implemented.

In addition, I'd argue that regardless of whether recursive_wrapper is made
to be NULL after move, variant should still try to prevent exposing it to
the user in the situations where there is an "int" in the list.

> - user may be obscured by the fact, that v2 from the example now contains
> int

Users who are obscured by seeing an int() in v2 would be even more obscured
by seeing a crash if v2 had a NULL recursive_wrapper instead.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalueand C++11 features seupport)

Peter Dimov-2
In reply to this post by ldionne
Louis Dionne wrote:
> My vote goes to II.
>
>     - Using moved-from objects should rarely happen, if ever.
>     - It is not even a breaking change since move from variants was not
>       supported in the past.

That's not quite true. If a type doesn't have a move constructor, move is
supported and same as copy.

> Moreover, I am explicitly against proposition III:
>
>     - The state of the moved-from variant is not explicitly set by the
>       programmer. I tend to prefer explicit to implicit.
>     - The type held in the moved-from variant depends on the types in the
>       variant, which could make it harder to use variants in generic code.

I fail to grasp the logic that simultaneously holds that

>     - Using moved-from objects should rarely happen, if ever.

and then proceeds to argue that moved-from objects should not be left in
such-and-such valid state, but must be left in an invalid state instead.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Robert Ramey
In reply to this post by Antony Polukhin
Antony Polukhin wrote:
> Current implementation of recursive_wrapper move constructor is not
> optimal:
>
> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
>    : p_(new T(std::move(operand.get()) ))
> { }
>
> During descussion were made following proposals:

<snip>

> Please, vote for solution or propose a better one.

A very good post and interesting discussion.  But one small sticking point
in my
opinion.  I would prefer that the word "vote" be replaced with "make
recommendation".

The word "vote" suggests that all input will be weighted equally.  Our
traditional
reviewing process recognises that all input is not equal and assigns to one
person - review manager - the role of weighing all the input.  In some cases
the review manager may (and I assume has) over-ruled the majority because
the arguments of the minority were better in his view.  So I would have
preferred that the the post used the phrase

"Please submit recommendation on design"

rather than

"Please vote for behavior"

Robert Ramey







_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Sergey Cheban
In reply to this post by Antony Polukhin
On 21.01.2013 17:39, Antony Polukhin wrote:

 > I: Leave it as is
 >    - bad performance
 >    - can not provide noexcept guarantee
 >
I vote against this solution because of it's disadvantages.

> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>    + good performance
>    + provides noexcept guarantee for move constructor
>    + optimization will be used even if varinat has no type with trivial
> default constructor
>    + easy to implement
>    - triggers an assert when user tries to reuse moved object
>    - adds an empty state to the recursive_wrapper
Let's think about non-recursive variants. For example:

// both CFileObj and CDatabaseObj are movable, not copyable and
// not nothrow-default-constructible
boost::variant< CFileObj, CDatabaseObj > v1( CFileObj(SomeFileName) );
boost::variant< CFileObj, CDatabaseObj > v2( std::move( v1 ) );

// What would be the state of v1 at this point?

It seems to me that the second solution solves only a part of the
problem. So, I don't vote for it.

>
> III: Make recursive_wrapper and variant cooperate, enable move for
> varinat in the presence of recursive_wrappers only when there is at
> least one type that is nothrow-default-constructible, regardless of
> whether it's current. It is easyer to understand by example:
>          typedef variant<int, recursive_wrapper<foo>> V;
>          V v1( std::move(v2) );
>          This move-constructs v1 from v2 and leaves int() into v2.
>    + good performance
>    + provides noexcept guarantee for rcursive_wrapper
>    + does not adds an empty state
>    - optimization won't trigger if varinat has no type with trivial
> default constructor
>    - hard to implement
>    - user may be obscured by the fact, that v2 from the example now contains int
I think this is a best solution. We have to leave the moved-from object
in some valid state and cannot specify this state in the code. So, we
need the default constructor (or, may be, something else).

> IV: After move away, delay construction of type held by
> recursive_wrapper till it will be be required.
>    +/- good performance? but more checks
>    + provides noexcept guarantee for rcursive_wrapper
>    + does not adds an explicit empty state
>    + optimization will be used even if varinat has no type with trivial
> default constructor
>    +/- not hard to implement
>    --- additional requirement for type T of recursive_wrapper: it must
> be default constructible
>    - less obvious behavior for user (for example get() function would
> construct values, allocate memory and can possibly throw)
I think that get() MUST NOT throw. So, I vote against this solution.

--
Sergey Cheban


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Andrey Semashev-2
On January 21, 2013 11:27:36 PM Sergey Cheban <[hidden email]> wrote:

>
> > II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
> >    + good performance
> >    + provides noexcept guarantee for move constructor
> >    + optimization will be used even if varinat has no type with trivial
> > default constructor
> >    + easy to implement
> >    - triggers an assert when user tries to reuse moved object
> >    - adds an empty state to the recursive_wrapper
> Let's think about non-recursive variants. For example:
>
> // both CFileObj and CDatabaseObj are movable, not copyable and
> // not nothrow-default-constructible
> boost::variant< CFileObj, CDatabaseObj > v1( CFileObj(SomeFileName) );
> boost::variant< CFileObj, CDatabaseObj > v2( std::move( v1 ) );
>
> // What would be the state of v1 at this point?

Moved-from CFileObj, I presume.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Antony Polukhin
2013/1/21 Andrey Semashev <[hidden email]>:

> On January 21, 2013 11:27:36 PM Sergey Cheban <[hidden email]> wrote:
>>
>>
>> > II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>> >    + good performance
>> >    + provides noexcept guarantee for move constructor
>> >    + optimization will be used even if varinat has no type with trivial
>> > default constructor
>> >    + easy to implement
>> >    - triggers an assert when user tries to reuse moved object
>> >    - adds an empty state to the recursive_wrapper
>> Let's think about non-recursive variants. For example:
>>
>> // both CFileObj and CDatabaseObj are movable, not copyable and
>> // not nothrow-default-constructible
>> boost::variant< CFileObj, CDatabaseObj > v1( CFileObj(SomeFileName) );
>> boost::variant< CFileObj, CDatabaseObj > v2( std::move( v1 ) );
>>
>> // What would be the state of v1 at this point?
>
>
> Moved-from CFileObj, I presume.

Correct, variant will contain CFileObj that was moved.

I've got some nice idea from discussion: nullable variant. If many
people use or want to use a variant with a type, that represents empty
state, we can create a separate nullable variant class. I think that
nullable variant can be implemented more efficient that the current
variant. For example we can simplify copy/move constructors and
assignment operators, guarantee fast noexcept default construction,
simplify metaprogamming and reduce compilation times. Maybe someone
want to implement it?

--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Jeffrey Lee Hellrung, Jr.-2
In reply to this post by Antony Polukhin
On Mon, Jan 21, 2013 at 5:39 AM, Antony Polukhin <[hidden email]>wrote:

> Current implementation of recursive_wrapper move constructor is not
> optimal:
>
> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand)
>     : p_(new T(std::move(operand.get()) ))
> { }
>
> During descussion were made following proposals:
>
> I: Leave it as is
>
[...]

> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>
[...]

> III: Make recursive_wrapper and variant cooperate, enable move for
> varinat in the presence of recursive_wrappers only when there is at
> least one type that is nothrow-default-constructible, regardless of
> whether it's current.

[...]

> IV: After move away, delay construction of type held by
> recursive_wrapper till it will be be required.
>
[...]

> Please, vote for solution or propose a better one.
>

IMHO, III is the only strictly correct solution, given variant's present
never-empty guarantee (which the original author deemed pretty important,
given the amount of documentation dedicated to it).

Aside: recursive_wrapper is only used within the context of a variant,
right?

- Jeff

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalueand C++11 features seupport)

ldionne
In reply to this post by Peter Dimov-2
Peter Dimov <lists <at> pdimov.com> writes:

>
> Louis Dionne wrote:
> > My vote goes to II.
> >
> >     - Using moved-from objects should rarely happen, if ever.
> >     - It is not even a breaking change since move from variants was not
> >       supported in the past.
>
> That's not quite true. If a type doesn't have a move constructor, move is
> supported and same as copy.

At first, I thought using proposition II would introduce a breaking change.
After reading Joel de Guzman's post, I learned this was not the case. What I
meant is that we don't run the risk of breaking existing code, which is one
less reason of not adopting proposition II. Still, thanks for the precision.

> > [snip]
>
> I fail to grasp the logic that simultaneously holds that
>
> >     - Using moved-from objects should rarely happen, if ever.
>
> and then proceeds to argue that moved-from objects should not be left in
> such-and-such valid state, but must be left in an invalid state instead.

In my post, I assume that the rare situations in which a moved-from variant
is used are programming errors. In such case, I would definitely prefer the
variant to be in an invalid state and to get an assertion ASAP so I can fix
my code.

With proposition III, using a moved-from variant implies that you know what's
in it after the move, i.e. the type of the held object and the fact that it is
default constructed. In such case, it is equivalent to create a default
constructed object of that type and to use it instead.

With proposition II, using a moved-from variant is just a no-no. You can
create a default constructed instance of the type you want if you need to
instead of relying on the implicit behavior of the moved-from variant.

The above assumes that the variant types contain a recursive_wrapper.

Best,

Louis Dionne



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalueand C++11 features seupport)

Peter Dimov-2
Louis Dionne wrote:
> With proposition III, using a moved-from variant implies that you know
> what's
> in it after the move, i.e. the type of the held object and the fact that
> it is
> default constructed.

No, [III] doesn't generally guarantee a specific value after a move. It just
guarantees a valid object; that is, one that doesn't crash or assert if you
try to use it. "Valid but unspecified" is the classic definition of a
moved-from object.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Sergey Cheban
In reply to this post by Andrey Semashev-2
21.01.2013 23:37, Andrey Semashev пишет:

> On January 21, 2013 11:27:36 PM Sergey Cheban <[hidden email]> wrote:
>>
>>> II: Set operand.p_ to NULL, add BOOST_ASSERT in get operations
>>>     + good performance
>>>     + provides noexcept guarantee for move constructor
>>>     + optimization will be used even if varinat has no type with trivial
>>> default constructor
>>>     + easy to implement
>>>     - triggers an assert when user tries to reuse moved object
>>>     - adds an empty state to the recursive_wrapper
>> Let's think about non-recursive variants. For example:
>>
>> // both CFileObj and CDatabaseObj are movable, not copyable and
>> // not nothrow-default-constructible
>> boost::variant< CFileObj, CDatabaseObj > v1( CFileObj(SomeFileName) );
>> boost::variant< CFileObj, CDatabaseObj > v2( std::move( v1 ) );
>>
>> // What would be the state of v1 at this point?
>
> Moved-from CFileObj, I presume.
Got it.
So, the reasonable and consistent value for the moved-from
variant<recursive_wrapper,int> is a moved-from recursive_wrapper, right?
In many cases, the moved-from state of the variables is equal to the
default-constructed state but this rule is not obligatory.

So, I think that NULL is a valid state of the moved-from
recursive_wrapper and the 2nd proposal is not worse (at least) than the
3rd one.




--
Best regards,
Sergey Cheban


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [variant] Please vote for behavior (Was: Basic rvalue and C++11 features seupport)

Joel de Guzman
In reply to this post by Antony Polukhin
On 1/22/13 3:57 AM, Antony Polukhin wrote:
> I've got some nice idea from discussion: nullable variant. If many
> people use or want to use a variant with a type, that represents empty
> state, we can create a separate nullable variant class. I think that
> nullable variant can be implemented more efficient that the current
> variant. For example we can simplify copy/move constructors and
> assignment operators, guarantee fast noexcept default construction,
> simplify metaprogamming and reduce compilation times. Maybe someone
> want to implement it?

I like it! If you implement it, I'll be your first user :-)
I don't really care much about this "never empty" guarantee and
I think it's not really worth the trouble.

Regards,
--
Joel de Guzman
http://www.boostpro.com
http://boost-spirit.com



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
1234 ... 7