diff options
-rw-r--r-- | absl/container/internal/btree.h | 39 | ||||
-rw-r--r-- | absl/functional/BUILD.bazel | 2 | ||||
-rw-r--r-- | absl/functional/CMakeLists.txt | 1 | ||||
-rw-r--r-- | absl/functional/function_ref_test.cc | 20 | ||||
-rw-r--r-- | absl/functional/internal/function_ref.h | 7 | ||||
-rw-r--r-- | absl/hash/internal/hash.h | 2 | ||||
-rw-r--r-- | absl/log/internal/conditions.h | 6 | ||||
-rw-r--r-- | absl/strings/escaping.cc | 5 | ||||
-rw-r--r-- | absl/strings/internal/charconv_bigint.cc | 6 | ||||
-rw-r--r-- | absl/synchronization/internal/pthread_waiter.cc | 2 | ||||
-rw-r--r-- | absl/synchronization/internal/sem_waiter.cc | 2 | ||||
-rw-r--r-- | absl/synchronization/internal/waiter_test.cc | 12 | ||||
-rw-r--r-- | absl/synchronization/mutex.h | 20 | ||||
-rw-r--r-- | absl/synchronization/mutex_test.cc | 105 |
14 files changed, 199 insertions, 30 deletions
diff --git a/absl/container/internal/btree.h b/absl/container/internal/btree.h index 2ed57a62..d19b1ee6 100644 --- a/absl/container/internal/btree.h +++ b/absl/container/internal/btree.h @@ -763,9 +763,12 @@ class btree_node { void clear_child(field_type i) { absl::container_internal::SanitizerPoisonObject(&mutable_child(i)); } - void set_child(field_type i, btree_node *c) { + void set_child_noupdate_position(field_type i, btree_node *c) { absl::container_internal::SanitizerUnpoisonObject(&mutable_child(i)); mutable_child(i) = c; + } + void set_child(field_type i, btree_node *c) { + set_child_noupdate_position(i, c); c->set_position(i); } void init_child(field_type i, btree_node *c) { @@ -948,18 +951,19 @@ class btree_node { void merge(btree_node *src, allocator_type *alloc); // Node allocation/deletion routines. - void init_leaf(field_type max_count, btree_node *parent) { + void init_leaf(field_type position, field_type max_count, + btree_node *parent) { set_generation(0); set_parent(parent); - set_position(0); + set_position(position); set_start(0); set_finish(0); set_max_count(max_count); absl::container_internal::SanitizerPoisonMemoryRegion( start_slot(), max_count * sizeof(slot_type)); } - void init_internal(btree_node *parent) { - init_leaf(kNodeSlots, parent); + void init_internal(field_type position, btree_node *parent) { + init_leaf(position, kNodeSlots, parent); // Set `max_count` to a sentinel value to indicate that this node is // internal. set_max_count(kInternalNodeMaxCount); @@ -1695,19 +1699,19 @@ class btree { } // Node creation/deletion routines. - node_type *new_internal_node(node_type *parent) { + node_type *new_internal_node(field_type position, node_type *parent) { node_type *n = allocate(node_type::InternalSize()); - n->init_internal(parent); + n->init_internal(position, parent); return n; } - node_type *new_leaf_node(node_type *parent) { + node_type *new_leaf_node(field_type position, node_type *parent) { node_type *n = allocate(node_type::LeafSize()); - n->init_leaf(kNodeSlots, parent); + n->init_leaf(position, kNodeSlots, parent); return n; } node_type *new_leaf_root_node(field_type max_count) { node_type *n = allocate(node_type::LeafSize(max_count)); - n->init_leaf(max_count, /*parent=*/n); + n->init_leaf(/*position=*/0, max_count, /*parent=*/n); return n; } @@ -1946,6 +1950,8 @@ void btree_node<P>::split(const int insert_position, btree_node *dest, allocator_type *alloc) { assert(dest->count() == 0); assert(max_count() == kNodeSlots); + assert(position() + 1 == dest->position()); + assert(parent() == dest->parent()); // We bias the split based on the position being inserted. If we're // inserting at the beginning of the left node then bias the split to put @@ -1968,7 +1974,7 @@ void btree_node<P>::split(const int insert_position, btree_node *dest, --mutable_finish(); parent()->emplace_value(position(), alloc, finish_slot()); value_destroy(finish(), alloc); - parent()->init_child(position() + 1, dest); + parent()->set_child_noupdate_position(position() + 1, dest); if (is_internal()) { for (field_type i = dest->start(), j = finish() + 1; i <= dest->finish(); @@ -2685,16 +2691,17 @@ void btree<P>::rebalance_or_split(iterator *iter) { // value. assert(parent->max_count() == kNodeSlots); if (parent->count() == kNodeSlots) { - iterator parent_iter(node->parent(), node->position()); + iterator parent_iter(parent, node->position()); rebalance_or_split(&parent_iter); + parent = node->parent(); } } else { // Rebalancing not possible because this is the root node. // Create a new root node and set the current root node as the child of the // new root. - parent = new_internal_node(parent); + parent = new_internal_node(/*position=*/0, parent); parent->set_generation(root()->generation()); - parent->init_child(parent->start(), root()); + parent->init_child(parent->start(), node); mutable_root() = parent; // If the former root was a leaf node, then it's now the rightmost node. assert(parent->start_child()->is_internal() || @@ -2704,11 +2711,11 @@ void btree<P>::rebalance_or_split(iterator *iter) { // Split the node. node_type *split_node; if (node->is_leaf()) { - split_node = new_leaf_node(parent); + split_node = new_leaf_node(node->position() + 1, parent); node->split(insert_position, split_node, mutable_allocator()); if (rightmost() == node) mutable_rightmost() = split_node; } else { - split_node = new_internal_node(parent); + split_node = new_internal_node(node->position() + 1, parent); node->split(insert_position, split_node, mutable_allocator()); } diff --git a/absl/functional/BUILD.bazel b/absl/functional/BUILD.bazel index c4fbce98..4ceac539 100644 --- a/absl/functional/BUILD.bazel +++ b/absl/functional/BUILD.bazel @@ -92,6 +92,7 @@ cc_library( copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ + ":any_invocable", "//absl/base:base_internal", "//absl/base:core_headers", "//absl/meta:type_traits", @@ -104,6 +105,7 @@ cc_test( srcs = ["function_ref_test.cc"], copts = ABSL_TEST_COPTS, deps = [ + ":any_invocable", ":function_ref", "//absl/container:test_instance_tracker", "//absl/memory", diff --git a/absl/functional/CMakeLists.txt b/absl/functional/CMakeLists.txt index c0f6eaaa..628b2ffc 100644 --- a/absl/functional/CMakeLists.txt +++ b/absl/functional/CMakeLists.txt @@ -90,6 +90,7 @@ absl_cc_library( DEPS absl::base_internal absl::core_headers + absl::any_invocable absl::meta PUBLIC ) diff --git a/absl/functional/function_ref_test.cc b/absl/functional/function_ref_test.cc index d0923fdd..f91e15e0 100644 --- a/absl/functional/function_ref_test.cc +++ b/absl/functional/function_ref_test.cc @@ -20,6 +20,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/container/internal/test_instance_tracker.h" +#include "absl/functional/any_invocable.h" #include "absl/memory/memory.h" namespace absl { @@ -157,6 +158,25 @@ TEST(FunctionRef, NullMemberPtrAssertFails) { EXPECT_DEBUG_DEATH({ FunctionRef<int(const S& s)> ref(mem_ptr); }, ""); } +TEST(FunctionRef, NullStdFunctionAssertPasses) { + std::function<void()> function = []() {}; + FunctionRef<void()> ref(function); +} + +TEST(FunctionRef, NullStdFunctionAssertFails) { + std::function<void()> function = nullptr; + EXPECT_DEBUG_DEATH({ FunctionRef<void()> ref(function); }, ""); +} + +TEST(FunctionRef, NullAnyInvocableAssertPasses) { + AnyInvocable<void() const> invocable = []() {}; + FunctionRef<void()> ref(invocable); +} +TEST(FunctionRef, NullAnyInvocableAssertFails) { + AnyInvocable<void() const> invocable = nullptr; + EXPECT_DEBUG_DEATH({ FunctionRef<void()> ref(invocable); }, ""); +} + #endif // GTEST_HAS_DEATH_TEST TEST(FunctionRef, CopiesAndMovesPerPassByValue) { diff --git a/absl/functional/internal/function_ref.h b/absl/functional/internal/function_ref.h index a1ddbb0c..1cd34a3c 100644 --- a/absl/functional/internal/function_ref.h +++ b/absl/functional/internal/function_ref.h @@ -20,6 +20,7 @@ #include <type_traits> #include "absl/base/internal/invoke.h" +#include "absl/functional/any_invocable.h" #include "absl/meta/type_traits.h" namespace absl { @@ -90,6 +91,12 @@ void AssertNonNull(const std::function<Sig>& f) { (void)f; } +template <typename Sig> +void AssertNonNull(const AnyInvocable<Sig>& f) { + assert(f != nullptr); + (void)f; +} + template <typename F> void AssertNonNull(const F&) {} diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index 7e534d85..e2397968 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -411,7 +411,7 @@ AbslHashValue(H hash_state, LongDouble value) { // AbslHashValue() for hashing pointers template <typename H, typename T> -H AbslHashValue(H hash_state, T* ptr) { + H AbslHashValue(H hash_state, T* ptr) { auto v = reinterpret_cast<uintptr_t>(ptr); // Due to alignment, pointers tend to have low bits as zero, and the next few // bits follow a pattern since they are also multiples of some base value. diff --git a/absl/log/internal/conditions.h b/absl/log/internal/conditions.h index b89f1dfd..b3ce2574 100644 --- a/absl/log/internal/conditions.h +++ b/absl/log/internal/conditions.h @@ -56,9 +56,15 @@ // the ternary expression does a better job avoiding spurious diagnostics // (dangling else, missing switch case) and preserving noreturn semantics (e.g. // on `LOG(FATAL)`) without requiring braces. +// +// The `switch` ensures that this expansion is the begnning of a statement (as +// opposed to an expression) and prevents shenanigans like +// `AFunction(LOG(INFO))` and `decltype(LOG(INFO))`. The apparently-redundant +// `default` case makes the condition more amenable to Clang dataflow analysis. #define ABSL_LOG_INTERNAL_STATELESS_CONDITION(condition) \ switch (0) \ case 0: \ + default: \ !(condition) ? (void)0 : ::absl::log_internal::Voidify()&& // `ABSL_LOG_INTERNAL_STATEFUL_CONDITION` applies a condition like diff --git a/absl/strings/escaping.cc b/absl/strings/escaping.cc index 6103750a..2827fbaa 100644 --- a/absl/strings/escaping.cc +++ b/absl/strings/escaping.cc @@ -846,10 +846,9 @@ void HexStringToBytesInternal(const char* from, T to, size_t num) { template <typename T> void BytesToHexStringInternal(const unsigned char* src, T dest, size_t num) { auto dest_ptr = &dest[0]; - for (auto src_ptr = src; src_ptr != (src + num); ++src_ptr) { + for (auto src_ptr = src; src_ptr != (src + num); ++src_ptr, dest_ptr += 2) { const char* hex_p = &numbers_internal::kHexTable[*src_ptr * 2]; - *dest_ptr++ = *hex_p++; - *dest_ptr++ = *hex_p; + std::copy(hex_p, hex_p + 2, dest_ptr); } } diff --git a/absl/strings/internal/charconv_bigint.cc b/absl/strings/internal/charconv_bigint.cc index 552fecc6..46b5289a 100644 --- a/absl/strings/internal/charconv_bigint.cc +++ b/absl/strings/internal/charconv_bigint.cc @@ -296,10 +296,8 @@ template <int max_words> std::min(n / kLargePowerOfFiveStep, kLargestPowerOfFiveIndex); if (first_pass) { // just copy, rather than multiplying by 1 - std::copy_n( - LargePowerOfFiveData(big_power), - LargePowerOfFiveSize(big_power), - answer.words_); + std::copy_n(LargePowerOfFiveData(big_power), + LargePowerOfFiveSize(big_power), answer.words_); answer.size_ = LargePowerOfFiveSize(big_power); first_pass = false; } else { diff --git a/absl/synchronization/internal/pthread_waiter.cc b/absl/synchronization/internal/pthread_waiter.cc index 366aaea2..bf700e95 100644 --- a/absl/synchronization/internal/pthread_waiter.cc +++ b/absl/synchronization/internal/pthread_waiter.cc @@ -81,6 +81,8 @@ PthreadWaiter::PthreadWaiter() : waiter_count_(0), wakeup_count_(0) { #if defined(__GLIBC__) && \ (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30)) #define ABSL_INTERNAL_HAVE_PTHREAD_COND_CLOCKWAIT 1 +#elif defined(__ANDROID_API__) && __ANDROID_API__ >= 30 +#define ABSL_INTERNAL_HAVE_PTHREAD_COND_CLOCKWAIT 1 #endif // Calls pthread_cond_timedwait() or possibly something else like diff --git a/absl/synchronization/internal/sem_waiter.cc b/absl/synchronization/internal/sem_waiter.cc index 3b3849bf..d62dbdc7 100644 --- a/absl/synchronization/internal/sem_waiter.cc +++ b/absl/synchronization/internal/sem_waiter.cc @@ -46,6 +46,8 @@ SemWaiter::SemWaiter() : wakeups_(0) { #if defined(__GLIBC__) && \ (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 30)) #define ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT 1 +#elif defined(__ANDROID_API__) && __ANDROID_API__ >= 30 +#define ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT 1 #endif // Calls sem_timedwait() or possibly something else like diff --git a/absl/synchronization/internal/waiter_test.cc b/absl/synchronization/internal/waiter_test.cc index 9a933940..992db29b 100644 --- a/absl/synchronization/internal/waiter_test.cc +++ b/absl/synchronization/internal/waiter_test.cc @@ -72,7 +72,7 @@ class WaiterTest : public ::testing::Test { TYPED_TEST_SUITE_P(WaiterTest); -constexpr absl::Duration slop = absl::Milliseconds(10); +absl::Duration WithTolerance(absl::Duration d) { return d * 0.95; } TYPED_TEST_P(WaiterTest, WaitNoTimeout) { absl::synchronization_internal::ThreadPool tp(1); @@ -90,7 +90,7 @@ TYPED_TEST_P(WaiterTest, WaitNoTimeout) { EXPECT_TRUE( waiter.Wait(absl::synchronization_internal::KernelTimeout::Never())); absl::Duration waited = absl::Now() - start; - EXPECT_GE(waited, absl::Seconds(2) - slop); + EXPECT_GE(waited, WithTolerance(absl::Seconds(2))); } TYPED_TEST_P(WaiterTest, WaitDurationWoken) { @@ -107,7 +107,7 @@ TYPED_TEST_P(WaiterTest, WaitDurationWoken) { EXPECT_TRUE(waiter.Wait( absl::synchronization_internal::KernelTimeout(absl::Seconds(10)))); absl::Duration waited = absl::Now() - start; - EXPECT_GE(waited, absl::Milliseconds(500) - slop); + EXPECT_GE(waited, WithTolerance(absl::Milliseconds(500))); EXPECT_LT(waited, absl::Seconds(2)); } @@ -125,7 +125,7 @@ TYPED_TEST_P(WaiterTest, WaitTimeWoken) { EXPECT_TRUE(waiter.Wait(absl::synchronization_internal::KernelTimeout( start + absl::Seconds(10)))); absl::Duration waited = absl::Now() - start; - EXPECT_GE(waited, absl::Milliseconds(500) - slop); + EXPECT_GE(waited, WithTolerance(absl::Milliseconds(500))); EXPECT_LT(waited, absl::Seconds(2)); } @@ -135,7 +135,7 @@ TYPED_TEST_P(WaiterTest, WaitDurationReached) { EXPECT_FALSE(waiter.Wait( absl::synchronization_internal::KernelTimeout(absl::Milliseconds(500)))); absl::Duration waited = absl::Now() - start; - EXPECT_GE(waited, absl::Milliseconds(500) - slop); + EXPECT_GE(waited, WithTolerance(absl::Milliseconds(500))); EXPECT_LT(waited, absl::Seconds(1)); } @@ -145,7 +145,7 @@ TYPED_TEST_P(WaiterTest, WaitTimeReached) { EXPECT_FALSE(waiter.Wait(absl::synchronization_internal::KernelTimeout( start + absl::Milliseconds(500)))); absl::Duration waited = absl::Now() - start; - EXPECT_GE(waited, absl::Milliseconds(500) - slop); + EXPECT_GE(waited, WithTolerance(absl::Milliseconds(500))); EXPECT_LT(waited, absl::Seconds(1)); } diff --git a/absl/synchronization/mutex.h b/absl/synchronization/mutex.h index 148fa191..29c049df 100644 --- a/absl/synchronization/mutex.h +++ b/absl/synchronization/mutex.h @@ -695,6 +695,20 @@ class Condition { template<typename T> Condition(bool (*func)(T *), T *arg); + // Same as above, but allows for cases where `arg` comes from a pointer that + // is convertible to the function parameter type `T*` but not an exact match. + // + // For example, the argument might be `X*` but the function takes `const X*`, + // or the argument might be `Derived*` while the function takes `Base*`, and + // so on for cases where the argument pointer can be implicitly converted. + // + // Implementation notes: This constructor overload is required in addition to + // the one above to allow deduction of `T` from `arg` for cases such as where + // a function template is passed as `func`. Also, the dummy `typename = void` + // template parameter exists just to work around a MSVC mangling bug. + template <typename T, typename = void> + Condition(bool (*func)(T *), typename absl::internal::identity<T>::type *arg); + // Templated version for invoking a method that returns a `bool`. // // `Condition(object, &Class::Method)` constructs a `Condition` that evaluates @@ -1023,6 +1037,12 @@ inline Condition::Condition(bool (*func)(T *), T *arg) StoreCallback(func); } +template <typename T, typename> +inline Condition::Condition(bool (*func)(T *), + typename absl::internal::identity<T>::type *arg) + // Just delegate to the overload above. + : Condition(func, arg) {} + template <typename T> inline Condition::Condition(T *object, bool (absl::internal::identity<T>::type::*method)()) diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index f76b1e8b..ec039a70 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -872,6 +872,111 @@ TEST(Mutex, LockedMutexDestructionBug) ABSL_NO_THREAD_SAFETY_ANALYSIS { } } +// Some functions taking pointers to non-const. +bool Equals42(int *p) { return *p == 42; } +bool Equals43(int *p) { return *p == 43; } + +// Some functions taking pointers to const. +bool ConstEquals42(const int *p) { return *p == 42; } +bool ConstEquals43(const int *p) { return *p == 43; } + +// Some function templates taking pointers. Note it's possible for `T` to be +// deduced as non-const or const, which creates the potential for ambiguity, +// but which the implementation is careful to avoid. +template <typename T> +bool TemplateEquals42(T *p) { + return *p == 42; +} +template <typename T> +bool TemplateEquals43(T *p) { + return *p == 43; +} + +TEST(Mutex, FunctionPointerCondition) { + // Some arguments. + int x = 42; + const int const_x = 42; + + // Parameter non-const, argument non-const. + EXPECT_TRUE(absl::Condition(Equals42, &x).Eval()); + EXPECT_FALSE(absl::Condition(Equals43, &x).Eval()); + + // Parameter const, argument non-const. + EXPECT_TRUE(absl::Condition(ConstEquals42, &x).Eval()); + EXPECT_FALSE(absl::Condition(ConstEquals43, &x).Eval()); + + // Parameter const, argument const. + EXPECT_TRUE(absl::Condition(ConstEquals42, &const_x).Eval()); + EXPECT_FALSE(absl::Condition(ConstEquals43, &const_x).Eval()); + + // Parameter type deduced, argument non-const. + EXPECT_TRUE(absl::Condition(TemplateEquals42, &x).Eval()); + EXPECT_FALSE(absl::Condition(TemplateEquals43, &x).Eval()); + + // Parameter type deduced, argument const. + EXPECT_TRUE(absl::Condition(TemplateEquals42, &const_x).Eval()); + EXPECT_FALSE(absl::Condition(TemplateEquals43, &const_x).Eval()); + + // Parameter non-const, argument const is not well-formed. + EXPECT_FALSE((std::is_constructible<absl::Condition, decltype(Equals42), + decltype(&const_x)>::value)); + // Validate use of is_constructible by contrasting to a well-formed case. + EXPECT_TRUE((std::is_constructible<absl::Condition, decltype(ConstEquals42), + decltype(&const_x)>::value)); +} + +// Example base and derived class for use in predicates and test below. Not a +// particularly realistic example, but it suffices for testing purposes. +struct Base { + explicit Base(int v) : value(v) {} + int value; +}; +struct Derived : Base { + explicit Derived(int v) : Base(v) {} +}; + +// Some functions taking pointer to non-const `Base`. +bool BaseEquals42(Base *p) { return p->value == 42; } +bool BaseEquals43(Base *p) { return p->value == 43; } + +// Some functions taking pointer to const `Base`. +bool ConstBaseEquals42(const Base *p) { return p->value == 42; } +bool ConstBaseEquals43(const Base *p) { return p->value == 43; } + +TEST(Mutex, FunctionPointerConditionWithDerivedToBaseConversion) { + // Some arguments. + Derived derived(42); + const Derived const_derived(42); + + // Parameter non-const base, argument derived non-const. + EXPECT_TRUE(absl::Condition(BaseEquals42, &derived).Eval()); + EXPECT_FALSE(absl::Condition(BaseEquals43, &derived).Eval()); + + // Parameter const base, argument derived non-const. + EXPECT_TRUE(absl::Condition(ConstBaseEquals42, &derived).Eval()); + EXPECT_FALSE(absl::Condition(ConstBaseEquals43, &derived).Eval()); + + // Parameter const base, argument derived const. + EXPECT_TRUE(absl::Condition(ConstBaseEquals42, &const_derived).Eval()); + EXPECT_FALSE(absl::Condition(ConstBaseEquals43, &const_derived).Eval()); + + // Parameter const base, argument derived const. + EXPECT_TRUE(absl::Condition(ConstBaseEquals42, &const_derived).Eval()); + EXPECT_FALSE(absl::Condition(ConstBaseEquals43, &const_derived).Eval()); + + // Parameter derived, argument base is not well-formed. + bool (*derived_pred)(const Derived *) = [](const Derived *) { return true; }; + EXPECT_FALSE((std::is_constructible<absl::Condition, decltype(derived_pred), + Base *>::value)); + EXPECT_FALSE((std::is_constructible<absl::Condition, decltype(derived_pred), + const Base *>::value)); + // Validate use of is_constructible by contrasting to well-formed cases. + EXPECT_TRUE((std::is_constructible<absl::Condition, decltype(derived_pred), + Derived *>::value)); + EXPECT_TRUE((std::is_constructible<absl::Condition, decltype(derived_pred), + const Derived *>::value)); +} + struct True { template <class... Args> bool operator()(Args...) const { |