Skip to content

Commit

Permalink
reset moved-from variant to valueless state after the move
Browse files Browse the repository at this point in the history
This is the simplest way to achieve constant-time[1] complexity of
variant move construction and assignment.

[1] Constant with respect to the size of the recursive structure held
by the variant object; it's still linear in the number of alternative
types in the variant type.
  • Loading branch information
lightmare committed Feb 6, 2017
1 parent 03ca19f commit cc5e553
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 23 deletions.
17 changes: 4 additions & 13 deletions include/mapbox/recursive_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,6 @@ class recursive_wrapper
public:
using type = T;

/**
* Default constructor default initializes the internally stored value.
* For POD types this means nothing is done and the storage is
* uninitialized.
*
* @throws std::bad_alloc if there is insufficient memory for an object
* of type T.
* @throws any exception thrown by the default constructur of T.
*/
recursive_wrapper()
: p_(new T){}

~recursive_wrapper() noexcept { delete p_; }

recursive_wrapper(recursive_wrapper const& operand)
Expand All @@ -53,7 +41,10 @@ class recursive_wrapper
: p_(new T(operand)) {}

recursive_wrapper(recursive_wrapper&& operand)
: p_(new T(std::move(operand.get()))) {}
: p_(operand.p_)
{
operand.p_ = nullptr;
}

recursive_wrapper(T&& operand)
: p_(new T(std::move(operand))) {}
Expand Down
3 changes: 3 additions & 0 deletions include/mapbox/variant.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ struct variant_helper<T, Types...>
if (old_type_index == sizeof...(Types))
{
new (new_value) T(std::move(*reinterpret_cast<T*>(old_value)));
reinterpret_cast<T*>(old_value)->~T();
}
else
{
Expand Down Expand Up @@ -613,6 +614,7 @@ class variant
: type_index(old.type_index)
{
helper_type::move(old.type_index, &old.data, &data);
old.type_index = detail::invalid_value;
}

private:
Expand All @@ -630,6 +632,7 @@ class variant
type_index = detail::invalid_value;
helper_type::move(rhs.type_index, &rhs.data, &data);
type_index = rhs.type_index;
rhs.type_index = detail::invalid_value;
}

public:
Expand Down
11 changes: 2 additions & 9 deletions test/t/recursive_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ TEST_CASE("recursive wrapper of int")
rwi b{a};
REQUIRE(b.get() == 8);

rwi c;
rwi c{-1};
c = b;
REQUIRE(b.get() == 8);
REQUIRE(c.get() == 8);
Expand Down Expand Up @@ -114,13 +114,6 @@ TEST_CASE("swap")
TEST_CASE("recursive wrapper of pair<int, int>")
{

SECTION("default constructed")
{
rwp a;
REQUIRE(a.get().first == 0);
REQUIRE(a.get().second == 0);
}

SECTION("construct with value")
{
rwp a{std::make_pair(1, 2)};
Expand All @@ -139,7 +132,7 @@ TEST_CASE("recursive wrapper of pair<int, int>")
REQUIRE(b.get().first == 3);
REQUIRE(b.get().second == 4);

rwp c;
rwp c{{-1, -2}};
c = b;
REQUIRE(b.get().first == 3);
REQUIRE(b.get().second == 4);
Expand Down
1 change: 0 additions & 1 deletion test/t/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ TEST_CASE("variant can be moved into vector", "[variant]")
variant_type v(std::string("test"));
std::vector<variant_type> vec;
vec.emplace_back(std::move(v));
REQUIRE(v.get<std::string>() != std::string("test"));
REQUIRE(vec.at(0).get<std::string>() == std::string("test"));
}

Expand Down

0 comments on commit cc5e553

Please sign in to comment.