On 01/13/13 13:33, Vicente J. Botet Escriba wrote:
Le 13/01/13 09:58, Antony Polukhin a écrit :
2013/1/6 Joel de Guzman <djowel@gmail.com>:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is: ...
I`ve finally got back from vacation, so let me add my 2 kopecks.
We can make move constructor fast and noexcept (just like Joel proposed) and also guarantee non-empty behavior. The idea is to delay construction of object:
T& get() { if (!get_pointer()) p_ = new T; return *get_pointer(); }
const T& get() const { if (!get_pointer()) p_ = new T; // mark p_ as mutable return *get_pointer(); }
So if move occurred object is empty, and if it is required again - it is default constructed.
Any objections?
This would make less efficient the get operation. I would prefer to force the boost::blank default construction.
Best, Vicente
+2 for this. According to what Peter says here: http://lists.boost.org/Archives/boost/2013/01/200132.php the problems stem from the never-empty guarantee: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... That page indicates the problem occurs when some unexpected event, such as an exception during target copy construction, happens. For example, the above variant doc says: The crux of the problem, then, is this: in the event that copy-construction of the content of v2 fails, how can v1 maintain its "never-empty" guarantee? By the time copy-construction from v2 is attempted, v1 has already destroyed its content! The docs on the current solution for this problem: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... detail the solution steps as follows: 1. Copy-construct the content of the left-hand side to the heap; call the pointer to this data backup. 2. Destroy the content of the left-hand side. 3. Copy-construct the content of the right-hand side in the (now-empty) storage of the left-hand side. 4. In the event of failure, copy backup to the left-hand side storage. 5. In the event of success, deallocate the data pointed to by backup. Thus, in the event of failure(item 4 in the above), the current solution restores the left-hand side to it's previous state. Thus, there is no indication in the variant that anything has gone wrong other than the fact that the variant's value has not changed from it's state before the assignment. This test for the exact same value seems to be an "additional complexity-of-use" which, according to: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... the never-empty guarantee was supposed to insulate the user from. Wouldn't the "implicit" boost::blank default construction be a "less complexity-of-use" alternative? IOW, when an exception occurs during an assignment of two distinct types, the variant's type is changed to boost::blank even when that is not among the explicit bounded types. This would be a flag to the user that something unusual had happened and would easily be tested for with the which() function which would return some value (e.g. -2 or -999 or whatever) indicating that the value is "undefined". -regards, Larry