diff --git a/include/continuable/detail/flat-variant.hpp b/include/continuable/detail/flat-variant.hpp index 384e286..ecc43e0 100644 --- a/include/continuable/detail/flat-variant.hpp +++ b/include/continuable/detail/flat-variant.hpp @@ -56,7 +56,7 @@ constexpr std::size_t max_element_of(std::initializer_list list) { return m; } template -constexpr std::size_t storage_of_impl() { +constexpr auto storage_of_impl() { constexpr auto size = max_element_of({sizeof(T)...}); constexpr auto align = max_element_of({alignof(T)...}); return std::aligned_storage_t{}; @@ -101,32 +101,37 @@ struct flat_variant_move_base { explicit flat_variant_move_base(flat_variant_move_base&& right) { Base& me = *static_cast(this); Base& other = *static_cast(&right); - assert(!other.is_empty()); -#ifndef _NDEBUG - me.set_slot(empty_slot::value); + if (other.is_empty()) { + me.set_slot(empty_slot::value); + } else { + + other.visit([&](auto&& value) { +#ifndef NDEBUG + me.set_slot(empty_slot::value); #endif + // NOLINTNEXTLINE(misc-move-forwarding-reference) + me.init(std::move(value), other.get_slot()); + }); + } - other.visit([&](auto&& value) { - // ... - me.init(std::move(value)); - }); - me.set_slot(other.get()); other.destroy(); } flat_variant_move_base& operator=(flat_variant_move_base const&) = default; flat_variant_move_base& operator=(flat_variant_move_base&& right) { Base& me = *static_cast(this); Base& other = *static_cast(&right); - assert(!other.is_empty()); me.weak_destroy(); - other.visit([&](auto&& value) { - // ... - me.init(std::move(value)); - }); - me.set_slot(other.get()); + if (other.is_empty()) { + me.set_slot(empty_slot::value); + } else { + other.visit([&](auto&& value) { + // ... + me.init(std::move(value), other.get_slot()); + }); + } other.destroy(); return *this; } @@ -142,17 +147,17 @@ struct flat_variant_copy_base : flat_variant_move_base { { Base& me = *static_cast(this); Base const& other = *static_cast(&right); - assert(!other.is_empty()); -#ifndef _NDEBUG - me.set_slot(empty_slot::value); + if (other.is_empty()) { + me.set_slot(empty_slot::value); + } else { + other.visit([&](auto&& value) { +#ifndef NDEBUG + me.set_slot(empty_slot::value); #endif - - other.visit([&](auto&& value) { - // ... - me.init(std::move(value)); - }); - me.set_slot(other.get()); + me.init(std::move(value), other.get_slot()); + }); + } } flat_variant_copy_base& operator=(flat_variant_copy_base&&) = default; flat_variant_copy_base& operator=(flat_variant_copy_base const& right) @@ -160,15 +165,17 @@ struct flat_variant_copy_base : flat_variant_move_base { { Base& me = *static_cast(this); Base const& other = *static_cast(&right); - assert(!other.is_empty()); me.weak_destroy(); - other.visit([&](auto&& value) { - // ... - me.init(std::move(value)); - }); - me.set_slot(other.get()); + if (other.is_empty()) { + me.set_slot(empty_slot::value); + } else { + other.visit([&](auto&& value) { + // ... + me.init(std::move(value), other.get_slot()); + }); + } return *this; } }; @@ -204,6 +211,8 @@ class flat_variant detail::every::value>, detail::flat_variant_base { + static_assert(sizeof...(T) > 0, "At least one paremeter T is required!"); + template friend class flat_variant; template @@ -213,9 +222,10 @@ class flat_variant template flat_variant(V&& value, detail::slot_t const slot) { - using type = std::decay_t; - new (&this->storage_) type(std::forward(value)); - set_slot(slot); +#ifndef NDEBUG + set_slot(detail::empty_slot::value); +#endif + init(std::forward(value), slot); } public: @@ -232,23 +242,22 @@ public: template < typename V, - std::enable_if_t>::value>* = nullptr, - std::size_t Index = traits::index_of_t, T...>::value> + std::enable_if_t>::value>* = nullptr> // Since the flat_variant isn't allowed through SFINAE // this overload is safed against the linted issue. // NOLINTNEXTLINE(misc-forwarding-reference-overload) explicit flat_variant(V&& value) - : flat_variant(std::forward(value), Index) { + : flat_variant(std::forward(value), + traits::index_of_t, T...>::value) { } template < typename V, - std::enable_if_t>::value>* = nullptr, - std::size_t Index = traits::index_of_t, T...>::value> + std::enable_if_t>::value>* = nullptr> flat_variant& operator=(V&& value) { weak_destroy(); - init(std::forward(value)); - set_slot(Index); + init(std::forward(value), + traits::index_of_t, T...>::value); return *this; } @@ -292,28 +301,27 @@ private: void visit(V&& visitor) { if (!is_empty()) { using callback_t = void (*)(flat_variant*, V &&); - constexpr callback_t const callbacks[] = { - &visit_dispatch... // ... - }; - callbacks[get()](this, std::forward(visitor)); + constexpr callback_t const callbacks[] = {&visit_dispatch...}; + callbacks[get_slot()](this, std::forward(visitor)); } } template void visit(V&& visitor) const { if (!is_empty()) { using callback_t = void (*)(flat_variant const*, V&&); - constexpr callback_t const callbacks[] = { - &visit_dispatch_const... // ... - }; - callbacks[get()](this, std::forward(visitor)); + constexpr callback_t const callbacks[] = {&visit_dispatch_const...}; + callbacks[get_slot()](this, std::forward(visitor)); } } template - void init(V&& value) { + void init(V&& value, detail::slot_t const slot) { assert(is_empty()); - using type = std::decay_t; + assert(sizeof(this->storage_) >= sizeof(std::decay_t)); + + using type = std::decay_t; new (&this->storage_) type(std::forward(value)); + set_slot(slot); } void destroy() { weak_destroy(); @@ -332,11 +340,11 @@ private: set_slot(detail::empty_slot::value); #endif } - detail::slot_t get() const noexcept { + detail::slot_t get_slot() const noexcept { return this->slot_; } bool is_slot(detail::slot_t const slot) const noexcept { - return get() == slot; + return get_slot() == slot; } void set_slot(detail::slot_t const slot) { this->slot_ = slot; diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt index f0c2d30..4ebec5d 100644 --- a/test/unit-test/CMakeLists.txt +++ b/test/unit-test/CMakeLists.txt @@ -16,6 +16,7 @@ target_link_libraries(test-continuable-base continuable-features-noexcept) add_executable(test-continuable-single + ${CMAKE_CURRENT_LIST_DIR}/single/test-continuable-flat-variant.cpp ${CMAKE_CURRENT_LIST_DIR}/single/test-continuable-expected.cpp ${CMAKE_CURRENT_LIST_DIR}/single/test-continuable-traverse.cpp ${CMAKE_CURRENT_LIST_DIR}/single/test-continuable-traverse-async.cpp) diff --git a/test/unit-test/single/test-continuable-flat-variant.cpp b/test/unit-test/single/test-continuable-flat-variant.cpp new file mode 100644 index 0000000..4b92572 --- /dev/null +++ b/test/unit-test/single/test-continuable-flat-variant.cpp @@ -0,0 +1,163 @@ + +/* + Copyright(c) 2015 - 2018 Denis Blank + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files(the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and / or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions : + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. +**/ + +#include +#include + +#include + +#include + +using cti::detail::container::flat_variant; + +static int const CANARY = 373671; + +using int_variant = flat_variant; + +TEST(flat_variant_single_test, is_move_constructible) { + { + int_variant e_old(CANARY); + int_variant e(std::move(e_old)); + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + EXPECT_EQ(e.cast(), CANARY); + } + + { + int_variant e(int_variant(tag1{})); + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + } +} + +TEST(flat_variant_single_test, is_value_move_assignable) { + int_variant old(CANARY); + int_variant e; + e = std::move(old); + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + EXPECT_EQ(e.cast(), CANARY); +} + +TEST(flat_variant_single_test, is_copy_constructible) { + { + int_variant const e_old(CANARY); + int_variant e(e_old); + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + EXPECT_EQ(e.cast(), CANARY); + } + + { + int_variant const e_old(tag1{}); + int_variant e(e_old); + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + } +} + +TEST(flat_variant_single_test, is_copy_assignable) { + { + int_variant const e_old(CANARY); + int_variant e; + e = e_old; + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + EXPECT_EQ(e.cast(), CANARY); + } + + { + int_variant const e_old(tag1{}); + int_variant e; + e = e_old; + + EXPECT_TRUE(bool(e)); + EXPECT_TRUE(e.is()); + } +} + +// This regression test shows a memory leak which happens when using the +// expected class move constructed from another expected object. +TEST(flat_variant_single_test, test_leak_regression) { + // expected_all_tests > > >::supply(int const&) + // const + // continuable/build/../test/unit-test/test-continuable-expected.cpp:52 + // 3: #3 0x11cf07a in + // expected_all_tests_is_value_assignable_Test > > >::TestBody() + // continuable/build/../test/unit-test/test-continuable-expected.cpp:133:15 + // 3: #4 0x1339e4e in void + // testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) + // continuable/build/../dep/googletest/googletest/googletest/src/gtest.cc:2395:10 + using type = std::shared_ptr; + + auto const validate = [](auto&& variant, std::size_t use) { + ASSERT_TRUE(variant.template is()); + ASSERT_EQ((*variant.template cast()), CANARY); + ASSERT_EQ(variant.template cast().use_count(), use); + }; + + bool destroyed = false; + { + type ptr(new int(CANARY), [&](int* val) { + destroyed = true; + delete val; + }); + + auto e1(flat_variant(std::move(ptr))); + validate(e1, 1); + + auto e2 = std::move(e1); + validate(e2, 1); + + flat_variant e3; + e3 = std::move(e2); + validate(e3, 1); + + { + flat_variant ec(e3); + validate(ec, 2); + } + + { + flat_variant ec = e3; + validate(ec, 2); + } + + { + flat_variant ec; + ec = e3; + validate(ec, 2); + } + } + + ASSERT_TRUE(destroyed); +}