From 6d9ea2b46f470406e1f49acc30b272c6e9f6cc5e Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Wed, 28 Sep 2022 10:44:11 -0700 Subject: Add common_policy_traits - a subset of hash_policy_traits that can be shared between raw_hash_set and btree. Also remove the transfer implementations from btree_set.h and flat_hash_set.h, which are equivalent to the default implementations. Motivation: this will simplify upcoming changes related to trivial relocation. PiperOrigin-RevId: 477493403 Change-Id: I75babef4c93dec3a8105f86c58af54199bb1ec9c --- CMake/AbseilDll.cmake | 1 + absl/container/BUILD.bazel | 26 ++++- absl/container/CMakeLists.txt | 27 +++++ absl/container/btree_set.h | 6 -- absl/container/flat_hash_set.h | 7 -- absl/container/internal/btree.h | 25 +---- absl/container/internal/common_policy_traits.h | 101 +++++++++++++++++ .../internal/common_policy_traits_test.cc | 119 +++++++++++++++++++++ absl/container/internal/hash_policy_traits.h | 59 +--------- absl/container/internal/hash_policy_traits_test.cc | 64 ----------- 10 files changed, 279 insertions(+), 156 deletions(-) create mode 100644 absl/container/internal/common_policy_traits.h create mode 100644 absl/container/internal/common_policy_traits_test.cc diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index cf6ec87b..d8ddcb3b 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -71,6 +71,7 @@ set(ABSL_INTERNAL_DLL_FILES "container/internal/btree.h" "container/internal/btree_container.h" "container/internal/common.h" + "container/internal/common_policy_traits.h" "container/internal/compressed_tuple.h" "container/internal/container_memory.h" "container/internal/counting_allocator.h" diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index d749c1ca..71afe9d2 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -460,7 +460,10 @@ cc_library( hdrs = ["internal/hash_policy_traits.h"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, - deps = ["//absl/meta:type_traits"], + deps = [ + ":common_policy_traits", + "//absl/meta:type_traits", + ], ) cc_test( @@ -474,6 +477,26 @@ cc_test( ], ) +cc_library( + name = "common_policy_traits", + hdrs = ["internal/common_policy_traits.h"], + copts = ABSL_DEFAULT_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + visibility = ["//visibility:private"], + deps = ["//absl/meta:type_traits"], +) + +cc_test( + name = "common_policy_traits_test", + srcs = ["internal/common_policy_traits_test.cc"], + copts = ABSL_TEST_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + deps = [ + ":common_policy_traits", + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "hashtable_debug", hdrs = ["internal/hashtable_debug.h"], @@ -905,6 +928,7 @@ cc_library( visibility = ["//visibility:public"], deps = [ ":common", + ":common_policy_traits", ":compressed_tuple", ":container_memory", ":layout", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 4155f335..a3fdb969 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -28,6 +28,7 @@ absl_cc_library( ${ABSL_DEFAULT_LINKOPTS} DEPS absl::container_common + absl::common_policy_traits absl::compare absl::compressed_tuple absl::container_memory @@ -531,6 +532,7 @@ absl_cc_library( COPTS ${ABSL_DEFAULT_COPTS} DEPS + absl::common_policy_traits absl::meta PUBLIC ) @@ -547,6 +549,31 @@ absl_cc_test( GTest::gmock_main ) +# Internal-only target, do not depend on directly. +absl_cc_library( + NAME + common_policy_traits + HDRS + "internal/common_policy_traits.h" + COPTS + ${ABSL_DEFAULT_COPTS} + DEPS + absl::meta + PUBLIC +) + +absl_cc_test( + NAME + common_policy_traits_test + SRCS + "internal/common_policy_traits_test.cc" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::common_policy_traits + GTest::gmock_main +) + # Internal-only target, do not depend on directly. absl_cc_library( NAME diff --git a/absl/container/btree_set.h b/absl/container/btree_set.h index 695b09f5..e823a2a0 100644 --- a/absl/container/btree_set.h +++ b/absl/container/btree_set.h @@ -760,12 +760,6 @@ struct set_slot_policy { static void destroy(Alloc *alloc, slot_type *slot) { absl::allocator_traits::destroy(*alloc, slot); } - - template - static void transfer(Alloc *alloc, slot_type *new_slot, slot_type *old_slot) { - construct(alloc, new_slot, old_slot); - destroy(alloc, old_slot); - } }; // A parameters structure for holding the type parameters for a btree_set. diff --git a/absl/container/flat_hash_set.h b/absl/container/flat_hash_set.h index 4938c703..f5376f99 100644 --- a/absl/container/flat_hash_set.h +++ b/absl/container/flat_hash_set.h @@ -474,13 +474,6 @@ struct FlatHashSetPolicy { absl::allocator_traits::destroy(*alloc, slot); } - template - static void transfer(Allocator* alloc, slot_type* new_slot, - slot_type* old_slot) { - construct(alloc, new_slot, std::move(*old_slot)); - destroy(alloc, old_slot); - } - static T& element(slot_type* slot) { return *slot; } template diff --git a/absl/container/internal/btree.h b/absl/container/internal/btree.h index 203823b0..ecf31bea 100644 --- a/absl/container/internal/btree.h +++ b/absl/container/internal/btree.h @@ -61,6 +61,7 @@ #include "absl/base/internal/raw_logging.h" #include "absl/base/macros.h" #include "absl/container/internal/common.h" +#include "absl/container/internal/common_policy_traits.h" #include "absl/container/internal/compressed_tuple.h" #include "absl/container/internal/container_memory.h" #include "absl/container/internal/layout.h" @@ -356,7 +357,7 @@ class map_value_compare { template -struct common_params { +struct common_params : common_policy_traits { using original_key_compare = Compare; // If Compare is a common comparator for a string-like type, then we adapt it @@ -438,28 +439,6 @@ struct common_params { absl::conditional_t<(kNodeSlotSpace / sizeof(slot_type) > (std::numeric_limits::max)()), uint16_t, uint8_t>; // NOLINT - - // The following methods are necessary for passing this struct as PolicyTraits - // for node_handle and/or are used within btree. - static value_type &element(slot_type *slot) { - return slot_policy::element(slot); - } - static const value_type &element(const slot_type *slot) { - return slot_policy::element(slot); - } - template - static void construct(Alloc *alloc, slot_type *slot, Args &&...args) { - slot_policy::construct(alloc, slot, std::forward(args)...); - } - static void construct(Alloc *alloc, slot_type *slot, slot_type *other) { - slot_policy::construct(alloc, slot, other); - } - static void destroy(Alloc *alloc, slot_type *slot) { - slot_policy::destroy(alloc, slot); - } - static void transfer(Alloc *alloc, slot_type *new_slot, slot_type *old_slot) { - slot_policy::transfer(alloc, new_slot, old_slot); - } }; // An adapter class that converts a lower-bound compare into an upper-bound diff --git a/absl/container/internal/common_policy_traits.h b/absl/container/internal/common_policy_traits.h new file mode 100644 index 00000000..cc2e89ba --- /dev/null +++ b/absl/container/internal/common_policy_traits.h @@ -0,0 +1,101 @@ +// Copyright 2022 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef ABSL_CONTAINER_INTERNAL_COMMON_POLICY_TRAITS_H_ +#define ABSL_CONTAINER_INTERNAL_COMMON_POLICY_TRAITS_H_ + +#include +#include +#include +#include +#include + +#include "absl/meta/type_traits.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace container_internal { + +// Defines how slots are initialized/destroyed/moved. +template +struct common_policy_traits { + // The actual object stored in the container. + using slot_type = typename Policy::slot_type; + + + // PRECONDITION: `slot` is UNINITIALIZED + // POSTCONDITION: `slot` is INITIALIZED + template + static void construct(Alloc* alloc, slot_type* slot, Args&&... args) { + Policy::construct(alloc, slot, std::forward(args)...); + } + + // PRECONDITION: `slot` is INITIALIZED + // POSTCONDITION: `slot` is UNINITIALIZED + template + static void destroy(Alloc* alloc, slot_type* slot) { + Policy::destroy(alloc, slot); + } + + // Transfers the `old_slot` to `new_slot`. Any memory allocated by the + // allocator inside `old_slot` to `new_slot` can be transferred. + // + // OPTIONAL: defaults to: + // + // clone(new_slot, std::move(*old_slot)); + // destroy(old_slot); + // + // PRECONDITION: `new_slot` is UNINITIALIZED and `old_slot` is INITIALIZED + // POSTCONDITION: `new_slot` is INITIALIZED and `old_slot` is + // UNINITIALIZED + template + static void transfer(Alloc* alloc, slot_type* new_slot, slot_type* old_slot) { + transfer_impl(alloc, new_slot, old_slot, 0); + } + + // PRECONDITION: `slot` is INITIALIZED + // POSTCONDITION: `slot` is INITIALIZED + // Note: we use remove_const_t so that the two overloads have different args + // in the case of sets with explicitly const value_types. + template + static auto element(absl::remove_const_t* slot) + -> decltype(P::element(slot)) { + return P::element(slot); + } + template + static auto element(const slot_type* slot) -> decltype(P::element(slot)) { + return P::element(slot); + } + + private: + // Use auto -> decltype as an enabler. + template + static auto transfer_impl(Alloc* alloc, slot_type* new_slot, + slot_type* old_slot, int) + -> decltype((void)P::transfer(alloc, new_slot, old_slot)) { + P::transfer(alloc, new_slot, old_slot); + } + template + static void transfer_impl(Alloc* alloc, slot_type* new_slot, + slot_type* old_slot, char) { + construct(alloc, new_slot, std::move(element(old_slot))); + destroy(alloc, old_slot); + } +}; + +} // namespace container_internal +ABSL_NAMESPACE_END +} // namespace absl + +#endif // ABSL_CONTAINER_INTERNAL_COMMON_POLICY_TRAITS_H_ diff --git a/absl/container/internal/common_policy_traits_test.cc b/absl/container/internal/common_policy_traits_test.cc new file mode 100644 index 00000000..768d870e --- /dev/null +++ b/absl/container/internal/common_policy_traits_test.cc @@ -0,0 +1,119 @@ +// Copyright 2022 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/container/internal/common_policy_traits.h" + +#include +#include +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace container_internal { +namespace { + +using ::testing::MockFunction; +using ::testing::Return; +using ::testing::ReturnRef; + +using Slot = int; + +struct PolicyWithoutOptionalOps { + using slot_type = Slot; + using key_type = Slot; + using init_type = Slot; + + static std::function construct; + static std::function destroy; + + static std::function element; +}; + +std::function PolicyWithoutOptionalOps::construct; +std::function PolicyWithoutOptionalOps::destroy; + +std::function PolicyWithoutOptionalOps::element; + +struct PolicyWithOptionalOps : PolicyWithoutOptionalOps { + static std::function transfer; +}; + +std::function PolicyWithOptionalOps::transfer; + +struct Test : ::testing::Test { + Test() { + PolicyWithoutOptionalOps::construct = [&](void* a1, Slot* a2, Slot a3) { + construct.Call(a1, a2, std::move(a3)); + }; + PolicyWithoutOptionalOps::destroy = [&](void* a1, Slot* a2) { + destroy.Call(a1, a2); + }; + + PolicyWithoutOptionalOps::element = [&](Slot* a1) -> Slot& { + return element.Call(a1); + }; + + PolicyWithOptionalOps::transfer = [&](void* a1, Slot* a2, Slot* a3) { + return transfer.Call(a1, a2, a3); + }; + } + + std::allocator alloc; + int a = 53; + + MockFunction construct; + MockFunction destroy; + + MockFunction element; + + MockFunction transfer; +}; + +TEST_F(Test, construct) { + EXPECT_CALL(construct, Call(&alloc, &a, 53)); + common_policy_traits::construct(&alloc, &a, 53); +} + +TEST_F(Test, destroy) { + EXPECT_CALL(destroy, Call(&alloc, &a)); + common_policy_traits::destroy(&alloc, &a); +} + +TEST_F(Test, element) { + int b = 0; + EXPECT_CALL(element, Call(&a)).WillOnce(ReturnRef(b)); + EXPECT_EQ(&b, &common_policy_traits::element(&a)); +} + +TEST_F(Test, without_transfer) { + int b = 42; + EXPECT_CALL(element, Call(&b)).WillOnce(::testing::ReturnRef(b)); + EXPECT_CALL(construct, Call(&alloc, &a, b)); + EXPECT_CALL(destroy, Call(&alloc, &b)); + common_policy_traits::transfer(&alloc, &a, &b); +} + +TEST_F(Test, with_transfer) { + int b = 42; + EXPECT_CALL(transfer, Call(&alloc, &a, &b)); + common_policy_traits::transfer(&alloc, &a, &b); +} + +} // namespace +} // namespace container_internal +ABSL_NAMESPACE_END +} // namespace absl diff --git a/absl/container/internal/hash_policy_traits.h b/absl/container/internal/hash_policy_traits.h index 46c97b18..164ec123 100644 --- a/absl/container/internal/hash_policy_traits.h +++ b/absl/container/internal/hash_policy_traits.h @@ -21,6 +21,7 @@ #include #include +#include "absl/container/internal/common_policy_traits.h" #include "absl/meta/type_traits.h" namespace absl { @@ -29,7 +30,7 @@ namespace container_internal { // Defines how slots are initialized/destroyed/moved. template -struct hash_policy_traits { +struct hash_policy_traits : common_policy_traits { // The type of the keys stored in the hashtable. using key_type = typename Policy::key_type; @@ -87,43 +88,6 @@ struct hash_policy_traits { // Defaults to false if not provided by the policy. using constant_iterators = ConstantIteratorsImpl<>; - // PRECONDITION: `slot` is UNINITIALIZED - // POSTCONDITION: `slot` is INITIALIZED - template - static void construct(Alloc* alloc, slot_type* slot, Args&&... args) { - Policy::construct(alloc, slot, std::forward(args)...); - } - - // PRECONDITION: `slot` is INITIALIZED - // POSTCONDITION: `slot` is UNINITIALIZED - template - static void destroy(Alloc* alloc, slot_type* slot) { - Policy::destroy(alloc, slot); - } - - // Transfers the `old_slot` to `new_slot`. Any memory allocated by the - // allocator inside `old_slot` to `new_slot` can be transferred. - // - // OPTIONAL: defaults to: - // - // clone(new_slot, std::move(*old_slot)); - // destroy(old_slot); - // - // PRECONDITION: `new_slot` is UNINITIALIZED and `old_slot` is INITIALIZED - // POSTCONDITION: `new_slot` is INITIALIZED and `old_slot` is - // UNINITIALIZED - template - static void transfer(Alloc* alloc, slot_type* new_slot, slot_type* old_slot) { - transfer_impl(alloc, new_slot, old_slot, 0); - } - - // PRECONDITION: `slot` is INITIALIZED - // POSTCONDITION: `slot` is INITIALIZED - template - static auto element(slot_type* slot) -> decltype(P::element(slot)) { - return P::element(slot); - } - // Returns the amount of memory owned by `slot`, exclusive of `sizeof(*slot)`. // // If `slot` is nullptr, returns the constant amount of memory owned by any @@ -174,8 +138,8 @@ struct hash_policy_traits { // Used for node handle manipulation. template static auto mutable_key(slot_type* slot) - -> decltype(P::apply(ReturnKey(), element(slot))) { - return P::apply(ReturnKey(), element(slot)); + -> decltype(P::apply(ReturnKey(), hash_policy_traits::element(slot))) { + return P::apply(ReturnKey(), hash_policy_traits::element(slot)); } // Returns the "value" (as opposed to the "key") portion of the element. Used @@ -184,21 +148,6 @@ struct hash_policy_traits { static auto value(T* elem) -> decltype(P::value(elem)) { return P::value(elem); } - - private: - // Use auto -> decltype as an enabler. - template - static auto transfer_impl(Alloc* alloc, slot_type* new_slot, - slot_type* old_slot, int) - -> decltype((void)P::transfer(alloc, new_slot, old_slot)) { - P::transfer(alloc, new_slot, old_slot); - } - template - static void transfer_impl(Alloc* alloc, slot_type* new_slot, - slot_type* old_slot, char) { - construct(alloc, new_slot, std::move(element(old_slot))); - destroy(alloc, old_slot); - } }; } // namespace container_internal diff --git a/absl/container/internal/hash_policy_traits_test.cc b/absl/container/internal/hash_policy_traits_test.cc index 6ef8b9e0..82d7cc3a 100644 --- a/absl/container/internal/hash_policy_traits_test.cc +++ b/absl/container/internal/hash_policy_traits_test.cc @@ -38,81 +38,31 @@ struct PolicyWithoutOptionalOps { using key_type = Slot; using init_type = Slot; - static std::function construct; - static std::function destroy; - static std::function element; static int apply(int v) { return apply_impl(v); } static std::function apply_impl; static std::function value; }; -std::function PolicyWithoutOptionalOps::construct; -std::function PolicyWithoutOptionalOps::destroy; - -std::function PolicyWithoutOptionalOps::element; std::function PolicyWithoutOptionalOps::apply_impl; std::function PolicyWithoutOptionalOps::value; -struct PolicyWithOptionalOps : PolicyWithoutOptionalOps { - static std::function transfer; -}; - -std::function PolicyWithOptionalOps::transfer; - struct Test : ::testing::Test { Test() { - PolicyWithoutOptionalOps::construct = [&](void* a1, Slot* a2, Slot a3) { - construct.Call(a1, a2, std::move(a3)); - }; - PolicyWithoutOptionalOps::destroy = [&](void* a1, Slot* a2) { - destroy.Call(a1, a2); - }; - - PolicyWithoutOptionalOps::element = [&](Slot* a1) -> Slot& { - return element.Call(a1); - }; PolicyWithoutOptionalOps::apply_impl = [&](int a1) -> int { return apply.Call(a1); }; PolicyWithoutOptionalOps::value = [&](Slot* a1) -> Slot& { return value.Call(a1); }; - - PolicyWithOptionalOps::transfer = [&](void* a1, Slot* a2, Slot* a3) { - return transfer.Call(a1, a2, a3); - }; } std::allocator alloc; int a = 53; - - MockFunction construct; - MockFunction destroy; - - MockFunction element; MockFunction apply; MockFunction value; - - MockFunction transfer; }; -TEST_F(Test, construct) { - EXPECT_CALL(construct, Call(&alloc, &a, 53)); - hash_policy_traits::construct(&alloc, &a, 53); -} - -TEST_F(Test, destroy) { - EXPECT_CALL(destroy, Call(&alloc, &a)); - hash_policy_traits::destroy(&alloc, &a); -} - -TEST_F(Test, element) { - int b = 0; - EXPECT_CALL(element, Call(&a)).WillOnce(ReturnRef(b)); - EXPECT_EQ(&b, &hash_policy_traits::element(&a)); -} - TEST_F(Test, apply) { EXPECT_CALL(apply, Call(42)).WillOnce(Return(1337)); EXPECT_EQ(1337, (hash_policy_traits::apply(42))); @@ -124,20 +74,6 @@ TEST_F(Test, value) { EXPECT_EQ(&b, &hash_policy_traits::value(&a)); } -TEST_F(Test, without_transfer) { - int b = 42; - EXPECT_CALL(element, Call(&b)).WillOnce(::testing::ReturnRef(b)); - EXPECT_CALL(construct, Call(&alloc, &a, b)); - EXPECT_CALL(destroy, Call(&alloc, &b)); - hash_policy_traits::transfer(&alloc, &a, &b); -} - -TEST_F(Test, with_transfer) { - int b = 42; - EXPECT_CALL(transfer, Call(&alloc, &a, &b)); - hash_policy_traits::transfer(&alloc, &a, &b); -} - } // namespace } // namespace container_internal ABSL_NAMESPACE_END -- cgit v1.2.3