From fe16a5e72d69d8319e53fb8b9c2e9d2b4e43a207 Mon Sep 17 00:00:00 2001 From: Andy Getzendanner Date: Wed, 17 Jan 2024 13:11:49 -0800 Subject: Add protected copy ctor+assign to absl::LogSink, and clarify thread-safety requirements to apply to the interface methods. PiperOrigin-RevId: 599266310 Change-Id: I3b5a91eb17b4b09feba5e048892875f3a747afb1 --- absl/log/log_sink.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/absl/log/log_sink.h b/absl/log/log_sink.h index 9bfa6f86..2910070d 100644 --- a/absl/log/log_sink.h +++ b/absl/log/log_sink.h @@ -32,15 +32,16 @@ ABSL_NAMESPACE_BEGIN // `absl::LogSink` is an interface which can be extended to intercept and // process particular messages (with `LOG.ToSinkOnly()` or // `LOG.ToSinkAlso()`) or all messages (if registered with -// `absl::AddLogSink`). Implementations must be thread-safe, and should take -// care not to take any locks that might be held by the `LOG` caller. +// `absl::AddLogSink`). Implementations must not take any locks that might be +// held by the `LOG` caller. class LogSink { public: virtual ~LogSink() = default; // LogSink::Send() // - // `Send` is called synchronously during the log statement. + // `Send` is called synchronously during the log statement. `Send` must be + // thread-safe. // // It is safe to use `LOG` within an implementation of `Send`. `ToSinkOnly` // and `ToSinkAlso` are safe in general but can be used to create an infinite @@ -50,9 +51,15 @@ class LogSink { // LogSink::Flush() // // Sinks that buffer messages should override this method to flush the buffer - // and return. + // and return. `Flush` must be thread-safe. virtual void Flush() {} + protected: + LogSink() = default; + // Implementations may be copyable and/or movable. + LogSink(const LogSink&) = default; + LogSink& operator=(const LogSink&) = default; + private: // https://lld.llvm.org/missingkeyfunction.html#missing-key-function virtual void KeyFunction() const final; // NOLINT(readability/inheritance) -- cgit v1.2.3 From 49ff696cda14c4825a99ced4ff6b6bbe20dd38ce Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 18 Jan 2024 09:10:55 -0800 Subject: Migrate empty CrcCordState to absl::NoDestructor. Note that this only changes how we allocate the empty state, and reference countings of `empty` stay the same. PiperOrigin-RevId: 599526339 Change-Id: I2c6aaf875c144c947e17fe8f69692b1195b55dd7 --- absl/crc/BUILD.bazel | 2 +- absl/crc/CMakeLists.txt | 1 + absl/crc/internal/crc_cord_state.cc | 7 ++++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/absl/crc/BUILD.bazel b/absl/crc/BUILD.bazel index f44c3f6b..d923aec4 100644 --- a/absl/crc/BUILD.bazel +++ b/absl/crc/BUILD.bazel @@ -182,8 +182,8 @@ cc_library( deps = [ ":crc32c", "//absl/base:config", + "//absl/base:no_destructor", "//absl/numeric:bits", - "//absl/strings", ], ) diff --git a/absl/crc/CMakeLists.txt b/absl/crc/CMakeLists.txt index ec7b4512..d52a1bc4 100644 --- a/absl/crc/CMakeLists.txt +++ b/absl/crc/CMakeLists.txt @@ -159,6 +159,7 @@ absl_cc_library( absl::crc32c absl::config absl::strings + absl::no_destructor ) absl_cc_test( diff --git a/absl/crc/internal/crc_cord_state.cc b/absl/crc/internal/crc_cord_state.cc index 28d04dc4..303a5559 100644 --- a/absl/crc/internal/crc_cord_state.cc +++ b/absl/crc/internal/crc_cord_state.cc @@ -17,6 +17,7 @@ #include #include "absl/base/config.h" +#include "absl/base/no_destructor.h" #include "absl/numeric/bits.h" namespace absl { @@ -24,14 +25,14 @@ ABSL_NAMESPACE_BEGIN namespace crc_internal { CrcCordState::RefcountedRep* CrcCordState::RefSharedEmptyRep() { - static CrcCordState::RefcountedRep* empty = new CrcCordState::RefcountedRep; + static absl::NoDestructor empty; assert(empty->count.load(std::memory_order_relaxed) >= 1); assert(empty->rep.removed_prefix.length == 0); assert(empty->rep.prefix_crc.empty()); - Ref(empty); - return empty; + Ref(empty.get()); + return empty.get(); } CrcCordState::CrcCordState() : refcounted_rep_(new RefcountedRep) {} -- cgit v1.2.3 From b03cda5ec9b1f5aa3e2d0e5db4e436a11ed193bc Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 18 Jan 2024 09:55:51 -0800 Subject: Added benchmarks for smaller size copy constructors. PiperOrigin-RevId: 599538858 Change-Id: I9e92f4c9cfef1bfe6f8f925efe0ede3f309b6bf4 --- absl/container/internal/raw_hash_set_benchmark.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/absl/container/internal/raw_hash_set_benchmark.cc b/absl/container/internal/raw_hash_set_benchmark.cc index 88b07373..05a06427 100644 --- a/absl/container/internal/raw_hash_set_benchmark.cc +++ b/absl/container/internal/raw_hash_set_benchmark.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -294,7 +295,7 @@ void BM_CopyCtorSparseInt(benchmark::State& state) { benchmark::DoNotOptimize(t2); } } -BENCHMARK(BM_CopyCtorSparseInt)->Range(128, 4096); +BENCHMARK(BM_CopyCtorSparseInt)->Range(1, 4096); void BM_CopyCtorInt(benchmark::State& state) { std::random_device rd; @@ -312,7 +313,7 @@ void BM_CopyCtorInt(benchmark::State& state) { benchmark::DoNotOptimize(t2); } } -BENCHMARK(BM_CopyCtorInt)->Range(128, 4096); +BENCHMARK(BM_CopyCtorInt)->Range(0, 4096); void BM_CopyCtorString(benchmark::State& state) { std::random_device rd; @@ -330,7 +331,7 @@ void BM_CopyCtorString(benchmark::State& state) { benchmark::DoNotOptimize(t2); } } -BENCHMARK(BM_CopyCtorString)->Range(128, 4096); +BENCHMARK(BM_CopyCtorString)->Range(0, 4096); void BM_CopyAssign(benchmark::State& state) { std::random_device rd; -- cgit v1.2.3 From 04d8afe7a3b36b57f6497c60dc6184e9dfeb85c1 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 19 Jan 2024 07:53:03 -0800 Subject: Remove code pieces for no longer supported MSVC versions. The current support policy is `_MSC_VER >= 1920`. PiperOrigin-RevId: 599833619 Change-Id: I9cf7393a5b659d1680765e37e0328539ccb870fa --- absl/numeric/int128_test.cc | 6 ------ absl/strings/string_view.h | 2 +- absl/strings/string_view_test.cc | 4 ---- absl/synchronization/lifetime_test.cc | 5 ++--- absl/types/optional_test.cc | 27 ++------------------------- 5 files changed, 5 insertions(+), 39 deletions(-) diff --git a/absl/numeric/int128_test.cc b/absl/numeric/int128_test.cc index 01e3eb5c..005ff9a4 100644 --- a/absl/numeric/int128_test.cc +++ b/absl/numeric/int128_test.cc @@ -26,12 +26,6 @@ #include "absl/hash/hash_testing.h" #include "absl/meta/type_traits.h" -#if defined(_MSC_VER) && _MSC_VER == 1900 -// Disable "unary minus operator applied to unsigned type" warnings in Microsoft -// Visual C++ 14 (2015). -#pragma warning(disable:4146) -#endif - #define MAKE_INT128(HI, LO) absl::MakeInt128(static_cast(HI), LO) namespace { diff --git a/absl/strings/string_view.h b/absl/strings/string_view.h index 04ca0a38..b393c6fc 100644 --- a/absl/strings/string_view.h +++ b/absl/strings/string_view.h @@ -670,7 +670,7 @@ class string_view { } static constexpr size_type StrlenInternal(absl::Nonnull str) { -#if defined(_MSC_VER) && _MSC_VER >= 1910 && !defined(__clang__) +#if defined(_MSC_VER) && !defined(__clang__) // MSVC 2017+ can evaluate this at compile-time. const char* begin = str; while (*str != '\0') ++str; diff --git a/absl/strings/string_view_test.cc b/absl/strings/string_view_test.cc index 5b1eb01a..251f1842 100644 --- a/absl/strings/string_view_test.cc +++ b/absl/strings/string_view_test.cc @@ -1051,9 +1051,6 @@ TEST(StringViewTest, ConstexprNullSafeStringView) { EXPECT_EQ(0u, s.size()); EXPECT_EQ(absl::string_view(), s); } -#if !defined(_MSC_VER) || _MSC_VER >= 1910 - // MSVC 2017+ is required for good constexpr string_view support. - // See the implementation of `absl::string_view::StrlenInternal()`. { static constexpr char kHi[] = "hi"; absl::string_view s = absl::NullSafeStringView(kHi); @@ -1066,7 +1063,6 @@ TEST(StringViewTest, ConstexprNullSafeStringView) { EXPECT_EQ(s.size(), 5u); EXPECT_EQ("hello", s); } -#endif } TEST(StringViewTest, ConstexprCompiles) { diff --git a/absl/synchronization/lifetime_test.cc b/absl/synchronization/lifetime_test.cc index d5ce35a1..4c4cff64 100644 --- a/absl/synchronization/lifetime_test.cc +++ b/absl/synchronization/lifetime_test.cc @@ -123,10 +123,9 @@ class OnDestruction { }; // These tests require that the compiler correctly supports C++11 constant -// initialization... but MSVC has a known regression since v19.10 till v19.25: +// initialization... but MSVC has a known regression (since v19.10) till v19.25: // https://developercommunity.visualstudio.com/content/problem/336946/class-with-constexpr-constructor-not-using-static.html -#if defined(__clang__) || \ - !(defined(_MSC_VER) && _MSC_VER > 1900 && _MSC_VER < 1925) +#if defined(__clang__) || !(defined(_MSC_VER) && _MSC_VER < 1925) // kConstInit // Test early usage. (Declaration comes first; definitions must appear after // the test runner.) diff --git a/absl/types/optional_test.cc b/absl/types/optional_test.cc index 5da297b0..a4daa799 100644 --- a/absl/types/optional_test.cc +++ b/absl/types/optional_test.cc @@ -994,25 +994,6 @@ TEST(optionalTest, PointerStuff) { #endif #endif -// MSVC has a bug with "cv-qualifiers in class construction", fixed in 2017. See -// https://docs.microsoft.com/en-us/cpp/cpp-conformance-improvements-2017#bug-fixes -// The compiler some incorrectly ignores the cv-qualifier when generating a -// class object via a constructor call. For example: -// -// class optional { -// constexpr T&& value() &&; -// constexpr const T&& value() const &&; -// } -// -// using COI = const absl::optional; -// static_assert(2 == COI(2).value(), ""); // const && -// -// This should invoke the "const &&" overload but since it ignores the const -// qualifier it finds the "&&" overload the best candidate. -#if defined(_MSC_VER) && _MSC_VER < 1910 -#define ABSL_SKIP_OVERLOAD_TEST_DUE_TO_MSVC_BUG -#endif - TEST(optionalTest, Value) { using O = absl::optional; using CO = const absl::optional; @@ -1032,8 +1013,7 @@ TEST(optionalTest, Value) { EXPECT_EQ("c&", TypeQuals(clvalue.value())); EXPECT_EQ("c&", TypeQuals(lvalue_c.value())); EXPECT_EQ("&&", TypeQuals(O(absl::in_place, "xvalue").value())); -#if !defined(ABSL_SKIP_OVERLOAD_TEST_DUE_TO_MSVC_BUG) && \ - !defined(ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG) +#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("c&&", TypeQuals(CO(absl::in_place, "cxvalue").value())); #endif EXPECT_EQ("c&&", TypeQuals(OC(absl::in_place, "xvalue_c").value())); @@ -1083,8 +1063,7 @@ TEST(optionalTest, DerefOperator) { EXPECT_EQ("&", TypeQuals(*lvalue)); EXPECT_EQ("c&", TypeQuals(*clvalue)); EXPECT_EQ("&&", TypeQuals(*O(absl::in_place, "xvalue"))); -#if !defined(ABSL_SKIP_OVERLOAD_TEST_DUE_TO_MSVC_BUG) && \ - !defined(ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG) +#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("c&&", TypeQuals(*CO(absl::in_place, "cxvalue"))); #endif EXPECT_EQ("c&&", TypeQuals(*OC(absl::in_place, "xvalue_c"))); @@ -1117,11 +1096,9 @@ TEST(optionalTest, ValueOr) { constexpr absl::optional copt_empty, copt_set = {1.2}; static_assert(42.0 == copt_empty.value_or(42), ""); static_assert(1.2 == copt_set.value_or(42), ""); -#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_MSVC_BUG using COD = const absl::optional; static_assert(42.0 == COD().value_or(42), ""); static_assert(1.2 == COD(1.2).value_or(42), ""); -#endif } // make_optional cannot be constexpr until C++17 -- cgit v1.2.3 From 2be67701e7a33b45d322064349827e1155953338 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 19 Jan 2024 10:29:06 -0800 Subject: Prevent brace initialization of AlphaNum This was not intended to be supported, and it has resulted in calls as `absl::StrCat({...})`, which are not supported and only work coincidentally for the first 4 arguments due to `absl::StrCat` having overloads that take `absl::AlphaNum` directly for those. The existing situation prevents modifying the implementations of such functions to alternatives that do not have such overloads for those arguments. PiperOrigin-RevId: 599872755 Change-Id: I02c90119b2b96a922cf7e3b5d5f02affe24a272d --- absl/strings/str_cat.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/absl/strings/str_cat.h b/absl/strings/str_cat.h index ea2c4dca..bc8ea7d4 100644 --- a/absl/strings/str_cat.h +++ b/absl/strings/str_cat.h @@ -93,6 +93,7 @@ #include #include #include +#include #include #include #include @@ -312,6 +313,10 @@ class AlphaNum { // No bool ctor -- bools convert to an integral type. // A bool ctor would also convert incoming pointers (bletch). + // Prevent brace initialization + template + AlphaNum(std::initializer_list) = delete; // NOLINT(runtime/explicit) + AlphaNum(int x) // NOLINT(runtime/explicit) : piece_(digits_, static_cast( numbers_internal::FastIntToBuffer(x, digits_) - -- cgit v1.2.3 From b21b4898f9f4b967d5faa27f3e090535c4de1422 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Mon, 22 Jan 2024 08:38:07 -0800 Subject: Disable ABSL_ATTRIBUTE_TRIVIAL_ABI in open-source builds Since compiler support for this attribute differs, if for example system libraries compiled with GCC are mixed with libraries compiled with Clang, types will have different ideas about their ABI. PiperOrigin-RevId: 600467146 Change-Id: I5729e54d34176d019a2dee9afc36ed9da1da10fa --- absl/base/attributes.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/absl/base/attributes.h b/absl/base/attributes.h index f21dcb3c..d4f67a12 100644 --- a/absl/base/attributes.h +++ b/absl/base/attributes.h @@ -843,15 +843,11 @@ // See also the upstream documentation: // https://clang.llvm.org/docs/AttributeReference.html#trivial-abi // -#if ABSL_HAVE_CPP_ATTRIBUTE(clang::trivial_abi) -#define ABSL_ATTRIBUTE_TRIVIAL_ABI [[clang::trivial_abi]] -#define ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI 1 -#elif ABSL_HAVE_ATTRIBUTE(trivial_abi) -#define ABSL_ATTRIBUTE_TRIVIAL_ABI __attribute__((trivial_abi)) -#define ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI 1 -#else +// b/321691395 - This is currently disabled in open-source builds since +// compiler support differs. If system libraries compiled with GCC are mixed +// with libraries compiled with Clang, types will have different ideas about +// their ABI, leading to hard to debug crashes. #define ABSL_ATTRIBUTE_TRIVIAL_ABI -#endif // ABSL_ATTRIBUTE_NO_UNIQUE_ADDRESS // -- cgit v1.2.3 From f7d2b13ef2466a5a3b7bcc535c49d3962a5c89f9 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 22 Jan 2024 09:09:23 -0800 Subject: Remove code pieces for no longer supported GCC versions. The minimum supported version today is GCC 7 (`__GNUC__ >= 7`). PiperOrigin-RevId: 600475215 Change-Id: I1aa46384f1e75f268649a48dbe2b42f3475bb07f --- absl/base/config.h | 8 ++------ absl/container/internal/compressed_tuple_test.cc | 2 -- absl/container/internal/hash_policy_testing.h | 3 +-- absl/flags/flag_test.cc | 8 -------- absl/types/optional_test.cc | 22 +--------------------- 5 files changed, 4 insertions(+), 39 deletions(-) diff --git a/absl/base/config.h b/absl/base/config.h index 0fb66927..5fa9f0ef 100644 --- a/absl/base/config.h +++ b/absl/base/config.h @@ -379,9 +379,7 @@ static_assert(ABSL_INTERNAL_INLINE_NAMESPACE_STR[0] != 'h' || #define ABSL_HAVE_EXCEPTIONS 1 #endif // defined(__EXCEPTIONS) && ABSL_HAVE_FEATURE(cxx_exceptions) // Handle remaining special cases and default to exceptions being supported. -#elif !(defined(__GNUC__) && (__GNUC__ < 5) && !defined(__EXCEPTIONS)) && \ - !(ABSL_INTERNAL_HAVE_MIN_GNUC_VERSION(5, 0) && \ - !defined(__cpp_exceptions)) && \ +#elif !(defined(__GNUC__) && !defined(__cpp_exceptions)) && \ !(defined(_MSC_VER) && !defined(_CPPUNWIND)) #define ABSL_HAVE_EXCEPTIONS 1 #endif @@ -902,9 +900,7 @@ static_assert(ABSL_INTERNAL_INLINE_NAMESPACE_STR[0] != 'h' || #error ABSL_INTERNAL_HAS_CXA_DEMANGLE cannot be directly set #elif defined(OS_ANDROID) && (defined(__i386__) || defined(__x86_64__)) #define ABSL_INTERNAL_HAS_CXA_DEMANGLE 0 -#elif defined(__GNUC__) && defined(__GNUC_MINOR__) && \ - (__GNUC__ >= 4 || (__GNUC__ >= 3 && __GNUC_MINOR__ >= 4)) && \ - !defined(__mips__) +#elif defined(__GNUC__) && !defined(__mips__) #define ABSL_INTERNAL_HAS_CXA_DEMANGLE 1 #elif defined(__clang__) && !defined(_MSC_VER) #define ABSL_INTERNAL_HAS_CXA_DEMANGLE 1 diff --git a/absl/container/internal/compressed_tuple_test.cc b/absl/container/internal/compressed_tuple_test.cc index 74111f97..da07baab 100644 --- a/absl/container/internal/compressed_tuple_test.cc +++ b/absl/container/internal/compressed_tuple_test.cc @@ -358,7 +358,6 @@ TEST(CompressedTupleTest, Constexpr) { EXPECT_EQ(x2, 5); EXPECT_EQ(x3, CallType::kConstRef); -#if !defined(__GNUC__) || defined(__clang__) || __GNUC__ > 4 constexpr CompressedTuple, TrivialStruct, int> trivial = {}; constexpr CallType trivial0 = trivial.get<0>().value(); constexpr int trivial1 = trivial.get<1>().value(); @@ -367,7 +366,6 @@ TEST(CompressedTupleTest, Constexpr) { EXPECT_EQ(trivial0, CallType::kConstRef); EXPECT_EQ(trivial1, 0); EXPECT_EQ(trivial2, 0); -#endif constexpr CompressedTuple, NonTrivialStruct, absl::optional> non_trivial = {}; diff --git a/absl/container/internal/hash_policy_testing.h b/absl/container/internal/hash_policy_testing.h index 01c40d2e..66bb12ec 100644 --- a/absl/container/internal/hash_policy_testing.h +++ b/absl/container/internal/hash_policy_testing.h @@ -174,8 +174,7 @@ ABSL_NAMESPACE_END // From GCC-4.9 Changelog: (src: https://gcc.gnu.org/gcc-4.9/changes.html) // "the unordered associative containers in and // meet the allocator-aware container requirements;" -#if (defined(__GLIBCXX__) && __GLIBCXX__ <= 20140425 ) || \ -( __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 9 )) +#if defined(__GLIBCXX__) && __GLIBCXX__ <= 20140425 #define ABSL_UNORDERED_SUPPORTS_ALLOC_CTORS 0 #else #define ABSL_UNORDERED_SUPPORTS_ALLOC_CTORS 1 diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 8d14ba8d..53ad4635 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc @@ -1044,13 +1044,7 @@ TEST_F(FlagTest, MacroWithinAbslFlag) { // -------------------------------------------------------------------- -#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ <= 5 -#define ABSL_SKIP_OPTIONAL_BOOL_TEST_DUE_TO_GCC_BUG -#endif - -#ifndef ABSL_SKIP_OPTIONAL_BOOL_TEST_DUE_TO_GCC_BUG ABSL_FLAG(absl::optional, optional_bool, absl::nullopt, "help"); -#endif ABSL_FLAG(absl::optional, optional_int, {}, "help"); ABSL_FLAG(absl::optional, optional_double, 9.3, "help"); ABSL_FLAG(absl::optional, optional_string, absl::nullopt, "help"); @@ -1064,7 +1058,6 @@ ABSL_FLAG(std::optional, std_optional_int64, std::nullopt, "help"); namespace { -#ifndef ABSL_SKIP_OPTIONAL_BOOL_TEST_DUE_TO_GCC_BUG TEST_F(FlagTest, TestOptionalBool) { EXPECT_FALSE(absl::GetFlag(FLAGS_optional_bool).has_value()); EXPECT_EQ(absl::GetFlag(FLAGS_optional_bool), absl::nullopt); @@ -1083,7 +1076,6 @@ TEST_F(FlagTest, TestOptionalBool) { } // -------------------------------------------------------------------- -#endif TEST_F(FlagTest, TestOptionalInt) { EXPECT_FALSE(absl::GetFlag(FLAGS_optional_int).has_value()); diff --git a/absl/types/optional_test.cc b/absl/types/optional_test.cc index a4daa799..115e20ce 100644 --- a/absl/types/optional_test.cc +++ b/absl/types/optional_test.cc @@ -982,18 +982,6 @@ TEST(optionalTest, PointerStuff) { static_assert((*opt1).x == ConstexprType::kCtorInt, ""); } -// gcc has a bug pre 4.9.1 where it doesn't do correct overload resolution -// when overloads are const-qualified and *this is an raluve. -// Skip that test to make the build green again when using the old compiler. -// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59296 is fixed in 4.9.1. -#if defined(__GNUC__) && !defined(__clang__) -#define GCC_VERSION \ - (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) -#if GCC_VERSION < 40901 -#define ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG -#endif -#endif - TEST(optionalTest, Value) { using O = absl::optional; using CO = const absl::optional; @@ -1006,16 +994,12 @@ TEST(optionalTest, Value) { EXPECT_EQ("lvalue_c", lvalue_c.value()); EXPECT_EQ("xvalue", O(absl::in_place, "xvalue").value()); EXPECT_EQ("xvalue_c", OC(absl::in_place, "xvalue_c").value()); -#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("cxvalue", CO(absl::in_place, "cxvalue").value()); -#endif EXPECT_EQ("&", TypeQuals(lvalue.value())); EXPECT_EQ("c&", TypeQuals(clvalue.value())); EXPECT_EQ("c&", TypeQuals(lvalue_c.value())); EXPECT_EQ("&&", TypeQuals(O(absl::in_place, "xvalue").value())); -#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("c&&", TypeQuals(CO(absl::in_place, "cxvalue").value())); -#endif EXPECT_EQ("c&&", TypeQuals(OC(absl::in_place, "xvalue_c").value())); #if !defined(ABSL_VOLATILE_RETURN_TYPES_DEPRECATED) @@ -1039,7 +1023,7 @@ TEST(optionalTest, Value) { // test constexpr value() constexpr absl::optional o1(1); static_assert(1 == o1.value(), ""); // const & -#if !defined(_MSC_VER) && !defined(ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG) +#ifndef _MSC_VER using COI = const absl::optional; static_assert(2 == COI(2).value(), ""); // const && #endif @@ -1057,15 +1041,11 @@ TEST(optionalTest, DerefOperator) { EXPECT_EQ("lvalue_c", *lvalue_c); EXPECT_EQ("xvalue", *O(absl::in_place, "xvalue")); EXPECT_EQ("xvalue_c", *OC(absl::in_place, "xvalue_c")); -#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("cxvalue", *CO(absl::in_place, "cxvalue")); -#endif EXPECT_EQ("&", TypeQuals(*lvalue)); EXPECT_EQ("c&", TypeQuals(*clvalue)); EXPECT_EQ("&&", TypeQuals(*O(absl::in_place, "xvalue"))); -#ifndef ABSL_SKIP_OVERLOAD_TEST_DUE_TO_GCC_BUG EXPECT_EQ("c&&", TypeQuals(*CO(absl::in_place, "cxvalue"))); -#endif EXPECT_EQ("c&&", TypeQuals(*OC(absl::in_place, "xvalue_c"))); #if !defined(ABSL_VOLATILE_RETURN_TYPES_DEPRECATED) -- cgit v1.2.3 From 4676ffa9811c10d11fc2c782d9399a3f41d739b8 Mon Sep 17 00:00:00 2001 From: Hannah Shi Date: Mon, 22 Jan 2024 11:55:11 -0800 Subject: PR #1604: Add privacy manifest Imported from GitHub PR https://github.com/abseil/abseil-cpp/pull/1604 Looks like abseil is not using any of the APIs listed in https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api?language=objc, so adding an empty manifest file. fixes #1602 Merge 79da4bd7b54949d5e01c04623842ea1d0d4fbe59 into f7d2b13ef2466a5a3b7bcc535c49d3962a5c89f9 Merging this change closes #1604 COPYBARA_INTEGRATE_REVIEW=https://github.com/abseil/abseil-cpp/pull/1604 from HannahShiSFB:privacy-manifests 79da4bd7b54949d5e01c04623842ea1d0d4fbe59 PiperOrigin-RevId: 600525731 Change-Id: I3fbc6dcc6e47032665a9fa72c000ae245a25dd52 --- PrivacyInfo.xcprivacy | 14 ++++++++++++++ absl/abseil.podspec.gen.py | 3 +++ 2 files changed, 17 insertions(+) create mode 100644 PrivacyInfo.xcprivacy diff --git a/PrivacyInfo.xcprivacy b/PrivacyInfo.xcprivacy new file mode 100644 index 00000000..3ff4a9d9 --- /dev/null +++ b/PrivacyInfo.xcprivacy @@ -0,0 +1,14 @@ + + + + + NSPrivacyTracking + + NSPrivacyCollectedDataTypes + + NSPrivacyTrackingDomains + + NSPrivacyAccessedAPITypes + + + diff --git a/absl/abseil.podspec.gen.py b/absl/abseil.podspec.gen.py index 63752980..c83edbfe 100755 --- a/absl/abseil.podspec.gen.py +++ b/absl/abseil.podspec.gen.py @@ -30,6 +30,9 @@ Pod::Spec.new do |s| :git => 'https://github.com/abseil/abseil-cpp.git', :tag => '${tag}', } + s.resource_bundles = { + s.module_name => 'PrivacyInfo.xcprivacy', + } s.module_name = 'absl' s.header_mappings_dir = 'absl' s.header_dir = 'absl' -- cgit v1.2.3 From 6dda8e527f19a6508ab855641914945a20d6b6df Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 23 Jan 2024 10:46:19 -0800 Subject: Always check if the new frame pointer is readable. Terminate the stack trace if it isn't. PiperOrigin-RevId: 600839499 Change-Id: I5692fa6cb52c4c8061b4ac14d8fba70f7fbabc52 --- absl/debugging/internal/stacktrace_aarch64-inl.inc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/absl/debugging/internal/stacktrace_aarch64-inl.inc b/absl/debugging/internal/stacktrace_aarch64-inl.inc index 1caf7bbe..1e07e042 100644 --- a/absl/debugging/internal/stacktrace_aarch64-inl.inc +++ b/absl/debugging/internal/stacktrace_aarch64-inl.inc @@ -122,13 +122,6 @@ static void **NextStackFrame(void **old_frame_pointer, const void *uc, if (pre_signal_frame_pointer >= old_frame_pointer) { new_frame_pointer = pre_signal_frame_pointer; } - // Check that alleged frame pointer is actually readable. This is to - // prevent "double fault" in case we hit the first fault due to e.g. - // stack corruption. - if (!absl::debugging_internal::AddressIsReadable( - new_frame_pointer)) - return nullptr; - } } #endif @@ -136,6 +129,14 @@ static void **NextStackFrame(void **old_frame_pointer, const void *uc, if ((reinterpret_cast(new_frame_pointer) & 7) != 0) return nullptr; + // Check that alleged frame pointer is actually readable. This is to + // prevent "double fault" in case we hit the first fault due to e.g. + // stack corruption. + if (!absl::debugging_internal::AddressIsReadable( + new_frame_pointer)) + return nullptr; + } + // Only check the size if both frames are in the same stack. if (InsideSignalStack(new_frame_pointer, stack_info) == InsideSignalStack(old_frame_pointer, stack_info)) { -- cgit v1.2.3 From 27f15a052ba54c984faa1eb6044de59b237e6f37 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 24 Jan 2024 10:08:22 -0800 Subject: Use absl::NoDestructor for global HashtablezSampler. PiperOrigin-RevId: 601156448 Change-Id: Id6d19bda7eb3acee11cfab3a95e611996d6ef4cc --- absl/container/BUILD.bazel | 1 + absl/container/CMakeLists.txt | 1 + absl/container/internal/hashtablez_sampler.cc | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 0ba2fa76..fda866a5 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -563,6 +563,7 @@ cc_library( "//absl/base", "//absl/base:config", "//absl/base:core_headers", + "//absl/base:no_destructor", "//absl/base:raw_logging_internal", "//absl/debugging:stacktrace", "//absl/memory", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 128cc0e9..449a2cad 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -602,6 +602,7 @@ absl_cc_library( absl::base absl::config absl::exponential_biased + absl::no_destructor absl::raw_logging_internal absl::sample_recorder absl::synchronization diff --git a/absl/container/internal/hashtablez_sampler.cc b/absl/container/internal/hashtablez_sampler.cc index 79a0973a..b21beef0 100644 --- a/absl/container/internal/hashtablez_sampler.cc +++ b/absl/container/internal/hashtablez_sampler.cc @@ -24,6 +24,7 @@ #include "absl/base/attributes.h" #include "absl/base/config.h" #include "absl/base/internal/raw_logging.h" +#include "absl/base/no_destructor.h" #include "absl/debugging/stacktrace.h" #include "absl/memory/memory.h" #include "absl/profiling/internal/exponential_biased.h" @@ -64,7 +65,7 @@ ABSL_PER_THREAD_TLS_KEYWORD SamplingState global_next_sample = {0, 0}; #endif // defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) HashtablezSampler& GlobalHashtablezSampler() { - static auto* sampler = new HashtablezSampler(); + static absl::NoDestructor sampler; return *sampler; } -- cgit v1.2.3 From 42624b3d9d56ae6fd5c48b28715044667942b9d8 Mon Sep 17 00:00:00 2001 From: Dmitri Gribenko Date: Thu, 25 Jan 2024 09:49:41 -0800 Subject: Add nullability annotations PiperOrigin-RevId: 601477629 Change-Id: I3b683d94192b04f233ce5ec636586ccb90d24602 --- absl/strings/escaping.cc | 46 +++++++++++++++++++++++++++++----------------- absl/strings/escaping.h | 17 +++++++++++------ 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/absl/strings/escaping.cc b/absl/strings/escaping.cc index 1c0eac42..5de12afa 100644 --- a/absl/strings/escaping.cc +++ b/absl/strings/escaping.cc @@ -25,6 +25,7 @@ #include "absl/base/config.h" #include "absl/base/internal/raw_logging.h" #include "absl/base/internal/unaligned_access.h" +#include "absl/base/nullability.h" #include "absl/strings/ascii.h" #include "absl/strings/charset.h" #include "absl/strings/internal/escaping.h" @@ -54,7 +55,8 @@ inline unsigned int hex_digit_to_int(char c) { return x & 0xf; } -inline bool IsSurrogate(char32_t c, absl::string_view src, std::string* error) { +inline bool IsSurrogate(char32_t c, absl::string_view src, + absl::Nullable error) { if (c >= 0xD800 && c <= 0xDFFF) { if (error) { *error = absl::StrCat("invalid surrogate character (0xD800-DFFF): \\", @@ -83,7 +85,9 @@ inline bool IsSurrogate(char32_t c, absl::string_view src, std::string* error) { // UnescapeCEscapeSequences(). // ---------------------------------------------------------------------- bool CUnescapeInternal(absl::string_view source, bool leave_nulls_escaped, - char* dest, ptrdiff_t* dest_len, std::string* error) { + absl::Nonnull dest, + absl::Nonnull dest_len, + absl::Nullable error) { char* d = dest; const char* p = source.data(); const char* end = p + source.size(); @@ -290,7 +294,8 @@ bool CUnescapeInternal(absl::string_view source, bool leave_nulls_escaped, // may be the same. // ---------------------------------------------------------------------- bool CUnescapeInternal(absl::string_view source, bool leave_nulls_escaped, - std::string* dest, std::string* error) { + absl::Nonnull dest, + absl::Nullable error) { strings_internal::STLStringResizeUninitialized(dest, source.size()); ptrdiff_t dest_size; @@ -392,7 +397,8 @@ inline size_t CEscapedLength(absl::string_view src) { return escaped_len; } -void CEscapeAndAppendInternal(absl::string_view src, std::string* dest) { +void CEscapeAndAppendInternal(absl::string_view src, + absl::Nonnull dest) { size_t escaped_len = CEscapedLength(src); if (escaped_len == src.size()) { dest->append(src.data(), src.size()); @@ -446,9 +452,10 @@ void CEscapeAndAppendInternal(absl::string_view src, std::string* dest) { // Reverses the mapping in Base64EscapeInternal; see that method's // documentation for details of the mapping. -bool Base64UnescapeInternal(const char* src_param, size_t szsrc, char* dest, - size_t szdest, const signed char* unbase64, - size_t* len) { +bool Base64UnescapeInternal(absl::Nullable src_param, size_t szsrc, + absl::Nullable dest, size_t szdest, + absl::Nonnull unbase64, + absl::Nonnull len) { static const char kPad64Equals = '='; static const char kPad64Dot = '.'; @@ -784,8 +791,9 @@ constexpr signed char kUnWebSafeBase64[] = { /* clang-format on */ template -bool Base64UnescapeInternal(const char* src, size_t slen, String* dest, - const signed char* unbase64) { +bool Base64UnescapeInternal(absl::Nullable src, size_t slen, + absl::Nonnull dest, + absl::Nonnull unbase64) { // Determine the size of the output string. Base64 encodes every 3 bytes into // 4 characters. Any leftover chars are added directly for good measure. const size_t dest_len = 3 * (slen / 4) + (slen % 4); @@ -835,7 +843,8 @@ constexpr char kHexValueLenient[256] = { // or a string. This works because we use the [] operator to access // individual characters at a time. template -void HexStringToBytesInternal(const char* from, T to, size_t num) { +void HexStringToBytesInternal(absl::Nullable from, T to, + size_t num) { for (size_t i = 0; i < num; i++) { to[i] = static_cast(kHexValueLenient[from[i * 2] & 0xFF] << 4) + (kHexValueLenient[from[i * 2 + 1] & 0xFF]); @@ -845,7 +854,8 @@ void HexStringToBytesInternal(const char* from, T to, size_t num) { // This is a templated function so that T can be either a char* or a // std::string. template -void BytesToHexStringInternal(const unsigned char* src, T dest, size_t num) { +void BytesToHexStringInternal(absl::Nullable src, T dest, + size_t num) { auto dest_ptr = &dest[0]; for (auto src_ptr = src; src_ptr != (src + num); ++src_ptr, dest_ptr += 2) { const char* hex_p = &numbers_internal::kHexTable[*src_ptr * 2]; @@ -860,8 +870,8 @@ void BytesToHexStringInternal(const unsigned char* src, T dest, size_t num) { // // See CUnescapeInternal() for implementation details. // ---------------------------------------------------------------------- -bool CUnescape(absl::string_view source, std::string* dest, - std::string* error) { +bool CUnescape(absl::string_view source, absl::Nonnull dest, + absl::Nullable error) { return CUnescapeInternal(source, kUnescapeNulls, dest, error); } @@ -883,21 +893,23 @@ std::string Utf8SafeCHexEscape(absl::string_view src) { return CEscapeInternal(src, true, true); } -bool Base64Unescape(absl::string_view src, std::string* dest) { +bool Base64Unescape(absl::string_view src, absl::Nonnull dest) { return Base64UnescapeInternal(src.data(), src.size(), dest, kUnBase64); } -bool WebSafeBase64Unescape(absl::string_view src, std::string* dest) { +bool WebSafeBase64Unescape(absl::string_view src, + absl::Nonnull dest) { return Base64UnescapeInternal(src.data(), src.size(), dest, kUnWebSafeBase64); } -void Base64Escape(absl::string_view src, std::string* dest) { +void Base64Escape(absl::string_view src, absl::Nonnull dest) { strings_internal::Base64EscapeInternal( reinterpret_cast(src.data()), src.size(), dest, true, strings_internal::kBase64Chars); } -void WebSafeBase64Escape(absl::string_view src, std::string* dest) { +void WebSafeBase64Escape(absl::string_view src, + absl::Nonnull dest) { strings_internal::Base64EscapeInternal( reinterpret_cast(src.data()), src.size(), dest, false, strings_internal::kWebSafeBase64Chars); diff --git a/absl/strings/escaping.h b/absl/strings/escaping.h index bf2a5898..f6f7a29c 100644 --- a/absl/strings/escaping.h +++ b/absl/strings/escaping.h @@ -28,6 +28,7 @@ #include #include "absl/base/macros.h" +#include "absl/base/nullability.h" #include "absl/strings/ascii.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" @@ -69,10 +70,12 @@ ABSL_NAMESPACE_BEGIN // ... // } // EXPECT_EQ(unescaped_s, "foo\rbar\nbaz\t"); -bool CUnescape(absl::string_view source, std::string* dest, std::string* error); +bool CUnescape(absl::string_view source, absl::Nonnull dest, + absl::Nullable error); // Overload of `CUnescape()` with no error reporting. -inline bool CUnescape(absl::string_view source, std::string* dest) { +inline bool CUnescape(absl::string_view source, + absl::Nonnull dest) { return CUnescape(source, dest, nullptr); } @@ -122,7 +125,7 @@ std::string Utf8SafeCHexEscape(absl::string_view src); // Encodes a `src` string into a base64-encoded 'dest' string with padding // characters. This function conforms with RFC 4648 section 4 (base64) and RFC // 2045. -void Base64Escape(absl::string_view src, std::string* dest); +void Base64Escape(absl::string_view src, absl::Nonnull dest); std::string Base64Escape(absl::string_view src); // WebSafeBase64Escape() @@ -130,7 +133,8 @@ std::string Base64Escape(absl::string_view src); // Encodes a `src` string into a base64 string, like Base64Escape() does, but // outputs '-' instead of '+' and '_' instead of '/', and does not pad 'dest'. // This function conforms with RFC 4648 section 5 (base64url). -void WebSafeBase64Escape(absl::string_view src, std::string* dest); +void WebSafeBase64Escape(absl::string_view src, + absl::Nonnull dest); std::string WebSafeBase64Escape(absl::string_view src); // Base64Unescape() @@ -140,7 +144,7 @@ std::string WebSafeBase64Escape(absl::string_view src); // `src` contains invalid characters, `dest` is cleared and returns `false`. // If padding is included (note that `Base64Escape()` does produce it), it must // be correct. In the padding, '=' and '.' are treated identically. -bool Base64Unescape(absl::string_view src, std::string* dest); +bool Base64Unescape(absl::string_view src, absl::Nonnull dest); // WebSafeBase64Unescape() // @@ -149,7 +153,8 @@ bool Base64Unescape(absl::string_view src, std::string* dest); // invalid characters, `dest` is cleared and returns `false`. If padding is // included (note that `WebSafeBase64Escape()` does not produce it), it must be // correct. In the padding, '=' and '.' are treated identically. -bool WebSafeBase64Unescape(absl::string_view src, std::string* dest); +bool WebSafeBase64Unescape(absl::string_view src, + absl::Nonnull dest); // HexStringToBytes() // -- cgit v1.2.3 From 9a79278a9793574cad2773b3445a887f949bc705 Mon Sep 17 00:00:00 2001 From: Matt Kulukundis Date: Mon, 29 Jan 2024 12:14:16 -0800 Subject: Fix a corner case in SpyHashState for exact boundaries. AbslHash allows for piecewise chunks to be streamed incrementally into hash states and requires them to hash identically to one giant stream. The exact size window for this is an internal details `PiecewiseChunkSize`. There was an off by one error in this code. Add tests and fix it. PiperOrigin-RevId: 602463183 Change-Id: I159bbb5e7e745f55b2fe6eaf0d2735bd0a08aca9 --- absl/hash/internal/spy_hash_state.h | 24 ++++++++++++------------ absl/strings/BUILD.bazel | 1 + absl/strings/CMakeLists.txt | 1 + absl/strings/cord_test.cc | 21 +++++++++++++++++++++ 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/absl/hash/internal/spy_hash_state.h b/absl/hash/internal/spy_hash_state.h index 09728266..357c301c 100644 --- a/absl/hash/internal/spy_hash_state.h +++ b/absl/hash/internal/spy_hash_state.h @@ -149,20 +149,20 @@ class SpyHashStateImpl : public HashStateBase> { const unsigned char* begin, size_t size) { const size_t large_chunk_stride = PiecewiseChunkSize(); - if (size > large_chunk_stride) { - // Combining a large contiguous buffer must have the same effect as - // doing it piecewise by the stride length, followed by the (possibly - // empty) remainder. - while (size >= large_chunk_stride) { - hash_state = SpyHashStateImpl::combine_contiguous( - std::move(hash_state), begin, large_chunk_stride); - begin += large_chunk_stride; - size -= large_chunk_stride; - } + // Combining a large contiguous buffer must have the same effect as + // doing it piecewise by the stride length, followed by the (possibly + // empty) remainder. + while (size > large_chunk_stride) { + hash_state = SpyHashStateImpl::combine_contiguous( + std::move(hash_state), begin, large_chunk_stride); + begin += large_chunk_stride; + size -= large_chunk_stride; } - hash_state.hash_representation_.emplace_back( - reinterpret_cast(begin), size); + if (size > 0) { + hash_state.hash_representation_.emplace_back( + reinterpret_cast(begin), size); + } return hash_state; } diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 8e8371b3..ed2ab04f 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -918,6 +918,7 @@ cc_test( "//absl/container:fixed_array", "//absl/functional:function_ref", "//absl/hash", + "//absl/hash:hash_testing", "//absl/log", "//absl/log:check", "//absl/random", diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index 1b245362..ce9c5e46 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -1072,6 +1072,7 @@ absl_cc_test( absl::fixed_array absl::function_ref absl::hash + absl::hash_testing absl::log absl::optional absl::random_random diff --git a/absl/strings/cord_test.cc b/absl/strings/cord_test.cc index f1a5f39c..69b1a69e 100644 --- a/absl/strings/cord_test.cc +++ b/absl/strings/cord_test.cc @@ -42,6 +42,7 @@ #include "absl/container/fixed_array.h" #include "absl/functional/function_ref.h" #include "absl/hash/hash.h" +#include "absl/hash/hash_testing.h" #include "absl/log/check.h" #include "absl/log/log.h" #include "absl/random/random.h" @@ -2013,6 +2014,26 @@ TEST(CordTest, CordMemoryUsageBTree) { rep2_size); } +TEST(CordTest, TestHashFragmentation) { + // Make sure we hit these boundary cases precisely. + EXPECT_EQ(1024, absl::hash_internal::PiecewiseChunkSize()); + EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly({ + absl::Cord(), + absl::MakeFragmentedCord({std::string(600, 'a'), std::string(600, 'a')}), + absl::MakeFragmentedCord({std::string(1200, 'a')}), + absl::MakeFragmentedCord({std::string(900, 'b'), std::string(900, 'b')}), + absl::MakeFragmentedCord({std::string(1800, 'b')}), + absl::MakeFragmentedCord( + {std::string(2000, 'c'), std::string(2000, 'c')}), + absl::MakeFragmentedCord({std::string(4000, 'c')}), + absl::MakeFragmentedCord({std::string(1024, 'd')}), + absl::MakeFragmentedCord({std::string(1023, 'd'), "d"}), + absl::MakeFragmentedCord({std::string(1025, 'e')}), + absl::MakeFragmentedCord({std::string(1024, 'e'), "e"}), + absl::MakeFragmentedCord({std::string(1023, 'e'), "e", "e"}), + })); +} + // Regtest for a change that had to be rolled back because it expanded out // of the InlineRep too soon, which was observable through MemoryUsage(). TEST_P(CordTest, CordMemoryUsageInlineRep) { -- cgit v1.2.3 From d5eb503257f23d41f3e67933ffd85fb989141acb Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 29 Jan 2024 13:33:24 -0800 Subject: Introduce `RawHashSetLayout` helper class. PiperOrigin-RevId: 602485199 Change-Id: I5cc10eb8dcfe5988cf939080a224522e02ad8607 --- absl/container/internal/raw_hash_set.h | 104 ++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 41 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 3518bc34..ba7df607 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -80,7 +80,7 @@ // slot_type slots[capacity]; // }; // -// The length of this array is computed by `AllocSize()` below. +// The length of this array is computed by `RawHashSetLayout::alloc_size` below. // // Control bytes (`ctrl_t`) are bytes (collected into groups of a // platform-specific size) that define the state of the corresponding slot in @@ -983,12 +983,6 @@ using HashSetIteratorGenerationInfo = HashSetIteratorGenerationInfoDisabled; // A valid capacity is a non-zero integer `2^m - 1`. inline bool IsValidCapacity(size_t n) { return ((n + 1) & n) == 0 && n > 0; } -// Computes the offset from the start of the backing allocation of control. -// infoz and growth_left are stored at the beginning of the backing array. -inline size_t ControlOffset(bool has_infoz) { - return (has_infoz ? sizeof(HashtablezInfoHandle) : 0) + sizeof(size_t); -} - // Returns the number of "cloned control bytes". // // This is the number of control bytes that are present both at the beginning @@ -996,29 +990,57 @@ inline size_t ControlOffset(bool has_infoz) { // `Group::kWidth`-width probe window starting from any control byte. constexpr size_t NumClonedBytes() { return Group::kWidth - 1; } -// Given the capacity of a table, computes the offset (from the start of the -// backing allocation) of the generation counter (if it exists). -inline size_t GenerationOffset(size_t capacity, bool has_infoz) { - assert(IsValidCapacity(capacity)); - const size_t num_control_bytes = capacity + 1 + NumClonedBytes(); - return ControlOffset(has_infoz) + num_control_bytes; +// Returns the number of control bytes including cloned. +inline size_t NumControlBytes(size_t capacity) { + return capacity + 1 + NumClonedBytes(); } -// Given the capacity of a table, computes the offset (from the start of the -// backing allocation) at which the slots begin. -inline size_t SlotOffset(size_t capacity, size_t slot_align, bool has_infoz) { - assert(IsValidCapacity(capacity)); - return (GenerationOffset(capacity, has_infoz) + NumGenerationBytes() + - slot_align - 1) & - (~slot_align + 1); +// Computes the offset from the start of the backing allocation of control. +// infoz and growth_left are stored at the beginning of the backing array. +inline static size_t ControlOffset(bool has_infoz) { + return (has_infoz ? sizeof(HashtablezInfoHandle) : 0) + sizeof(size_t); } -// Given the capacity of a table, computes the total size of the backing -// array. -inline size_t AllocSize(size_t capacity, size_t slot_size, size_t slot_align, - bool has_infoz) { - return SlotOffset(capacity, slot_align, has_infoz) + capacity * slot_size; -} +// Helper class for computing offsets and allocation size of hash set fields. +class RawHashSetLayout { + public: + explicit RawHashSetLayout(size_t capacity, size_t slot_align, bool has_infoz) + : capacity_(capacity), + control_offset_(ControlOffset(has_infoz)), + generation_offset_(control_offset_ + NumControlBytes(capacity)), + slot_offset_( + (generation_offset_ + NumGenerationBytes() + slot_align - 1) & + (~slot_align + 1)) { + assert(IsValidCapacity(capacity)); + } + + // Returns the capacity of a table. + size_t capacity() const { return capacity_; } + + // Returns precomputed offset from the start of the backing allocation of + // control. + size_t control_offset() const { return control_offset_; } + + // Given the capacity of a table, computes the offset (from the start of the + // backing allocation) of the generation counter (if it exists). + size_t generation_offset() const { return generation_offset_; } + + // Given the capacity of a table, computes the offset (from the start of the + // backing allocation) at which the slots begin. + size_t slot_offset() const { return slot_offset_; } + + // Given the capacity of a table, computes the total size of the backing + // array. + size_t alloc_size(size_t slot_size) const { + return slot_offset_ + capacity_ * slot_size; + } + + private: + size_t capacity_; + size_t control_offset_; + size_t generation_offset_; + size_t slot_offset_; +}; // CommonFields hold the fields in raw_hash_set that do not depend // on template parameters. This allows us to conveniently pass all @@ -1116,7 +1138,8 @@ class CommonFields : public CommonFieldsGenerationInfo { // The size of the backing array allocation. size_t alloc_size(size_t slot_size, size_t slot_align) const { - return AllocSize(capacity(), slot_size, slot_align, has_infoz()); + return RawHashSetLayout(capacity(), slot_align, has_infoz()) + .alloc_size(slot_size); } // Returns the number of control bytes set to kDeleted. For testing only. @@ -1608,27 +1631,25 @@ class HashSetResizeHelper { sample_size > 0 ? Sample(sample_size) : c.infoz(); const bool has_infoz = infoz.IsSampled(); - const size_t cap = c.capacity(); - const size_t alloc_size = - AllocSize(cap, SizeOfSlot, AlignOfSlot, has_infoz); - char* mem = static_cast( - Allocate(&alloc, alloc_size)); + RawHashSetLayout layout(c.capacity(), AlignOfSlot, has_infoz); + char* mem = static_cast(Allocate( + &alloc, layout.alloc_size(SizeOfSlot))); const GenerationType old_generation = c.generation(); - c.set_generation_ptr(reinterpret_cast( - mem + GenerationOffset(cap, has_infoz))); + c.set_generation_ptr( + reinterpret_cast(mem + layout.generation_offset())); c.set_generation(NextGeneration(old_generation)); - c.set_control(reinterpret_cast(mem + ControlOffset(has_infoz))); - c.set_slots(mem + SlotOffset(cap, AlignOfSlot, has_infoz)); + c.set_control(reinterpret_cast(mem + layout.control_offset())); + c.set_slots(mem + layout.slot_offset()); ResetGrowthLeft(c); const bool grow_single_group = - IsGrowingIntoSingleGroupApplicable(old_capacity_, c.capacity()); + IsGrowingIntoSingleGroupApplicable(old_capacity_, layout.capacity()); if (old_capacity_ != 0 && grow_single_group) { if (TransferUsesMemcpy) { GrowSizeIntoSingleGroupTransferable(c, old_slots, SizeOfSlot); DeallocateOld(alloc, SizeOfSlot, old_slots); } else { - GrowIntoSingleGroupShuffleControlBytes(c.control(), c.capacity()); + GrowIntoSingleGroupShuffleControlBytes(c.control(), layout.capacity()); } } else { ResetCtrl(c, SizeOfSlot); @@ -1636,7 +1657,7 @@ class HashSetResizeHelper { c.set_has_infoz(has_infoz); if (has_infoz) { - infoz.RecordStorageChanged(c.size(), cap); + infoz.RecordStorageChanged(c.size(), layout.capacity()); if (grow_single_group || old_capacity_ == 0) { infoz.RecordRehash(0); } @@ -1675,9 +1696,10 @@ class HashSetResizeHelper { template void DeallocateOld(CharAlloc alloc_ref, size_t slot_size, void* old_slots) { SanitizerUnpoisonMemoryRegion(old_slots, slot_size * old_capacity_); + auto layout = RawHashSetLayout(old_capacity_, AlignOfSlot, had_infoz_); Deallocate( - &alloc_ref, old_ctrl_ - ControlOffset(had_infoz_), - AllocSize(old_capacity_, slot_size, AlignOfSlot, had_infoz_)); + &alloc_ref, old_ctrl_ - layout.control_offset(), + layout.alloc_size(slot_size)); } private: -- cgit v1.2.3 From 04af270f6d3a0e647946663764a5f869c104af8f Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Mon, 29 Jan 2024 15:04:23 -0800 Subject: Add empty WORKSPACE.bzlmod When bzlmod is enabled and WORKSPACE.bzlmod exists, the content of WORKSPACE is ignored. This prevents bzlmod builds from unintentionally depending on the WORKSPACE file. This exposed some small problems with our clang-cl configuration, which are also fixed in this change. PiperOrigin-RevId: 602511311 Change-Id: I0ba411edde2df0e17f4eeede4117ddb6934dd8f8 --- BUILD.bazel | 12 +++++++++++- MODULE.bazel | 3 +++ WORKSPACE.bzlmod | 19 +++++++++++++++++++ absl/BUILD.bazel | 11 ----------- ci/windows_clangcl_bazel.bat | 2 +- 5 files changed, 34 insertions(+), 13 deletions(-) create mode 100644 WORKSPACE.bzlmod diff --git a/BUILD.bazel b/BUILD.bazel index 79fb0ecd..03122a95 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,4 +1,3 @@ -# # Copyright 2020 The Abseil Authors. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,3 +22,14 @@ exports_files([ "AUTHORS", "LICENSE", ]) + +# For building with clang-cl. +# https://bazel.build/configure/windows#clang +platform( + name = "x64_windows-clang-cl", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@bazel_tools//tools/cpp:clang-cl", + ], +) diff --git a/MODULE.bazel b/MODULE.bazel index 18190e8e..fc1534a7 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -20,6 +20,9 @@ module( compatibility_level = 1, ) +cc_configure = use_extension("@bazel_tools//tools/cpp:cc_configure.bzl", "cc_configure_extension") +use_repo(cc_configure, "local_config_cc") + # Only direct dependencies need to be listed below. # Please keep the versions in sync with the versions in the WORKSPACE file. diff --git a/WORKSPACE.bzlmod b/WORKSPACE.bzlmod new file mode 100644 index 00000000..83e67baa --- /dev/null +++ b/WORKSPACE.bzlmod @@ -0,0 +1,19 @@ +# Copyright 2024 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. + +# https://bazel.build/external/migration#workspace.bzlmod +# +# This file is intentionally empty. When bzlmod is enabled and this +# file exists, the contents of WORKSPACE is ignored. This prevents +# bzlmod builds from unintentionally depending on the WORKSPACE file. diff --git a/absl/BUILD.bazel b/absl/BUILD.bazel index 14c30b38..253c0aef 100644 --- a/absl/BUILD.bazel +++ b/absl/BUILD.bazel @@ -68,17 +68,6 @@ config_setting( visibility = [":__subpackages__"], ) -# x64_windows-clang-cl - used for selecting clang-cl for CI builds -platform( - name = "x64_windows-clang-cl", - constraint_values = [ - "@platforms//cpu:x86_64", - "@platforms//os:windows", - "@bazel_tools//tools/cpp:clang-cl", - ], - visibility = [":__subpackages__"], -) - config_setting( name = "osx", constraint_values = [ diff --git a/ci/windows_clangcl_bazel.bat b/ci/windows_clangcl_bazel.bat index 5162628b..ac2f9167 100755 --- a/ci/windows_clangcl_bazel.bat +++ b/ci/windows_clangcl_bazel.bat @@ -49,7 +49,7 @@ IF NOT "%ALTERNATE_OPTIONS%"=="" copy %ALTERNATE_OPTIONS% absl\base\options.h --define=absl=1 ^ --distdir=%KOKORO_GFILE_DIR%\distdir ^ --enable_bzlmod=true ^ - --extra_execution_platforms=//absl:x64_windows-clang-cl ^ + --extra_execution_platforms=//:x64_windows-clang-cl ^ --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl ^ --keep_going ^ --test_env="GTEST_INSTALL_FAILURE_SIGNAL_HANDLER=1" ^ -- cgit v1.2.3 From cbdbec098f6365c5afc1e3c088a6f48fb8835b59 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 30 Jan 2024 08:13:49 -0800 Subject: Use absl::NoDestructor for cordz global queue. Also updated the return value to reference to clarify non-nullability. PiperOrigin-RevId: 602730828 Change-Id: Ia36f7fde3cc87ac597ba4f194eebe9ebb90a1a09 --- absl/strings/BUILD.bazel | 2 +- absl/strings/CMakeLists.txt | 1 + absl/strings/internal/cordz_handle.cc | 48 +++++++++++++++++------------------ 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index ed2ab04f..1b6c7e4b 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -614,8 +614,8 @@ cc_library( "//absl:__subpackages__", ], deps = [ - "//absl/base", "//absl/base:config", + "//absl/base:no_destructor", "//absl/base:raw_logging_internal", "//absl/synchronization", ], diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index ce9c5e46..9258e553 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -799,6 +799,7 @@ absl_cc_library( DEPS absl::base absl::config + absl::no_destructor absl::raw_logging_internal absl::synchronization ) diff --git a/absl/strings/internal/cordz_handle.cc b/absl/strings/internal/cordz_handle.cc index a7061dbe..53d5f529 100644 --- a/absl/strings/internal/cordz_handle.cc +++ b/absl/strings/internal/cordz_handle.cc @@ -16,6 +16,7 @@ #include #include "absl/base/internal/raw_logging.h" // For ABSL_RAW_CHECK +#include "absl/base/no_destructor.h" #include "absl/synchronization/mutex.h" namespace absl { @@ -43,33 +44,32 @@ struct Queue { } }; -static Queue* GlobalQueue() { - static Queue* global_queue = new Queue; - return global_queue; +static Queue& GlobalQueue() { + static absl::NoDestructor global_queue; + return *global_queue; } } // namespace CordzHandle::CordzHandle(bool is_snapshot) : is_snapshot_(is_snapshot) { - Queue* global_queue = GlobalQueue(); + Queue& global_queue = GlobalQueue(); if (is_snapshot) { - MutexLock lock(&global_queue->mutex); - CordzHandle* dq_tail = - global_queue->dq_tail.load(std::memory_order_acquire); + MutexLock lock(&global_queue.mutex); + CordzHandle* dq_tail = global_queue.dq_tail.load(std::memory_order_acquire); if (dq_tail != nullptr) { dq_prev_ = dq_tail; dq_tail->dq_next_ = this; } - global_queue->dq_tail.store(this, std::memory_order_release); + global_queue.dq_tail.store(this, std::memory_order_release); } } CordzHandle::~CordzHandle() { - Queue* global_queue = GlobalQueue(); + Queue& global_queue = GlobalQueue(); if (is_snapshot_) { std::vector to_delete; { - MutexLock lock(&global_queue->mutex); + MutexLock lock(&global_queue.mutex); CordzHandle* next = dq_next_; if (dq_prev_ == nullptr) { // We were head of the queue, delete every CordzHandle until we reach @@ -85,7 +85,7 @@ CordzHandle::~CordzHandle() { if (next) { next->dq_prev_ = dq_prev_; } else { - global_queue->dq_tail.store(dq_prev_, std::memory_order_release); + global_queue.dq_tail.store(dq_prev_, std::memory_order_release); } } for (CordzHandle* handle : to_delete) { @@ -95,20 +95,20 @@ CordzHandle::~CordzHandle() { } bool CordzHandle::SafeToDelete() const { - return is_snapshot_ || GlobalQueue()->IsEmpty(); + return is_snapshot_ || GlobalQueue().IsEmpty(); } void CordzHandle::Delete(CordzHandle* handle) { assert(handle); if (handle) { - Queue* const queue = GlobalQueue(); + Queue& queue = GlobalQueue(); if (!handle->SafeToDelete()) { - MutexLock lock(&queue->mutex); - CordzHandle* dq_tail = queue->dq_tail.load(std::memory_order_acquire); + MutexLock lock(&queue.mutex); + CordzHandle* dq_tail = queue.dq_tail.load(std::memory_order_acquire); if (dq_tail != nullptr) { handle->dq_prev_ = dq_tail; dq_tail->dq_next_ = handle; - queue->dq_tail.store(handle, std::memory_order_release); + queue.dq_tail.store(handle, std::memory_order_release); return; } } @@ -118,9 +118,9 @@ void CordzHandle::Delete(CordzHandle* handle) { std::vector CordzHandle::DiagnosticsGetDeleteQueue() { std::vector handles; - Queue* global_queue = GlobalQueue(); - MutexLock lock(&global_queue->mutex); - CordzHandle* dq_tail = global_queue->dq_tail.load(std::memory_order_acquire); + Queue& global_queue = GlobalQueue(); + MutexLock lock(&global_queue.mutex); + CordzHandle* dq_tail = global_queue.dq_tail.load(std::memory_order_acquire); for (const CordzHandle* p = dq_tail; p; p = p->dq_prev_) { handles.push_back(p); } @@ -133,9 +133,9 @@ bool CordzHandle::DiagnosticsHandleIsSafeToInspect( if (handle == nullptr) return true; if (handle->is_snapshot_) return false; bool snapshot_found = false; - Queue* global_queue = GlobalQueue(); - MutexLock lock(&global_queue->mutex); - for (const CordzHandle* p = global_queue->dq_tail; p; p = p->dq_prev_) { + Queue& global_queue = GlobalQueue(); + MutexLock lock(&global_queue.mutex); + for (const CordzHandle* p = global_queue.dq_tail; p; p = p->dq_prev_) { if (p == handle) return !snapshot_found; if (p == this) snapshot_found = true; } @@ -150,8 +150,8 @@ CordzHandle::DiagnosticsGetSafeToInspectDeletedHandles() { return handles; } - Queue* global_queue = GlobalQueue(); - MutexLock lock(&global_queue->mutex); + Queue& global_queue = GlobalQueue(); + MutexLock lock(&global_queue.mutex); for (const CordzHandle* p = dq_next_; p != nullptr; p = p->dq_next_) { if (!p->is_snapshot()) { handles.push_back(p); -- cgit v1.2.3 From 779a3565ac6c5b69dd1ab9183e500a27633117d5 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Tue, 30 Jan 2024 10:13:25 -0800 Subject: Avoid export of testonly target absl::test_allocator in CMake builds Closes #1536 PiperOrigin-RevId: 602764437 Change-Id: Ia5c20a3874262a2ddb8797f608af17d7e86dd6d6 --- absl/container/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 449a2cad..ee9ca9c3 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -213,6 +213,7 @@ absl_cc_library( DEPS absl::config GTest::gmock + TESTONLY ) absl_cc_test( -- cgit v1.2.3 From c44dd5acd7848a175d1fb939cdb81a4cf8d4c912 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 30 Jan 2024 12:52:43 -0800 Subject: Early return from destroy_slots for trivially destructible types in flat_hash_{*}. PiperOrigin-RevId: 602813933 Change-Id: I744fe438281755a141b2fd47e54ab9c6c0fad5a3 --- absl/container/BUILD.bazel | 2 + absl/container/CMakeLists.txt | 2 + absl/container/flat_hash_map.h | 5 +- absl/container/flat_hash_map_test.cc | 12 ++++ absl/container/flat_hash_set.h | 5 +- absl/container/flat_hash_set_test.cc | 12 ++++ absl/container/internal/common_policy_traits.h | 12 +++- .../internal/common_policy_traits_test.cc | 65 +++++++++++++++------- absl/container/internal/container_memory.h | 15 ++++- absl/container/internal/container_memory_test.cc | 18 ++++++ absl/container/internal/raw_hash_set.h | 1 + 11 files changed, 122 insertions(+), 27 deletions(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index fda866a5..bc5b2201 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -264,6 +264,7 @@ cc_test( deps = [ ":flat_hash_map", ":hash_generator_testing", + ":test_allocator", ":unordered_map_constructor_test", ":unordered_map_lookup_test", ":unordered_map_members_test", @@ -301,6 +302,7 @@ cc_test( ":container_memory", ":flat_hash_set", ":hash_generator_testing", + ":test_allocator", ":unordered_set_constructor_test", ":unordered_set_lookup_test", ":unordered_set_members_test", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index ee9ca9c3..3f80d29b 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -307,6 +307,7 @@ absl_cc_test( absl::check absl::flat_hash_map absl::hash_generator_testing + absl::test_allocator absl::type_traits absl::unordered_map_constructor_test absl::unordered_map_lookup_test @@ -348,6 +349,7 @@ absl_cc_test( absl::hash_generator_testing absl::memory absl::strings + absl::test_allocator absl::unordered_set_constructor_test absl::unordered_set_lookup_test absl::unordered_set_members_test diff --git a/absl/container/flat_hash_map.h b/absl/container/flat_hash_map.h index acd013b0..808a62ba 100644 --- a/absl/container/flat_hash_map.h +++ b/absl/container/flat_hash_map.h @@ -573,9 +573,10 @@ struct FlatHashMapPolicy { slot_policy::construct(alloc, slot, std::forward(args)...); } + // Returns std::true_type in case destroy is trivial. template - static void destroy(Allocator* alloc, slot_type* slot) { - slot_policy::destroy(alloc, slot); + static auto destroy(Allocator* alloc, slot_type* slot) { + return slot_policy::destroy(alloc, slot); } template diff --git a/absl/container/flat_hash_map_test.cc b/absl/container/flat_hash_map_test.cc index d90fe9d5..8ef1a62b 100644 --- a/absl/container/flat_hash_map_test.cc +++ b/absl/container/flat_hash_map_test.cc @@ -22,6 +22,7 @@ #include "gtest/gtest.h" #include "absl/container/internal/hash_generator_testing.h" +#include "absl/container/internal/test_allocator.h" #include "absl/container/internal/unordered_map_constructor_test.h" #include "absl/container/internal/unordered_map_lookup_test.h" #include "absl/container/internal/unordered_map_members_test.h" @@ -351,6 +352,17 @@ TEST(FlatHashMap, RecursiveTypeCompiles) { t.m[0] = RecursiveType{}; } +TEST(FlatHashMap, FlatHashMapPolicyDestroyReturnsTrue) { + EXPECT_TRUE( + (decltype(FlatHashMapPolicy::destroy>( + nullptr, nullptr))())); + EXPECT_FALSE( + (decltype(FlatHashMapPolicy::destroy>( + nullptr, nullptr))())); + EXPECT_FALSE((decltype(FlatHashMapPolicy>::destroy< + std::allocator>(nullptr, nullptr))())); +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/flat_hash_set.h b/absl/container/flat_hash_set.h index a94a82a0..cd74923c 100644 --- a/absl/container/flat_hash_set.h +++ b/absl/container/flat_hash_set.h @@ -29,6 +29,7 @@ #ifndef ABSL_CONTAINER_FLAT_HASH_SET_H_ #define ABSL_CONTAINER_FLAT_HASH_SET_H_ +#include #include #include @@ -473,9 +474,11 @@ struct FlatHashSetPolicy { std::forward(args)...); } + // Return std::true_type in case destroy is trivial. template - static void destroy(Allocator* alloc, slot_type* slot) { + static auto destroy(Allocator* alloc, slot_type* slot) { absl::allocator_traits::destroy(*alloc, slot); + return IsDestructionTrivial(); } static T& element(slot_type* slot) { return *slot; } diff --git a/absl/container/flat_hash_set_test.cc b/absl/container/flat_hash_set_test.cc index a60b4bf5..9ce9267e 100644 --- a/absl/container/flat_hash_set_test.cc +++ b/absl/container/flat_hash_set_test.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -24,6 +25,7 @@ #include "absl/base/config.h" #include "absl/container/internal/container_memory.h" #include "absl/container/internal/hash_generator_testing.h" +#include "absl/container/internal/test_allocator.h" #include "absl/container/internal/unordered_set_constructor_test.h" #include "absl/container/internal/unordered_set_lookup_test.h" #include "absl/container/internal/unordered_set_members_test.h" @@ -237,6 +239,16 @@ TEST(FlatHashSet, PoisonInline) { } } +TEST(FlatHashSet, FlatHashSetPolicyDestroyReturnsTrue) { + EXPECT_TRUE((decltype(FlatHashSetPolicy::destroy>( + nullptr, nullptr))())); + EXPECT_FALSE( + (decltype(FlatHashSetPolicy::destroy>( + nullptr, nullptr))())); + EXPECT_FALSE((decltype(FlatHashSetPolicy>::destroy< + std::allocator>(nullptr, nullptr))())); +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/common_policy_traits.h b/absl/container/internal/common_policy_traits.h index 57eac678..77df4790 100644 --- a/absl/container/internal/common_policy_traits.h +++ b/absl/container/internal/common_policy_traits.h @@ -45,9 +45,10 @@ struct common_policy_traits { // PRECONDITION: `slot` is INITIALIZED // POSTCONDITION: `slot` is UNINITIALIZED + // Returns std::true_type in case destroy is trivial. template - static void destroy(Alloc* alloc, slot_type* slot) { - Policy::destroy(alloc, slot); + static auto destroy(Alloc* alloc, slot_type* slot) { + return Policy::destroy(alloc, slot); } // Transfers the `old_slot` to `new_slot`. Any memory allocated by the @@ -86,6 +87,13 @@ struct common_policy_traits { std::true_type>::value; } + // Returns true if destroy is trivial and can be omitted. + template + static constexpr bool destroy_is_trivial() { + return std::is_same(nullptr, nullptr)), + std::true_type>::value; + } + private: // To rank the overloads below for overload resolution. Rank0 is preferred. struct Rank2 {}; diff --git a/absl/container/internal/common_policy_traits_test.cc b/absl/container/internal/common_policy_traits_test.cc index faee3e7a..8d8f8baa 100644 --- a/absl/container/internal/common_policy_traits_test.cc +++ b/absl/container/internal/common_policy_traits_test.cc @@ -39,44 +39,59 @@ struct PolicyWithoutOptionalOps { using key_type = Slot; using init_type = Slot; - static std::function construct; - static std::function destroy; + struct PolicyFunctions { + std::function construct; + std::function destroy; + std::function element; + }; + + static PolicyFunctions* functions() { + static PolicyFunctions* functions = new PolicyFunctions(); + return functions; + } - static std::function element; + static void construct(void* a, Slot* b, Slot c) { + functions()->construct(a, b, c); + } + static void destroy(void* a, Slot* b) { functions()->destroy(a, b); } + static Slot& element(Slot* b) { return functions()->element(b); } }; -std::function PolicyWithoutOptionalOps::construct; -std::function PolicyWithoutOptionalOps::destroy; - -std::function PolicyWithoutOptionalOps::element; - struct PolicyWithOptionalOps : PolicyWithoutOptionalOps { - static std::function transfer; + struct TransferFunctions { + std::function transfer; + }; + + static TransferFunctions* transfer_fn() { + static TransferFunctions* transfer_fn = new TransferFunctions(); + return transfer_fn; + } + static void transfer(void* a, Slot* b, Slot* c) { + transfer_fn()->transfer(a, b, c); + } }; -std::function PolicyWithOptionalOps::transfer; -struct PolicyWithMemcpyTransfer : PolicyWithoutOptionalOps { - static std::function transfer; +struct PolicyWithMemcpyTransferAndTrivialDestroy : PolicyWithoutOptionalOps { + static std::true_type transfer(void*, Slot*, Slot*) { return {}; } + static std::true_type destroy(void*, Slot*) { return {}; } }; -std::function - PolicyWithMemcpyTransfer::transfer; struct Test : ::testing::Test { Test() { - PolicyWithoutOptionalOps::construct = [&](void* a1, Slot* a2, Slot a3) { + PolicyWithoutOptionalOps::functions()->construct = [&](void* a1, Slot* a2, + Slot a3) { construct.Call(a1, a2, std::move(a3)); }; - PolicyWithoutOptionalOps::destroy = [&](void* a1, Slot* a2) { + PolicyWithoutOptionalOps::functions()->destroy = [&](void* a1, Slot* a2) { destroy.Call(a1, a2); }; - PolicyWithoutOptionalOps::element = [&](Slot* a1) -> Slot& { + PolicyWithoutOptionalOps::functions()->element = [&](Slot* a1) -> Slot& { return element.Call(a1); }; - PolicyWithOptionalOps::transfer = [&](void* a1, Slot* a2, Slot* a3) { - return transfer.Call(a1, a2, a3); - }; + PolicyWithOptionalOps::transfer_fn()->transfer = + [&](void* a1, Slot* a2, Slot* a3) { return transfer.Call(a1, a2, a3); }; } std::allocator alloc; @@ -125,7 +140,15 @@ TEST(TransferUsesMemcpy, Basic) { EXPECT_FALSE( common_policy_traits::transfer_uses_memcpy()); EXPECT_TRUE( - common_policy_traits::transfer_uses_memcpy()); + common_policy_traits< + PolicyWithMemcpyTransferAndTrivialDestroy>::transfer_uses_memcpy()); +} + +TEST(DestroyIsTrivial, Basic) { + EXPECT_FALSE(common_policy_traits::destroy_is_trivial< + std::allocator>()); + EXPECT_TRUE(common_policy_traits:: + destroy_is_trivial>()); } } // namespace diff --git a/absl/container/internal/container_memory.h b/absl/container/internal/container_memory.h index 3262d4eb..d86d7b80 100644 --- a/absl/container/internal/container_memory.h +++ b/absl/container/internal/container_memory.h @@ -68,6 +68,18 @@ void* Allocate(Alloc* alloc, size_t n) { return p; } +// Returns true if the destruction of the value with given Allocator will be +// trivial. +template +constexpr auto IsDestructionTrivial() { + constexpr bool result = + std::is_trivially_destructible::value && + std::is_same::template rebind_alloc, + std::allocator>::value; + return std::integral_constant(); +} + // The pointer must have been previously obtained by calling // Allocate(alloc, n). template @@ -414,12 +426,13 @@ struct map_slot_policy { } template - static void destroy(Allocator* alloc, slot_type* slot) { + static auto destroy(Allocator* alloc, slot_type* slot) { if (kMutableKeys::value) { absl::allocator_traits::destroy(*alloc, &slot->mutable_value); } else { absl::allocator_traits::destroy(*alloc, &slot->value); } + return IsDestructionTrivial(); } template diff --git a/absl/container/internal/container_memory_test.cc b/absl/container/internal/container_memory_test.cc index 90d64bf5..c973ac25 100644 --- a/absl/container/internal/container_memory_test.cc +++ b/absl/container/internal/container_memory_test.cc @@ -280,6 +280,24 @@ TEST(MapSlotPolicy, TransferReturnsTrue) { } } +TEST(MapSlotPolicy, DestroyReturnsTrue) { + { + using slot_policy = map_slot_policy; + EXPECT_TRUE( + (std::is_same>( + nullptr, nullptr)), + std::true_type>::value)); + } + { + EXPECT_FALSE(std::is_trivially_destructible>::value); + using slot_policy = map_slot_policy>; + EXPECT_TRUE( + (std::is_same>( + nullptr, nullptr)), + std::false_type>::value)); + } +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index ba7df607..9f16a815 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -2881,6 +2881,7 @@ class raw_hash_set { } inline void destroy_slots() { + if (PolicyTraits::template destroy_is_trivial()) return; const size_t cap = capacity(); const ctrl_t* ctrl = control(); slot_type* slot = slot_array(); -- cgit v1.2.3 From 0aefaf7ff41ac37852d5c4c1f6eb3d5464793f1c Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 30 Jan 2024 15:53:28 -0800 Subject: Missing parenthesis. PiperOrigin-RevId: 602864815 Change-Id: I74db20c93f6a7d9046779677cb4a328dc54bae69 --- absl/strings/escaping.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/strings/escaping.h b/absl/strings/escaping.h index f6f7a29c..8f980178 100644 --- a/absl/strings/escaping.h +++ b/absl/strings/escaping.h @@ -66,7 +66,7 @@ ABSL_NAMESPACE_BEGIN // // std::string s = "foo\\rbar\\nbaz\\t"; // std::string unescaped_s; -// if (!absl::CUnescape(s, &unescaped_s) { +// if (!absl::CUnescape(s, &unescaped_s)) { // ... // } // EXPECT_EQ(unescaped_s, "foo\rbar\nbaz\t"); -- cgit v1.2.3 From 2812af9184eaa2bfd18d1545c57bcf8cbee88a9d Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 31 Jan 2024 01:37:11 -0800 Subject: Avoid extra `& msbs` on every iteration over the mask for GroupPortableImpl. PiperOrigin-RevId: 602974812 Change-Id: Ic35b41e321b9456a8ddd83470ee2eb07c51e3180 --- absl/container/internal/raw_hash_set.h | 49 ++++++++++++++----------- absl/container/internal/raw_hash_set_test.cc | 54 ++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 24 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 9f16a815..c464de6a 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -374,6 +374,9 @@ uint32_t TrailingZeros(T x) { return static_cast(countr_zero(x)); } +// 8 bytes bitmask with most significant bit set for every byte. +constexpr uint64_t kMsbs8Bytes = 0x8080808080808080ULL; + // An abstract bitmask, such as that emitted by a SIMD instruction. // // Specifically, this type implements a simple bitset whose representation is @@ -423,27 +426,35 @@ class NonIterableBitMask { // an ordinary 16-bit bitset occupying the low 16 bits of `mask`. When // `SignificantBits` is 8 and `Shift` is 3, abstract bits are represented as // the bytes `0x00` and `0x80`, and it occupies all 64 bits of the bitmask. +// If NullifyBitsOnIteration is true (only allowed for Shift == 3), +// non zero abstract bit is allowed to have additional bits +// (e.g., `0xff`, `0x83` and `0x9c` are ok, but `0x6f` is not). // // For example: // for (int i : BitMask(0b101)) -> yields 0, 2 // for (int i : BitMask(0x0000000080800000)) -> yields 2, 3 -template +template class BitMask : public NonIterableBitMask { using Base = NonIterableBitMask; static_assert(std::is_unsigned::value, ""); static_assert(Shift == 0 || Shift == 3, ""); + static_assert(!NullifyBitsOnIteration || Shift == 3, ""); public: - explicit BitMask(T mask) : Base(mask) {} + explicit BitMask(T mask) : Base(mask) { + if (Shift == 3 && !NullifyBitsOnIteration) { + assert(this->mask_ == (this->mask_ & kMsbs8Bytes)); + } + } // BitMask is an iterator over the indices of its abstract bits. using value_type = int; using iterator = BitMask; using const_iterator = BitMask; BitMask& operator++() { - if (Shift == 3) { - constexpr uint64_t msbs = 0x8080808080808080ULL; - this->mask_ &= msbs; + if (Shift == 3 && NullifyBitsOnIteration) { + this->mask_ &= kMsbs8Bytes; } this->mask_ &= (this->mask_ - 1); return *this; @@ -685,10 +696,11 @@ struct GroupAArch64Impl { ctrl = vld1_u8(reinterpret_cast(pos)); } - BitMask Match(h2_t hash) const { + auto Match(h2_t hash) const { uint8x8_t dup = vdup_n_u8(hash); auto mask = vceq_u8(ctrl, dup); - return BitMask( + return BitMask( vget_lane_u64(vreinterpret_u64_u8(mask), 0)); } @@ -704,12 +716,13 @@ struct GroupAArch64Impl { // Returns a bitmask representing the positions of full slots. // Note: for `is_small()` tables group may contain the "same" slot twice: // original and mirrored. - BitMask MaskFull() const { + auto MaskFull() const { uint64_t mask = vget_lane_u64( vreinterpret_u64_u8(vcge_s8(vreinterpret_s8_u8(ctrl), vdup_n_s8(static_cast(0)))), 0); - return BitMask(mask); + return BitMask(mask); } NonIterableBitMask MaskEmptyOrDeleted() const { @@ -736,11 +749,10 @@ struct GroupAArch64Impl { void ConvertSpecialToEmptyAndFullToDeleted(ctrl_t* dst) const { uint64_t mask = vget_lane_u64(vreinterpret_u64_u8(ctrl), 0); - constexpr uint64_t msbs = 0x8080808080808080ULL; constexpr uint64_t slsbs = 0x0202020202020202ULL; constexpr uint64_t midbs = 0x7e7e7e7e7e7e7e7eULL; auto x = slsbs & (mask >> 6); - auto res = (x + midbs) | msbs; + auto res = (x + midbs) | kMsbs8Bytes; little_endian::Store64(dst, res); } @@ -768,30 +780,26 @@ struct GroupPortableImpl { // v = 0x1716151413121110 // hash = 0x12 // retval = (v - lsbs) & ~v & msbs = 0x0000000080800000 - constexpr uint64_t msbs = 0x8080808080808080ULL; constexpr uint64_t lsbs = 0x0101010101010101ULL; auto x = ctrl ^ (lsbs * hash); - return BitMask((x - lsbs) & ~x & msbs); + return BitMask((x - lsbs) & ~x & kMsbs8Bytes); } NonIterableBitMask MaskEmpty() const { - constexpr uint64_t msbs = 0x8080808080808080ULL; return NonIterableBitMask((ctrl & ~(ctrl << 6)) & - msbs); + kMsbs8Bytes); } // Returns a bitmask representing the positions of full slots. // Note: for `is_small()` tables group may contain the "same" slot twice: // original and mirrored. BitMask MaskFull() const { - constexpr uint64_t msbs = 0x8080808080808080ULL; - return BitMask((ctrl ^ msbs) & msbs); + return BitMask((ctrl ^ kMsbs8Bytes) & kMsbs8Bytes); } NonIterableBitMask MaskEmptyOrDeleted() const { - constexpr uint64_t msbs = 0x8080808080808080ULL; return NonIterableBitMask((ctrl & ~(ctrl << 7)) & - msbs); + kMsbs8Bytes); } uint32_t CountLeadingEmptyOrDeleted() const { @@ -803,9 +811,8 @@ struct GroupPortableImpl { } void ConvertSpecialToEmptyAndFullToDeleted(ctrl_t* dst) const { - constexpr uint64_t msbs = 0x8080808080808080ULL; constexpr uint64_t lsbs = 0x0101010101010101ULL; - auto x = ctrl & msbs; + auto x = ctrl & kMsbs8Bytes; auto res = (~x + (x >> 7)) & ~lsbs; little_endian::Store64(dst, res); } diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index f9797f56..6baaa060 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -15,6 +15,7 @@ #include "absl/container/internal/raw_hash_set.h" #include +#include #include #include #include @@ -75,6 +76,7 @@ struct RawHashSetTestOnlyAccess { namespace { using ::testing::ElementsAre; +using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::Ge; using ::testing::Lt; @@ -156,20 +158,66 @@ TEST(BitMask, Smoke) { EXPECT_THAT((BitMask(0xAA)), ElementsAre(1, 3, 5, 7)); } -TEST(BitMask, WithShift) { +TEST(BitMask, WithShift_MatchPortable) { // See the non-SSE version of Group for details on what this math is for. uint64_t ctrl = 0x1716151413121110; uint64_t hash = 0x12; - constexpr uint64_t msbs = 0x8080808080808080ULL; constexpr uint64_t lsbs = 0x0101010101010101ULL; auto x = ctrl ^ (lsbs * hash); - uint64_t mask = (x - lsbs) & ~x & msbs; + uint64_t mask = (x - lsbs) & ~x & kMsbs8Bytes; EXPECT_EQ(0x0000000080800000, mask); BitMask b(mask); EXPECT_EQ(*b, 2); } +constexpr uint64_t kSome8BytesMask = /* */ 0x8000808080008000ULL; +constexpr uint64_t kSome8BytesMaskAllOnes = 0xff00ffffff00ff00ULL; +constexpr auto kSome8BytesMaskBits = std::array{1, 3, 4, 5, 7}; + + +TEST(BitMask, WithShift_FullMask) { + EXPECT_THAT((BitMask(kMsbs8Bytes)), + ElementsAre(0, 1, 2, 3, 4, 5, 6, 7)); + EXPECT_THAT( + (BitMask(kMsbs8Bytes)), + ElementsAre(0, 1, 2, 3, 4, 5, 6, 7)); + EXPECT_THAT( + (BitMask(~uint64_t{0})), + ElementsAre(0, 1, 2, 3, 4, 5, 6, 7)); +} + +TEST(BitMask, WithShift_EmptyMask) { + EXPECT_THAT((BitMask(0)), ElementsAre()); + EXPECT_THAT((BitMask(0)), + ElementsAre()); +} + +TEST(BitMask, WithShift_SomeMask) { + EXPECT_THAT((BitMask(kSome8BytesMask)), + ElementsAreArray(kSome8BytesMaskBits)); + EXPECT_THAT((BitMask( + kSome8BytesMask)), + ElementsAreArray(kSome8BytesMaskBits)); + EXPECT_THAT((BitMask( + kSome8BytesMaskAllOnes)), + ElementsAreArray(kSome8BytesMaskBits)); +} + +TEST(BitMask, WithShift_SomeMaskExtraBitsForNullify) { + // Verify that adding extra bits into non zero bytes is fine. + uint64_t extra_bits = 77; + for (int i = 0; i < 100; ++i) { + // Add extra bits, but keep zero bytes untouched. + uint64_t extra_mask = extra_bits & kSome8BytesMaskAllOnes; + EXPECT_THAT((BitMask( + kSome8BytesMask | extra_mask)), + ElementsAreArray(kSome8BytesMaskBits)) + << i << " " << extra_mask; + extra_bits = (extra_bits + 1) * 3; + } +} + TEST(BitMask, LeadingTrailing) { EXPECT_EQ((BitMask(0x00001a40).LeadingZeros()), 3); EXPECT_EQ((BitMask(0x00001a40).TrailingZeros()), 6); -- cgit v1.2.3 From 780bfc194d807dbd56363635ca40bf96743aa00b Mon Sep 17 00:00:00 2001 From: Shahriar Rouf Date: Wed, 31 Jan 2024 10:07:48 -0800 Subject: Replace `testonly = 1` with `testonly = True` in abseil BUILD files. https://bazel.build/build/style-guide#other-conventions PiperOrigin-RevId: 603084345 Change-Id: Ibd7c9573d820f88059d12c46ff82d7d322d002ae --- absl/base/BUILD.bazel | 18 +++++++++--------- absl/container/BUILD.bazel | 40 ++++++++++++++++++++-------------------- absl/crc/BUILD.bazel | 2 +- absl/flags/BUILD.bazel | 2 +- absl/hash/BUILD.bazel | 6 +++--- absl/log/internal/BUILD.bazel | 2 +- absl/numeric/BUILD.bazel | 2 +- absl/profiling/BUILD.bazel | 2 +- absl/random/BUILD.bazel | 6 +++--- absl/random/internal/BUILD.bazel | 8 ++++---- absl/strings/BUILD.bazel | 8 ++++---- absl/synchronization/BUILD.bazel | 10 +++++----- absl/time/BUILD.bazel | 2 +- 13 files changed, 54 insertions(+), 54 deletions(-) diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index 0eb735da..1eb8f098 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -294,7 +294,7 @@ cc_library( cc_library( name = "atomic_hook_test_helper", - testonly = 1, + testonly = True, srcs = ["internal/atomic_hook_test_helper.cc"], hdrs = ["internal/atomic_hook_test_helper.h"], copts = ABSL_DEFAULT_COPTS, @@ -380,7 +380,7 @@ cc_test( cc_library( name = "exception_testing", - testonly = 1, + testonly = True, hdrs = ["internal/exception_testing.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -404,7 +404,7 @@ cc_library( cc_library( name = "exception_safety_testing", - testonly = 1, + testonly = True, srcs = ["internal/exception_safety_testing.cc"], hdrs = ["internal/exception_safety_testing.h"], copts = ABSL_TEST_COPTS, @@ -470,7 +470,7 @@ cc_test( # AbslInternalSpinLockDelay and AbslInternalSpinLockWake. cc_library( name = "spinlock_test_common", - testonly = 1, + testonly = True, srcs = ["spinlock_test_common.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -507,7 +507,7 @@ cc_test( cc_library( name = "spinlock_benchmark_common", - testonly = 1, + testonly = True, srcs = ["internal/spinlock_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -527,7 +527,7 @@ cc_library( cc_binary( name = "spinlock_benchmark", - testonly = 1, + testonly = True, copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, tags = ["benchmark"], @@ -608,7 +608,7 @@ cc_test( cc_binary( name = "no_destructor_benchmark", - testonly = 1, + testonly = True, srcs = ["no_destructor_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -710,7 +710,7 @@ cc_test( cc_library( name = "scoped_set_env", - testonly = 1, + testonly = True, srcs = ["internal/scoped_set_env.cc"], hdrs = ["internal/scoped_set_env.h"], linkopts = ABSL_DEFAULT_LINKOPTS, @@ -784,7 +784,7 @@ cc_test( cc_binary( name = "strerror_benchmark", - testonly = 1, + testonly = True, srcs = ["internal/strerror_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index bc5b2201..91633948 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -108,7 +108,7 @@ cc_test( cc_binary( name = "fixed_array_benchmark", - testonly = 1, + testonly = True, srcs = ["fixed_array_benchmark.cc"], copts = ABSL_TEST_COPTS + ["$(STACK_FRAME_UNLIMITED)"], linkopts = ABSL_DEFAULT_LINKOPTS, @@ -151,7 +151,7 @@ cc_library( cc_library( name = "test_allocator", - testonly = 1, + testonly = True, copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, textual_hdrs = ["internal/test_allocator.h"], @@ -181,7 +181,7 @@ cc_test( cc_binary( name = "inlined_vector_benchmark", - testonly = 1, + testonly = True, srcs = ["inlined_vector_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -210,7 +210,7 @@ cc_test( cc_library( name = "test_instance_tracker", - testonly = 1, + testonly = True, srcs = ["internal/test_instance_tracker.cc"], hdrs = ["internal/test_instance_tracker.h"], copts = ABSL_DEFAULT_COPTS, @@ -449,7 +449,7 @@ cc_test( cc_library( name = "hash_generator_testing", - testonly = 1, + testonly = True, srcs = ["internal/hash_generator_testing.cc"], hdrs = ["internal/hash_generator_testing.h"], copts = ABSL_TEST_COPTS, @@ -465,7 +465,7 @@ cc_library( cc_library( name = "hash_policy_testing", - testonly = 1, + testonly = True, hdrs = ["internal/hash_policy_testing.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -707,7 +707,7 @@ cc_test( cc_binary( name = "raw_hash_set_benchmark", - testonly = 1, + testonly = True, srcs = ["internal/raw_hash_set_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -724,7 +724,7 @@ cc_binary( cc_binary( name = "raw_hash_set_probe_benchmark", - testonly = 1, + testonly = True, srcs = ["internal/raw_hash_set_probe_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = select({ @@ -798,7 +798,7 @@ cc_test( cc_binary( name = "layout_benchmark", - testonly = 1, + testonly = True, srcs = ["internal/layout_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -814,7 +814,7 @@ cc_binary( cc_library( name = "tracked", - testonly = 1, + testonly = True, hdrs = ["internal/tracked.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -825,7 +825,7 @@ cc_library( cc_library( name = "unordered_map_constructor_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_map_constructor_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -838,7 +838,7 @@ cc_library( cc_library( name = "unordered_map_lookup_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_map_lookup_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -851,7 +851,7 @@ cc_library( cc_library( name = "unordered_map_modifiers_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_map_modifiers_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -864,7 +864,7 @@ cc_library( cc_library( name = "unordered_set_constructor_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_set_constructor_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -878,7 +878,7 @@ cc_library( cc_library( name = "unordered_set_members_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_set_members_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -890,7 +890,7 @@ cc_library( cc_library( name = "unordered_map_members_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_map_members_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -902,7 +902,7 @@ cc_library( cc_library( name = "unordered_set_lookup_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_set_lookup_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -915,7 +915,7 @@ cc_library( cc_library( name = "unordered_set_modifiers_test", - testonly = 1, + testonly = True, hdrs = ["internal/unordered_set_modifiers_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -1008,7 +1008,7 @@ cc_library( cc_library( name = "btree_test_common", - testonly = 1, + testonly = True, hdrs = ["btree_test.h"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -1059,7 +1059,7 @@ cc_test( cc_binary( name = "btree_benchmark", - testonly = 1, + testonly = True, srcs = [ "btree_benchmark.cc", ], diff --git a/absl/crc/BUILD.bazel b/absl/crc/BUILD.bazel index d923aec4..9dc81819 100644 --- a/absl/crc/BUILD.bazel +++ b/absl/crc/BUILD.bazel @@ -203,7 +203,7 @@ cc_test( cc_binary( name = "crc32c_benchmark", - testonly = 1, + testonly = True, srcs = ["crc32c_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index d3b06227..657f8d2c 100644 --- a/absl/flags/BUILD.bazel +++ b/absl/flags/BUILD.bazel @@ -404,7 +404,7 @@ cc_test( cc_binary( name = "flag_benchmark", - testonly = 1, + testonly = True, srcs = [ "flag_benchmark.cc", ], diff --git a/absl/hash/BUILD.bazel b/absl/hash/BUILD.bazel index 1e8ad451..fe567e91 100644 --- a/absl/hash/BUILD.bazel +++ b/absl/hash/BUILD.bazel @@ -61,7 +61,7 @@ cc_library( cc_library( name = "hash_testing", - testonly = 1, + testonly = True, hdrs = ["hash_testing.h"], linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ @@ -128,7 +128,7 @@ cc_test( cc_binary( name = "hash_benchmark", - testonly = 1, + testonly = True, srcs = ["hash_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -148,7 +148,7 @@ cc_binary( cc_library( name = "spy_hash_state", - testonly = 1, + testonly = True, hdrs = ["internal/spy_hash_state.h"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/log/internal/BUILD.bazel b/absl/log/internal/BUILD.bazel index 1be13499..2a8c1a47 100644 --- a/absl/log/internal/BUILD.bazel +++ b/absl/log/internal/BUILD.bazel @@ -400,7 +400,7 @@ cc_library( cc_binary( name = "vlog_config_benchmark", - testonly = 1, + testonly = True, srcs = ["vlog_config_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/numeric/BUILD.bazel b/absl/numeric/BUILD.bazel index db02b9c0..41c015db 100644 --- a/absl/numeric/BUILD.bazel +++ b/absl/numeric/BUILD.bazel @@ -46,7 +46,7 @@ cc_library( cc_binary( name = "bits_benchmark", - testonly = 1, + testonly = True, srcs = ["bits_benchmark.cc"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/profiling/BUILD.bazel b/absl/profiling/BUILD.bazel index 86f205f9..abe127ec 100644 --- a/absl/profiling/BUILD.bazel +++ b/absl/profiling/BUILD.bazel @@ -126,7 +126,7 @@ cc_test( cc_binary( name = "periodic_sampler_benchmark", - testonly = 1, + testonly = True, srcs = ["internal/periodic_sampler_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/random/BUILD.bazel b/absl/random/BUILD.bazel index 80c4f055..bec979d6 100644 --- a/absl/random/BUILD.bazel +++ b/absl/random/BUILD.bazel @@ -132,7 +132,7 @@ cc_library( cc_library( name = "mock_distributions", - testonly = 1, + testonly = True, hdrs = ["mock_distributions.h"], linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ @@ -146,7 +146,7 @@ cc_library( cc_library( name = "mocking_bit_gen", - testonly = 1, + testonly = True, hdrs = [ "mocking_bit_gen.h", ], @@ -521,7 +521,7 @@ cc_test( # Benchmarks for various methods / test utilities cc_binary( name = "benchmarks", - testonly = 1, + testonly = True, srcs = [ "benchmarks.cc", ], diff --git a/absl/random/internal/BUILD.bazel b/absl/random/internal/BUILD.bazel index 71a742ee..69fb5f2b 100644 --- a/absl/random/internal/BUILD.bazel +++ b/absl/random/internal/BUILD.bazel @@ -137,7 +137,7 @@ cc_library( cc_library( name = "explicit_seed_seq", - testonly = 1, + testonly = True, hdrs = [ "explicit_seed_seq.h", ], @@ -151,7 +151,7 @@ cc_library( cc_library( name = "sequence_urbg", - testonly = 1, + testonly = True, hdrs = [ "sequence_urbg.h", ], @@ -375,7 +375,7 @@ cc_binary( cc_library( name = "distribution_test_util", - testonly = 1, + testonly = True, srcs = [ "chi_square.cc", "distribution_test_util.cc", @@ -534,7 +534,7 @@ cc_library( cc_library( name = "mock_overload_set", - testonly = 1, + testonly = True, hdrs = ["mock_overload_set.h"], linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 1b6c7e4b..942c5324 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -829,7 +829,7 @@ cc_test( cc_library( name = "cord_test_helpers", - testonly = 1, + testonly = True, hdrs = [ "cord_test_helpers.h", ], @@ -845,7 +845,7 @@ cc_library( cc_library( name = "cord_rep_test_util", - testonly = 1, + testonly = True, hdrs = ["internal/cord_rep_test_util.h"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -859,7 +859,7 @@ cc_library( cc_library( name = "cordz_test_helpers", - testonly = 1, + testonly = True, hdrs = ["cordz_test_helpers.h"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -1449,7 +1449,7 @@ cc_test( cc_binary( name = "atod_manual_test", - testonly = 1, + testonly = True, srcs = ["atod_manual_test.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/synchronization/BUILD.bazel b/absl/synchronization/BUILD.bazel index de06ebdd..f747d14a 100644 --- a/absl/synchronization/BUILD.bazel +++ b/absl/synchronization/BUILD.bazel @@ -183,7 +183,7 @@ cc_test( cc_binary( name = "blocking_counter_benchmark", - testonly = 1, + testonly = True, srcs = ["blocking_counter_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -230,7 +230,7 @@ cc_test( cc_library( name = "thread_pool", - testonly = 1, + testonly = True, hdrs = ["internal/thread_pool.h"], linkopts = ABSL_DEFAULT_LINKOPTS, visibility = [ @@ -281,7 +281,7 @@ cc_test( cc_library( name = "mutex_benchmark_common", - testonly = 1, + testonly = True, srcs = ["mutex_benchmark.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -300,7 +300,7 @@ cc_library( cc_binary( name = "mutex_benchmark", - testonly = 1, + testonly = True, copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ @@ -326,7 +326,7 @@ cc_test( cc_library( name = "per_thread_sem_test_common", - testonly = 1, + testonly = True, srcs = ["internal/per_thread_sem_test.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/time/BUILD.bazel b/absl/time/BUILD.bazel index e3fe705b..05f1f2f2 100644 --- a/absl/time/BUILD.bazel +++ b/absl/time/BUILD.bazel @@ -65,7 +65,7 @@ cc_library( cc_library( name = "test_util", - testonly = 1, + testonly = True, srcs = ["internal/test_util.cc"], hdrs = ["internal/test_util.h"], copts = ABSL_DEFAULT_COPTS, -- cgit v1.2.3 From 4c7e7c7d94eaaa3bff3142c257d880a468a35934 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 31 Jan 2024 13:45:52 -0800 Subject: Type erased hash_slot_fn that depends only on key types (and hash function). PiperOrigin-RevId: 603148301 Change-Id: Ie2e5702995c9e1ef4d5aaab23bc89a1eb5007a86 --- absl/container/BUILD.bazel | 4 ++ absl/container/CMakeLists.txt | 3 + absl/container/flat_hash_map.h | 7 +++ absl/container/flat_hash_set.h | 6 ++ absl/container/internal/container_memory.h | 20 +++++++ absl/container/internal/container_memory_test.cc | 14 +++++ absl/container/internal/hash_policy_traits.h | 35 ++++++++++++ absl/container/internal/hash_policy_traits_test.cc | 64 ++++++++++++++++++++++ absl/container/internal/raw_hash_set.cc | 4 +- absl/container/internal/raw_hash_set.h | 15 ++--- .../internal/raw_hash_set_allocator_test.cc | 8 ++- absl/container/internal/raw_hash_set_benchmark.cc | 11 ++++ .../internal/raw_hash_set_probe_benchmark.cc | 5 ++ absl/container/internal/raw_hash_set_test.cc | 19 ++++++- absl/container/node_hash_map.h | 8 +++ absl/container/node_hash_set.h | 7 +++ 16 files changed, 214 insertions(+), 16 deletions(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 91633948..5160ccea 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -357,6 +357,7 @@ cc_library( copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ + ":container_memory", ":hash_function_defaults", ":node_slot_policy", ":raw_hash_set", @@ -504,6 +505,7 @@ cc_test( copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ + ":container_memory", ":hash_policy_traits", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", @@ -714,6 +716,7 @@ cc_binary( tags = ["benchmark"], visibility = ["//visibility:private"], deps = [ + ":container_memory", ":hash_function_defaults", ":raw_hash_set", "//absl/base:raw_logging_internal", @@ -753,6 +756,7 @@ cc_test( copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ + ":container_memory", ":raw_hash_set", ":tracked", "//absl/base:config", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 3f80d29b..85af0bd7 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -401,6 +401,7 @@ absl_cc_library( COPTS ${ABSL_DEFAULT_COPTS} DEPS + absl::container_memory absl::core_headers absl::hash_function_defaults absl::node_slot_policy @@ -560,6 +561,7 @@ absl_cc_test( COPTS ${ABSL_TEST_COPTS} DEPS + absl::container_memory absl::hash_policy_traits GTest::gmock_main ) @@ -776,6 +778,7 @@ absl_cc_test( ${ABSL_TEST_COPTS} DEPS absl::config + absl::container_memory absl::raw_hash_set absl::tracked GTest::gmock_main diff --git a/absl/container/flat_hash_map.h b/absl/container/flat_hash_map.h index 808a62ba..2a1ad0fb 100644 --- a/absl/container/flat_hash_map.h +++ b/absl/container/flat_hash_map.h @@ -593,6 +593,13 @@ struct FlatHashMapPolicy { std::forward(args)...); } + template + static constexpr HashSlotFn get_hash_slot_fn() { + return memory_internal::IsLayoutCompatible::value + ? &TypeErasedApplyToSlotFn + : nullptr; + } + static size_t space_used(const slot_type*) { return 0; } static std::pair& element(slot_type* slot) { return slot->value; } diff --git a/absl/container/flat_hash_set.h b/absl/container/flat_hash_set.h index cd74923c..1be00195 100644 --- a/absl/container/flat_hash_set.h +++ b/absl/container/flat_hash_set.h @@ -29,6 +29,7 @@ #ifndef ABSL_CONTAINER_FLAT_HASH_SET_H_ #define ABSL_CONTAINER_FLAT_HASH_SET_H_ +#include #include #include #include @@ -492,6 +493,11 @@ struct FlatHashSetPolicy { } static size_t space_used(const T*) { return 0; } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return &TypeErasedApplyToSlotFn; + } }; } // namespace container_internal diff --git a/absl/container/internal/container_memory.h b/absl/container/internal/container_memory.h index d86d7b80..ba8e08a2 100644 --- a/absl/container/internal/container_memory.h +++ b/absl/container/internal/container_memory.h @@ -464,6 +464,26 @@ struct map_slot_policy { } }; +// Type erased function for computing hash of the slot. +using HashSlotFn = size_t (*)(const void* hash_fn, void* slot); + +// Type erased function to apply `Fn` to data inside of the `slot`. +// The data is expected to have type `T`. +template +size_t TypeErasedApplyToSlotFn(const void* fn, void* slot) { + const auto* f = static_cast(fn); + return (*f)(*static_cast(slot)); +} + +// Type erased function to apply `Fn` to data inside of the `*slot_ptr`. +// The data is expected to have type `T`. +template +size_t TypeErasedDerefAndApplyToSlotFn(const void* fn, void* slot_ptr) { + const auto* f = static_cast(fn); + const T* slot = *static_cast(slot_ptr); + return (*f)(*slot); +} + } // namespace container_internal ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/container/internal/container_memory_test.cc b/absl/container/internal/container_memory_test.cc index c973ac25..7e4357d5 100644 --- a/absl/container/internal/container_memory_test.cc +++ b/absl/container/internal/container_memory_test.cc @@ -298,6 +298,20 @@ TEST(MapSlotPolicy, DestroyReturnsTrue) { } } +TEST(ApplyTest, TypeErasedApplyToSlotFn) { + size_t x = 7; + auto fn = [](size_t v) { return v * 2; }; + EXPECT_EQ((TypeErasedApplyToSlotFn(&fn, &x)), 14); +} + +TEST(ApplyTest, TypeErasedDerefAndApplyToSlotFn) { + size_t x = 7; + auto fn = [](size_t v) { return v * 2; }; + size_t* x_ptr = &x; + EXPECT_EQ( + (TypeErasedDerefAndApplyToSlotFn(&fn, &x_ptr)), 14); +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/hash_policy_traits.h b/absl/container/internal/hash_policy_traits.h index 164ec123..86ffd1be 100644 --- a/absl/container/internal/hash_policy_traits.h +++ b/absl/container/internal/hash_policy_traits.h @@ -148,6 +148,41 @@ struct hash_policy_traits : common_policy_traits { static auto value(T* elem) -> decltype(P::value(elem)) { return P::value(elem); } + + using HashSlotFn = size_t (*)(const void* hash_fn, void* slot); + + template + static constexpr HashSlotFn get_hash_slot_fn() { +// get_hash_slot_fn may return nullptr to signal that non type erased function +// should be used. GCC warns against comparing function address with nullptr. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +// silent error: the address of * will never be NULL [-Werror=address] +#pragma GCC diagnostic ignored "-Waddress" +#endif + return Policy::template get_hash_slot_fn() == nullptr + ? &hash_slot_fn_non_type_erased + : Policy::template get_hash_slot_fn(); +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#endif + } + + private: + template + struct HashElement { + template + size_t operator()(const K& key, Args&&...) const { + return h(key); + } + const Hash& h; + }; + + template + static size_t hash_slot_fn_non_type_erased(const void* hash_fn, void* slot) { + return Policy::apply(HashElement{*static_cast(hash_fn)}, + Policy::element(static_cast(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 82d7cc3a..2d2c7c2c 100644 --- a/absl/container/internal/hash_policy_traits_test.cc +++ b/absl/container/internal/hash_policy_traits_test.cc @@ -14,12 +14,14 @@ #include "absl/container/internal/hash_policy_traits.h" +#include #include #include #include #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/container/internal/container_memory.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -42,6 +44,11 @@ struct PolicyWithoutOptionalOps { static int apply(int v) { return apply_impl(v); } static std::function apply_impl; static std::function value; + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; std::function PolicyWithoutOptionalOps::apply_impl; @@ -74,6 +81,63 @@ TEST_F(Test, value) { EXPECT_EQ(&b, &hash_policy_traits::value(&a)); } +struct Hash { + size_t operator()(Slot a) const { return static_cast(a) * 5; } +}; + +struct PolicyNoHashFn { + using slot_type = Slot; + using key_type = Slot; + using init_type = Slot; + + static size_t* apply_called_count; + + static Slot& element(Slot* slot) { return *slot; } + template + static size_t apply(const Fn& fn, int v) { + ++(*apply_called_count); + return fn(v); + } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } +}; + +size_t* PolicyNoHashFn::apply_called_count; + +struct PolicyCustomHashFn : PolicyNoHashFn { + template + static constexpr HashSlotFn get_hash_slot_fn() { + return &TypeErasedApplyToSlotFn; + } +}; + +TEST(HashTest, PolicyNoHashFn_get_hash_slot_fn) { + size_t apply_called_count = 0; + PolicyNoHashFn::apply_called_count = &apply_called_count; + + Hash hasher; + Slot value = 7; + auto* fn = hash_policy_traits::get_hash_slot_fn(); + EXPECT_NE(fn, nullptr); + EXPECT_EQ(fn(&hasher, &value), hasher(value)); + EXPECT_EQ(apply_called_count, 1); +} + +TEST(HashTest, PolicyCustomHashFn_get_hash_slot_fn) { + size_t apply_called_count = 0; + PolicyNoHashFn::apply_called_count = &apply_called_count; + + Hash hasher; + Slot value = 7; + auto* fn = hash_policy_traits::get_hash_slot_fn(); + EXPECT_EQ(fn, PolicyCustomHashFn::get_hash_slot_fn()); + EXPECT_EQ(fn(&hasher, &value), hasher(value)); + EXPECT_EQ(apply_called_count, 0); +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 9f8ea519..02301e19 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -140,7 +140,7 @@ static inline void* PrevSlot(void* slot, size_t slot_size) { return reinterpret_cast(reinterpret_cast(slot) - slot_size); } -void DropDeletesWithoutResize(CommonFields& common, +void DropDeletesWithoutResize(CommonFields& common, const void* hash_fn, const PolicyFunctions& policy, void* tmp_space) { void* set = &common; void* slot_array = common.slot_array(); @@ -175,7 +175,7 @@ void DropDeletesWithoutResize(CommonFields& common, ++i, slot_ptr = NextSlot(slot_ptr, slot_size)) { assert(slot_ptr == SlotAddress(slot_array, i, slot_size)); if (!IsDeleted(ctrl[i])) continue; - const size_t hash = (*hasher)(set, slot_ptr); + const size_t hash = (*hasher)(hash_fn, slot_ptr); const FindInfo target = find_first_non_full(common, hash); const size_t new_i = target.offset; total_probe_length += target.probe_length; diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index c464de6a..3b793825 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1802,7 +1802,7 @@ struct PolicyFunctions { size_t slot_size; // Returns the hash of the pointed-to slot. - size_t (*hash_slot)(void* set, void* slot); + size_t (*hash_slot)(const void* hash_fn, void* slot); // Transfer the contents of src_slot to dst_slot. void (*transfer)(void* set, void* dst_slot, void* src_slot); @@ -1847,7 +1847,7 @@ ABSL_ATTRIBUTE_NOINLINE void TransferRelocatable(void*, void* dst, void* src) { } // Type-erased version of raw_hash_set::drop_deletes_without_resize. -void DropDeletesWithoutResize(CommonFields& common, +void DropDeletesWithoutResize(CommonFields& common, const void* hash_fn, const PolicyFunctions& policy, void* tmp_space); // A SwissTable. @@ -2989,7 +2989,7 @@ class raw_hash_set { inline void drop_deletes_without_resize() { // Stack-allocate space for swapping elements. alignas(slot_type) unsigned char tmp[sizeof(slot_type)]; - DropDeletesWithoutResize(common(), GetPolicyFunctions(), tmp); + DropDeletesWithoutResize(common(), &hash_ref(), GetPolicyFunctions(), tmp); } // Called whenever the table *might* need to conditionally grow. @@ -3238,13 +3238,6 @@ class raw_hash_set { return settings_.template get<3>(); } - // Make type-specific functions for this type's PolicyFunctions struct. - static size_t hash_slot_fn(void* set, void* slot) { - auto* h = static_cast(set); - return PolicyTraits::apply( - HashElement{h->hash_ref()}, - PolicyTraits::element(static_cast(slot))); - } static void transfer_slot_fn(void* set, void* dst, void* src) { auto* h = static_cast(set); h->transfer(static_cast(dst), static_cast(src)); @@ -3266,7 +3259,7 @@ class raw_hash_set { static const PolicyFunctions& GetPolicyFunctions() { static constexpr PolicyFunctions value = { sizeof(slot_type), - &raw_hash_set::hash_slot_fn, + PolicyTraits::template get_hash_slot_fn(), PolicyTraits::transfer_uses_memcpy() ? TransferRelocatable : &raw_hash_set::transfer_slot_fn, diff --git a/absl/container/internal/raw_hash_set_allocator_test.cc b/absl/container/internal/raw_hash_set_allocator_test.cc index 05dcfaa6..7e7a5063 100644 --- a/absl/container/internal/raw_hash_set_allocator_test.cc +++ b/absl/container/internal/raw_hash_set_allocator_test.cc @@ -25,6 +25,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/base/config.h" +#include "absl/container/internal/container_memory.h" #include "absl/container/internal/raw_hash_set.h" #include "absl/container/internal/tracked.h" @@ -133,7 +134,7 @@ class CheckedAlloc { }; struct Identity { - int32_t operator()(int32_t v) const { return v; } + size_t operator()(int32_t v) const { return static_cast(v); } }; struct Policy { @@ -178,6 +179,11 @@ struct Policy { } static slot_type& element(slot_type* slot) { return *slot; } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; template diff --git a/absl/container/internal/raw_hash_set_benchmark.cc b/absl/container/internal/raw_hash_set_benchmark.cc index 05a06427..bc8184d4 100644 --- a/absl/container/internal/raw_hash_set_benchmark.cc +++ b/absl/container/internal/raw_hash_set_benchmark.cc @@ -24,6 +24,7 @@ #include #include "absl/base/internal/raw_logging.h" +#include "absl/container/internal/container_memory.h" #include "absl/container/internal/hash_function_defaults.h" #include "absl/container/internal/raw_hash_set.h" #include "absl/strings/str_format.h" @@ -59,6 +60,11 @@ struct IntPolicy { static auto apply(F&& f, int64_t x) -> decltype(std::forward(f)(x, x)) { return std::forward(f)(x, x); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; class StringPolicy { @@ -117,6 +123,11 @@ class StringPolicy { return apply_impl(std::forward(f), PairArgs(std::forward(args)...)); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; struct StringHash : container_internal::hash_default_hash { diff --git a/absl/container/internal/raw_hash_set_probe_benchmark.cc b/absl/container/internal/raw_hash_set_probe_benchmark.cc index 5d4184b2..8f36305d 100644 --- a/absl/container/internal/raw_hash_set_probe_benchmark.cc +++ b/absl/container/internal/raw_hash_set_probe_benchmark.cc @@ -70,6 +70,11 @@ struct Policy { -> decltype(std::forward(f)(arg, arg)) { return std::forward(f)(arg, arg); } + + template + static constexpr auto get_hash_slot_fn() { + return nullptr; + } }; absl::BitGen& GlobalBitGen() { diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 6baaa060..7ec72b22 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -405,6 +405,11 @@ struct ValuePolicy { return absl::container_internal::DecomposeValue( std::forward(f), std::forward(args)...); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; using IntPolicy = ValuePolicy; @@ -468,6 +473,11 @@ class StringPolicy { return apply_impl(std::forward(f), PairArgs(std::forward(args)...)); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; struct StringHash : absl::Hash { @@ -923,6 +933,11 @@ struct DecomposePolicy { static auto apply(F&& f, const T& x) -> decltype(std::forward(f)(x, x)) { return std::forward(f)(x, x); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return nullptr; + } }; template @@ -1083,7 +1098,7 @@ size_t MaxDensitySize(size_t n) { } struct Modulo1000Hash { - size_t operator()(int x) const { return x % 1000; } + size_t operator()(int64_t x) const { return static_cast(x) % 1000; } }; struct Modulo1000HashTable @@ -2664,7 +2679,7 @@ using RawHashSetAlloc = raw_hash_set, TEST(Table, AllocatorPropagation) { TestAllocPropagation(); } struct CountedHash { - size_t operator()(int value) const { + size_t operator()(int64_t value) const { ++count; return static_cast(value); } diff --git a/absl/container/node_hash_map.h b/absl/container/node_hash_map.h index a396de2e..72e78958 100644 --- a/absl/container/node_hash_map.h +++ b/absl/container/node_hash_map.h @@ -36,6 +36,7 @@ #ifndef ABSL_CONTAINER_NODE_HASH_MAP_H_ #define ABSL_CONTAINER_NODE_HASH_MAP_H_ +#include #include #include #include @@ -590,6 +591,13 @@ class NodeHashMapPolicy static Value& value(value_type* elem) { return elem->second; } static const Value& value(const value_type* elem) { return elem->second; } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return memory_internal::IsLayoutCompatible::value + ? &TypeErasedDerefAndApplyToSlotFn + : nullptr; + } }; } // namespace container_internal diff --git a/absl/container/node_hash_set.h b/absl/container/node_hash_set.h index 421ff460..ec9ab519 100644 --- a/absl/container/node_hash_set.h +++ b/absl/container/node_hash_set.h @@ -35,10 +35,12 @@ #ifndef ABSL_CONTAINER_NODE_HASH_SET_H_ #define ABSL_CONTAINER_NODE_HASH_SET_H_ +#include #include #include "absl/algorithm/container.h" #include "absl/base/macros.h" +#include "absl/container/internal/container_memory.h" #include "absl/container/internal/hash_function_defaults.h" // IWYU pragma: export #include "absl/container/internal/node_slot_policy.h" #include "absl/container/internal/raw_hash_set.h" // IWYU pragma: export @@ -487,6 +489,11 @@ struct NodeHashSetPolicy } static size_t element_space_used(const T*) { return sizeof(T); } + + template + static constexpr HashSlotFn get_hash_slot_fn() { + return &TypeErasedDerefAndApplyToSlotFn; + } }; } // namespace container_internal -- cgit v1.2.3 From 513a6f93997844a931c68506840a2ca9f5485753 Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Thu, 1 Feb 2024 05:37:01 -0800 Subject: Optimize `Cord::Swap()` for missed compiler optimization in clang. PiperOrigin-RevId: 603342563 Change-Id: I1cd80103377f457770d5178dad8b56ae459cbd55 --- absl/strings/cord.h | 3 ++- absl/strings/internal/cord_internal.h | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/absl/strings/cord.h b/absl/strings/cord.h index b3e556b6..d2ba9673 100644 --- a/absl/strings/cord.h +++ b/absl/strings/cord.h @@ -1170,7 +1170,8 @@ inline void Cord::InlineRep::Swap(absl::Nonnull rhs) { if (rhs == this) { return; } - std::swap(data_, rhs->data_); + using std::swap; + swap(data_, rhs->data_); } inline absl::Nullable Cord::InlineRep::data() const { diff --git a/absl/strings/internal/cord_internal.h b/absl/strings/internal/cord_internal.h index 8744540e..549f9175 100644 --- a/absl/strings/internal/cord_internal.h +++ b/absl/strings/internal/cord_internal.h @@ -520,6 +520,7 @@ class InlineData { constexpr InlineData(const InlineData& rhs) noexcept; InlineData& operator=(const InlineData& rhs) noexcept; + friend void swap(InlineData& lhs, InlineData& rhs) noexcept; friend bool operator==(const InlineData& lhs, const InlineData& rhs) { #ifdef ABSL_INTERNAL_CORD_HAVE_SANITIZER @@ -770,6 +771,12 @@ class InlineData { char data[kMaxInline + 1]; AsTree as_tree; }; + + // TODO(b/145829486): see swap(InlineData, InlineData) for more info. + inline void SwapValue(Rep rhs, Rep& refrhs) { + memcpy(&refrhs, this, sizeof(*this)); + memcpy(this, &rhs, sizeof(*this)); + } }; // Private implementation of `Compare()` @@ -884,6 +891,19 @@ inline void CordRep::Unref(CordRep* rep) { } } +inline void swap(InlineData& lhs, InlineData& rhs) noexcept { + lhs.unpoison(); + rhs.unpoison(); + // TODO(b/145829486): `std::swap(lhs.rep_, rhs.rep_)` results in bad codegen + // on clang, spilling the temporary swap value on the stack. Since `Rep` is + // trivial, we can make clang DTRT by calling a hand-rolled `SwapValue` where + // we pass `rhs` both by value (register allocated) and by reference. The IR + // then folds and inlines correctly into an optimized swap without spill. + lhs.rep_.SwapValue(rhs.rep_, rhs.rep_); + rhs.poison(); + lhs.poison(); +} + } // namespace cord_internal ABSL_NAMESPACE_END -- cgit v1.2.3 From a3ee6ce2e6eee06941a1e9479720cecbd898f4ca Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 1 Feb 2024 05:47:35 -0800 Subject: Add ABSL_ATTRIBUTE_UNINITIALIZED macros for use with clang and GCC's `uninitialized` PiperOrigin-RevId: 603344396 Change-Id: I06721246bf205284843065687378c6d905cbf879 --- absl/base/attributes.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/absl/base/attributes.h b/absl/base/attributes.h index d4f67a12..373b6d77 100644 --- a/absl/base/attributes.h +++ b/absl/base/attributes.h @@ -871,4 +871,23 @@ #define ABSL_ATTRIBUTE_NO_UNIQUE_ADDRESS #endif +// ABSL_ATTRIBUTE_UNINITIALIZED +// +// GCC and Clang support a flag `-ftrivial-auto-var-init=