diff --git a/include/tsl/array_hash.h b/include/tsl/array_hash.h index 9003875..9455ccc 100644 --- a/include/tsl/array_hash.h +++ b/include/tsl/array_hash.h @@ -721,6 +721,20 @@ class array_bucket { 1); }; +// Store addressable bool objects instead of std::vector proxies so that +// the mapped-value reference and pointer types remain the same for every T. +class bool_value { + public: + template + explicit bool_value(Args&&... args) : m_value(std::forward(args)...) {} + + operator bool&() noexcept { return m_value; } + operator const bool&() const noexcept { return m_value; } + + private: + bool m_value; +}; + template class value_container { public: @@ -738,7 +752,10 @@ class value_container { static constexpr float VECTOR_GROWTH_RATE = 1.5f; // TODO use a sparse array? or a std::deque - std::vector m_values; + using stored_type = + typename std::conditional::value, bool_value, + T>::type; + std::vector m_values; }; template <> @@ -1448,11 +1465,11 @@ class array_hash : private value_container, return; } - std::vector new_values; + decltype(this->m_values) new_values; new_values.reserve(size()); for (auto it = begin(); it != end(); ++it) { - new_values.push_back(std::move_if_noexcept(it.value())); + new_values.emplace_back(std::move_if_noexcept(it.value())); } IndexSizeT ivalue = 0; @@ -1645,7 +1662,7 @@ class array_hash : private value_container, void serialize_bucket_values(Serializer& serializer, const array_bucket& bucket) const { for (auto it = bucket.begin(); it != bucket.end(); ++it) { - serializer(this->m_values[it.value()]); + serializer(static_cast(this->m_values[it.value()])); } } diff --git a/tests/array_map_tests.cpp b/tests/array_map_tests.cpp index 9d685c2..c4c3fb0 100644 --- a/tests/array_map_tests.cpp +++ b/tests/array_map_tests.cpp @@ -38,6 +38,7 @@ BOOST_AUTO_TEST_SUITE(test_array_map) using test_types = boost::mpl::list< tsl::array_map, tsl::array_map, + tsl::array_map, tsl::array_pg_map, tsl::array_map, tsl::array_map, tsl::array_map, @@ -1064,4 +1065,216 @@ BOOST_AUTO_TEST_CASE(test_precalculated_hash) { 0); } +/** + * bool mapped values must provide the same references as other mapped types. + */ +using bool_map_types = boost::mpl::list< + tsl::array_map, tsl::array_pg_map, + tsl::array_map, + tsl::ah::str_equal, false>>; + +BOOST_AUTO_TEST_CASE_TEMPLATE(test_bool_references, AMap, bool_map_types) { + static_assert(std::is_same::value, + "Mutable iterators must return bool references."); + static_assert(std::is_same::value, + "Const iterators must return const bool references."); + static_assert(std::is_same()["key"]), + bool&>::value, "operator[] must return bool&."); + static_assert(std::is_same().at("key")), + const bool&>::value, "at must return const bool&."); + static_assert(std::is_same() + .operator->()), bool*>::value, + "Mutable iterators must return bool pointers."); + static_assert(std::is_same() + .operator->()), const bool*>::value, + "Const iterators must return const bool pointers."); + + AMap map; + BOOST_CHECK_EQUAL(map["default"], false); + map["enabled"] = true; + map.insert("disabled", false); + const AMap& const_map = map; + auto it = map.find("enabled"); + auto cit = const_map.find("enabled"); + bool& value = it.value(); + const bool& const_value = cit.value(); + BOOST_CHECK(std::addressof(value) == std::addressof(map.at("enabled"))); + BOOST_CHECK(std::addressof(value) == it.operator->()); + BOOST_CHECK(std::addressof(const_value) == cit.operator->()); + BOOST_CHECK(std::addressof(value) == std::addressof(const_value)); + BOOST_CHECK(std::addressof(value) == std::addressof(const_map.at("enabled"))); + *it = false; + BOOST_CHECK_EQUAL(const_value, false); + *it.operator->() = true; + BOOST_CHECK_EQUAL(const_map.at("enabled"), true); + map.mutable_iterator(cit).value() = false; + BOOST_CHECK_EQUAL(const_value, false); + map.at("enabled") = true; + BOOST_CHECK_EQUAL(*cit, true); + for (bool& flag : map) { + flag = !flag; + } + BOOST_CHECK_EQUAL(const_map.at("enabled"), false); + BOOST_CHECK_EQUAL(const_map.at("disabled"), true); + BOOST_CHECK_EQUAL(const_map.at("default"), true); + BOOST_CHECK_THROW(map.at("missing"), std::out_of_range); + BOOST_CHECK_THROW(const_map.at("missing"), std::out_of_range); + BOOST_CHECK_EQUAL(map.size(), 3); + + const char binary_key[] = {'a', '\0', 'b'}; + map.insert_ks(binary_key, sizeof(binary_key), true); + map.at_ks(binary_key, sizeof(binary_key)) = false; + BOOST_CHECK_EQUAL(const_map.at_ks(binary_key, sizeof(binary_key)), false); + auto range = map.equal_range("enabled"); + BOOST_REQUIRE_EQUAL(std::distance(range.first, range.second), 1); + range.first.value() = true; + BOOST_CHECK_EQUAL(const_map.at("enabled"), true); +} + +struct explicit_bool_value { + explicit operator bool() const { return true; } +}; + +struct throwing_bool_value { + explicit operator bool() const { throw std::runtime_error("bool conversion"); } +}; + +BOOST_AUTO_TEST_CASE_TEMPLATE(test_bool_mutations, AMap, bool_map_types) { + AMap map; + map.emplace("empty"); + BOOST_CHECK_EQUAL(map.at("empty"), false); + map.emplace("explicit", explicit_bool_value{}); + BOOST_CHECK_EQUAL(map.at("explicit"), true); + BOOST_CHECK_THROW(map.emplace("throws", throwing_bool_value{}), + std::runtime_error); + BOOST_CHECK_EQUAL(map.count("throws"), 0); + BOOST_CHECK_EQUAL(map.size(), 2); + BOOST_CHECK_EQUAL(map.at("explicit"), true); + map.clear(); + + for (std::size_t i = 0; i < 200; ++i) { + map[utils::get_key(i)] = i % 2 != 0; + } + map.reserve(1000); + for (std::size_t i = 0; i < 200; i += 3) { + BOOST_CHECK_EQUAL(map.erase(utils::get_key(i)), 1); + } + map.shrink_to_fit(); + map.rehash(512); + for (std::size_t i = 0; i < 200; ++i) { + if (i % 3 == 0) { + BOOST_CHECK_EQUAL(map.count(utils::get_key(i)), 0); + } else { + BOOST_CHECK_EQUAL(map.at(utils::get_key(i)), i % 2 != 0); + } + } + auto duplicate = map.insert(utils::get_key(1), false); + BOOST_CHECK(!duplicate.second); + BOOST_CHECK_EQUAL(duplicate.first.value(), true); + auto assigned = map.insert_or_assign(utils::get_key(1), false); + BOOST_CHECK(!assigned.second); + BOOST_CHECK_EQUAL(assigned.first.value(), false); + BOOST_CHECK(map.insert_or_assign("new", true).second); + + const AMap expected(map); + AMap copied; + copied = map; + BOOST_CHECK(copied == expected); + AMap moved(std::move(map)); + BOOST_CHECK(moved == expected); + BOOST_CHECK(map.empty()); + map["reused"] = true; + BOOST_CHECK_EQUAL(map.at("reused"), true); + map = std::move(moved); + BOOST_CHECK(map == expected); + BOOST_CHECK(moved.empty()); + moved["reused"] = false; + BOOST_CHECK_EQUAL(moved.at("reused"), false); + using std::swap; + swap(map, moved); + BOOST_CHECK(moved == expected); + BOOST_CHECK_EQUAL(map.size(), 1); + map.clear(); + map.shrink_to_fit(); + BOOST_CHECK(map.empty()); + BOOST_CHECK(map.insert("after_clear", true).second); +} + +// Reject any implementation detail (such as a stored bool wrapper) deduced as +// a serializer argument, and record the exact mapped-value callback sequence. +class bool_serializer { + public: + template + void operator()(const T& value) { + static_assert(std::is_same::value || + std::is_same::value || + std::is_same::value, + "Only the documented serialization types are allowed."); + serial(value); + if (std::is_same::value) { + values.push_back(static_cast(value)); + } + } + + void operator()(const char32_t* data, std::uint64_t size) { + serial(data, size); + } + + serializer serial; + std::vector values; +}; + +using bool_serialization_types = boost::mpl::list< + tsl::array_map, tsl::array_pg_map>; + +BOOST_AUTO_TEST_CASE_TEMPLATE(test_bool_serialization, AMap, + bool_serialization_types) { + AMap map; + for (std::size_t i = 0; i < 100; ++i) { + map.insert(utils::get_key(i), i % 2 != 0); + } + // Leave some erased values awaiting compaction; they must not be serialized. + for (std::size_t i = 0; i < 10; ++i) { + map.erase(utils::get_key(i)); + } + bool_serializer serial; + map.serialize(serial); + std::vector expected; + for (const bool& value : map) { + expected.push_back(value); + } + BOOST_CHECK(serial.values == expected); + BOOST_CHECK_EQUAL(serial.values.size(), map.size()); + + for (bool hash_compatible : {false, true}) { + deserializer dserial(serial.serial.str()); + auto restored = AMap::deserialize(dserial, hash_compatible); + BOOST_CHECK(restored == map); + restored.at(utils::get_key(10)) = true; + BOOST_CHECK_EQUAL(restored.at(utils::get_key(10)), true); + restored.erase(utils::get_key(11)); + restored.shrink_to_fit(); + BOOST_CHECK_EQUAL(restored.at(utils::get_key(10)), true); + } + + for (std::size_t bucket_count : {std::size_t(0), std::size_t(1)}) { + AMap empty(bucket_count); + bool_serializer empty_serial; + empty.serialize(empty_serial); + BOOST_CHECK(empty_serial.values.empty()); + for (bool hash_compatible : {false, true}) { + deserializer dserial(empty_serial.serial.str()); + auto restored = AMap::deserialize(dserial, hash_compatible); + BOOST_CHECK(restored.empty()); + // Mutating a deserialized zero-bucket map is a separate existing issue, + // also reproducible with non-bool mapped values on the unmodified base. + if (restored.bucket_count() != 0) { + restored[U"new"] = true; + BOOST_CHECK_EQUAL(restored.at(U"new"), true); + } + } + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/utils.h b/tests/utils.h index a62db98..62a6732 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -176,6 +176,11 @@ inline std::basic_string utils::get_key( return key; } +template <> +inline bool utils::get_value(std::size_t counter) { + return counter % 2 != 0; +} + template <> inline std::int64_t utils::get_value(std::size_t counter) { return boost::numeric_cast(counter * 2);