From c6b3f2cf583d8fec124f9d70a172b513363506fe Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 13 Aug 2020 14:05:00 -0700 Subject: [PATCH] Export of internal Abseil changes -- 0c8282d75798c77733eee6167870bcc6acc0bfc1 by Evan Brown : Provide mutable access to the key in node handles using std::launder when compiled with C++17 or later. Also, document why we can't provide mutable access to the key without C++17. Note: we use Policy::mutable_key() because btree already uses Policy::key() internally to get const key access, and we want to avoid calling std::launder unless we need mutable access to the key. PiperOrigin-RevId: 326519000 -- 8018d0c3044400f0a731b0d2d00b606742c98818 by Xiaoyi Zhang : Move `Status` internal symbols from the public header into an internal header file. PiperOrigin-RevId: 326471847 -- 87a7644864ba7c003b0611898aaba1b71c840376 by Abseil Team : Avoid a costly divide (the division accounts for 10% of the time spent in the function). When the division is signed, the compiler has to generate a div. When it is unsigned, it can generate a shift: https://godbolt.org/z/vGfTv4. As per the test above the div, we know that the value is unsigned. PiperOrigin-RevId: 326453275 GitOrigin-RevId: 0c8282d75798c77733eee6167870bcc6acc0bfc1 Change-Id: I0a953558358055ab3dc6a533d8930698509b1195 --- CMake/AbseilDll.cmake | 1 + absl/container/btree_map.h | 10 ++++ absl/container/btree_test.cc | 29 +++++++++++ absl/container/flat_hash_map.h | 5 ++ absl/container/flat_hash_map_test.cc | 15 ++++++ absl/container/internal/btree.h | 7 ++- absl/container/internal/common.h | 7 ++- absl/container/internal/container_memory.h | 15 ++++++ absl/container/internal/hash_policy_traits.h | 31 +++++++++--- absl/container/node_hash_map.h | 5 ++ absl/container/node_hash_map_test.cc | 15 ++++++ absl/status/BUILD.bazel | 1 + absl/status/CMakeLists.txt | 1 + absl/status/internal/status_internal.h | 51 ++++++++++++++++++++ absl/status/status.h | 22 +-------- 15 files changed, 184 insertions(+), 31 deletions(-) create mode 100644 absl/status/internal/status_internal.h diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index af7c004f..7958ace7 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -174,6 +174,7 @@ set(ABSL_INTERNAL_DLL_FILES "random/uniform_int_distribution.h" "random/uniform_real_distribution.h" "random/zipf_distribution.h" + "status/internal/status_internal.h" "status/status.h" "status/status.cc" "status/status_payload_printer.h" diff --git a/absl/container/btree_map.h b/absl/container/btree_map.h index bb450ead..448ac48a 100644 --- a/absl/container/btree_map.h +++ b/absl/container/btree_map.h @@ -325,6 +325,11 @@ class btree_map // does not contain an element with a matching key, this function returns an // empty node handle. // + // NOTE: when compiled in an earlier version of C++ than C++17, + // `node_type::key()` returns a const reference to the key instead of a + // mutable reference. We cannot safely return a mutable reference without + // std::launder (which is not available before C++17). + // // NOTE: In this context, `node_type` refers to the C++17 concept of a // move-only type that owns and provides access to the elements in associative // containers (https://en.cppreference.com/w/cpp/container/node_handle). @@ -652,6 +657,11 @@ class btree_multimap // does not contain an element with a matching key, this function returns an // empty node handle. // + // NOTE: when compiled in an earlier version of C++ than C++17, + // `node_type::key()` returns a const reference to the key instead of a + // mutable reference. We cannot safely return a mutable reference without + // std::launder (which is not available before C++17). + // // NOTE: In this context, `node_type` refers to the C++17 concept of a // move-only type that owns and provides access to the elements in associative // containers (https://en.cppreference.com/w/cpp/container/node_handle). diff --git a/absl/container/btree_test.cc b/absl/container/btree_test.cc index 0c5f9368..43704206 100644 --- a/absl/container/btree_test.cc +++ b/absl/container/btree_test.cc @@ -2551,6 +2551,35 @@ TEST(Btree, HeterogeneousInsertOrAssign) { } #endif +// This test requires std::launder for mutable key access in node handles. +#if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 +TEST(Btree, NodeHandleMutableKeyAccess) { + { + absl::btree_map map; + + map["key1"] = "mapped"; + + auto nh = map.extract(map.begin()); + nh.key().resize(3); + map.insert(std::move(nh)); + + EXPECT_THAT(map, ElementsAre(Pair("key", "mapped"))); + } + // Also for multimap. + { + absl::btree_multimap map; + + map.emplace("key1", "mapped"); + + auto nh = map.extract(map.begin()); + nh.key().resize(3); + map.insert(std::move(nh)); + + EXPECT_THAT(map, ElementsAre(Pair("key", "mapped"))); + } +} +#endif + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/flat_hash_map.h b/absl/container/flat_hash_map.h index fcb70d86..dbd37d3c 100644 --- a/absl/container/flat_hash_map.h +++ b/absl/container/flat_hash_map.h @@ -383,6 +383,11 @@ class flat_hash_map : public absl::container_internal::raw_hash_map< // key value and returns a node handle owning that extracted data. If the // `flat_hash_map` does not contain an element with a matching key, this // function returns an empty node handle. + // + // NOTE: when compiled in an earlier version of C++ than C++17, + // `node_type::key()` returns a const reference to the key instead of a + // mutable reference. We cannot safely return a mutable reference without + // std::launder (which is not available before C++17). using Base::extract; // flat_hash_map::merge() diff --git a/absl/container/flat_hash_map_test.cc b/absl/container/flat_hash_map_test.cc index 2823c32b..89ec60c9 100644 --- a/absl/container/flat_hash_map_test.cc +++ b/absl/container/flat_hash_map_test.cc @@ -267,6 +267,21 @@ TEST(FlatHashMap, EraseIf) { } } +// This test requires std::launder for mutable key access in node handles. +#if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 +TEST(FlatHashMap, NodeHandleMutableKeyAccess) { + flat_hash_map map; + + map["key1"] = "mapped"; + + auto nh = map.extract(map.begin()); + nh.key().resize(3); + map.insert(std::move(nh)); + + EXPECT_THAT(map, testing::ElementsAre(Pair("key", "mapped"))); +} +#endif + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/btree.h b/absl/container/internal/btree.h index 293e5771..4d405734 100644 --- a/absl/container/internal/btree.h +++ b/absl/container/internal/btree.h @@ -292,6 +292,11 @@ struct map_params : common_params decltype(slot_policy::mutable_key(s)) { + return slot_policy::mutable_key(s); + } static mapped_type &value(value_type *value) { return value->second; } }; @@ -1043,7 +1048,7 @@ class btree { #endif } - enum { + enum : uint32_t { kNodeValues = node_type::kNodeValues, kMinNodeValues = kNodeValues / 2, }; diff --git a/absl/container/internal/common.h b/absl/container/internal/common.h index 8990f294..030e9d4a 100644 --- a/absl/container/internal/common.h +++ b/absl/container/internal/common.h @@ -146,8 +146,11 @@ class node_handle decltype(PolicyTraits::key(std::declval())) { - return PolicyTraits::key(this->slot()); + // When C++17 is available, we can use std::launder to provide mutable + // access to the key. Otherwise, we provide const access. + auto key() const + -> decltype(PolicyTraits::mutable_key(std::declval())) { + return PolicyTraits::mutable_key(this->slot()); } mapped_type& mapped() const { diff --git a/absl/container/internal/container_memory.h b/absl/container/internal/container_memory.h index c7777a93..d02c576d 100644 --- a/absl/container/internal/container_memory.h +++ b/absl/container/internal/container_memory.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -352,6 +353,20 @@ struct map_slot_policy { return slot->value; } + // When C++17 is available, we can use std::launder to provide mutable + // access to the key for use in node handle. +#if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 + static K& mutable_key(slot_type* slot) { + // Still check for kMutableKeys so that we can avoid calling std::launder + // unless necessary because it can interfere with optimizations. + return kMutableKeys::value ? slot->key + : *std::launder(const_cast( + std::addressof(slot->value.first))); + } +#else // !(defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606) + static const K& mutable_key(slot_type* slot) { return key(slot); } +#endif + static const K& key(const slot_type* slot) { return kMutableKeys::value ? slot->key : slot->value.first; } diff --git a/absl/container/internal/hash_policy_traits.h b/absl/container/internal/hash_policy_traits.h index 3e1209c6..46c97b18 100644 --- a/absl/container/internal/hash_policy_traits.h +++ b/absl/container/internal/hash_policy_traits.h @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -29,15 +30,34 @@ namespace container_internal { // Defines how slots are initialized/destroyed/moved. template struct hash_policy_traits { + // The type of the keys stored in the hashtable. + using key_type = typename Policy::key_type; + private: struct ReturnKey { - // We return `Key` here. + // When C++17 is available, we can use std::launder to provide mutable + // access to the key for use in node handle. +#if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 + template ::value, int> = 0> + static key_type& Impl(Key&& k, int) { + return *std::launder( + const_cast(std::addressof(std::forward(k)))); + } +#endif + + template + static Key Impl(Key&& k, char) { + return std::forward(k); + } + // When Key=T&, we forward the lvalue reference. // When Key=T, we return by value to avoid a dangling reference. // eg, for string_hash_map. template - Key operator()(Key&& k, const Args&...) const { - return std::forward(k); + auto operator()(Key&& k, const Args&...) const + -> decltype(Impl(std::forward(k), 0)) { + return Impl(std::forward(k), 0); } }; @@ -52,9 +72,6 @@ struct hash_policy_traits { // The actual object stored in the hash table. using slot_type = typename Policy::slot_type; - // The type of the keys stored in the hashtable. - using key_type = typename Policy::key_type; - // The argument type for insertions into the hashtable. This is different // from value_type for increased performance. See initializer_list constructor // and insert() member functions for more details. @@ -156,7 +173,7 @@ struct hash_policy_traits { // Returns the "key" portion of the slot. // Used for node handle manipulation. template - static auto key(slot_type* slot) + static auto mutable_key(slot_type* slot) -> decltype(P::apply(ReturnKey(), element(slot))) { return P::apply(ReturnKey(), element(slot)); } diff --git a/absl/container/node_hash_map.h b/absl/container/node_hash_map.h index 174b971e..9852ff7f 100644 --- a/absl/container/node_hash_map.h +++ b/absl/container/node_hash_map.h @@ -374,6 +374,11 @@ class node_hash_map // key value and returns a node handle owning that extracted data. If the // `node_hash_map` does not contain an element with a matching key, this // function returns an empty node handle. + // + // NOTE: when compiled in an earlier version of C++ than C++17, + // `node_type::key()` returns a const reference to the key instead of a + // mutable reference. We cannot safely return a mutable reference without + // std::launder (which is not available before C++17). using Base::extract; // node_hash_map::merge() diff --git a/absl/container/node_hash_map_test.cc b/absl/container/node_hash_map_test.cc index 5d74b814..8f59a1e4 100644 --- a/absl/container/node_hash_map_test.cc +++ b/absl/container/node_hash_map_test.cc @@ -254,6 +254,21 @@ TEST(NodeHashMap, EraseIf) { } } +// This test requires std::launder for mutable key access in node handles. +#if defined(__cpp_lib_launder) && __cpp_lib_launder >= 201606 +TEST(NodeHashMap, NodeHandleMutableKeyAccess) { + node_hash_map map; + + map["key1"] = "mapped"; + + auto nh = map.extract(map.begin()); + nh.key().resize(3); + map.insert(std::move(nh)); + + EXPECT_THAT(map, testing::ElementsAre(Pair("key", "mapped"))); +} +#endif + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/status/BUILD.bazel b/absl/status/BUILD.bazel index d164252d..c4c5e497 100644 --- a/absl/status/BUILD.bazel +++ b/absl/status/BUILD.bazel @@ -31,6 +31,7 @@ licenses(["notice"]) # Apache 2.0 cc_library( name = "status", srcs = [ + "internal/status_internal.h", "status.cc", "status_payload_printer.cc", ], diff --git a/absl/status/CMakeLists.txt b/absl/status/CMakeLists.txt index c041d695..b48989d1 100644 --- a/absl/status/CMakeLists.txt +++ b/absl/status/CMakeLists.txt @@ -19,6 +19,7 @@ absl_cc_library( HDRS "status.h" SRCS + "internal/status_internal.h" "status.cc" "status_payload_printer.h" "status_payload_printer.cc" diff --git a/absl/status/internal/status_internal.h b/absl/status/internal/status_internal.h new file mode 100644 index 00000000..1f82b8e4 --- /dev/null +++ b/absl/status/internal/status_internal.h @@ -0,0 +1,51 @@ +// Copyright 2019 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_STATUS_INTERNAL_STATUS_INTERNAL_H_ +#define ABSL_STATUS_INTERNAL_STATUS_INTERNAL_H_ + +#include + +#include "absl/container/inlined_vector.h" +#include "absl/strings/cord.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN + +enum class StatusCode : int; + +namespace status_internal { + +// Container for status payloads. +struct Payload { + std::string type_url; + absl::Cord payload; +}; + +using Payloads = absl::InlinedVector; + +// Reference-counted representation of Status data. +struct StatusRep { + std::atomic ref; + absl::StatusCode code; + std::string message; + std::unique_ptr payloads; +}; + +absl::StatusCode MapToLocalCode(int value); +} // namespace status_internal + +ABSL_NAMESPACE_END +} // namespace absl + +#endif // ABSL_STATUS_INTERNAL_STATUS_INTERNAL_H_ diff --git a/absl/status/status.h b/absl/status/status.h index 42530621..8097b05a 100644 --- a/absl/status/status.h +++ b/absl/status/status.h @@ -18,6 +18,7 @@ #include #include "absl/container/inlined_vector.h" +#include "absl/status/internal/status_internal.h" #include "absl/strings/cord.h" #include "absl/types/optional.h" @@ -164,27 +165,6 @@ std::string StatusCodeToString(StatusCode code); // Streams StatusCodeToString(code) to `os`. std::ostream& operator<<(std::ostream& os, StatusCode code); -namespace status_internal { - -// Container for status payloads. -struct Payload { - std::string type_url; - absl::Cord payload; -}; - -using Payloads = absl::InlinedVector; - -// Reference-counted representation of Status data. -struct StatusRep { - std::atomic ref; - absl::StatusCode code; - std::string message; - std::unique_ptr payloads; -}; - -absl::StatusCode MapToLocalCode(int value); -} // namespace status_internal - class ABSL_MUST_USE_RESULT Status final { public: // Creates an OK status with no message or payload.