diff options
author | Abseil Team <absl-team@google.com> | 2019-05-10 12:38:49 -0700 |
---|---|---|
committer | Bara Kopi <bara@kopi.com> | 2019-05-10 16:08:13 -0400 |
commit | 0cbdc774b97f7e80ab60dbe2ed4eaca3b2e33fc8 (patch) | |
tree | 0d5be86e52fab1aa1901898b9ea7fe6f4b79e41e /absl | |
parent | 27c2f6e2f3b5929fbd322b0f0ca392eb02efd9f8 (diff) |
Export of internal Abseil changes.
--
ab1a58c85a462884413ec0022dc1fff19ccb8602 by Abseil Team <absl-team@google.com>:
Clarified the documentation in str_format.h to say that use of absl::FormatSpec
is ok for wrapper functions. Added tests that express this.
PiperOrigin-RevId: 247657991
--
fef9481e58d579f1514babcb960ca60a51883fd8 by CJ Johnson <johnsoncj@google.com>:
Adds exception safety tests for InlinedVector::InlinedVector() and InlinedVector::InlinedVector(const allocator_type&).
PiperOrigin-RevId: 247617048
--
ef3217e1cd1e9a6ff5f2025e061b8ce3735af78f by Abseil Team <absl-team@google.com>:
Internal change.
PiperOrigin-RevId: 247614063
--
ed4c3345c4a04d8ec5c9e627058f17fce55925b1 by CJ Johnson <johnsoncj@google.com>:
Update InlinedVector::clear()
Introduces inlined_vector_exception_safety_test with the first test (clear), adds new benchmarks (for clear), and updates the implementation of clear.
PiperOrigin-RevId: 247496049
--
144a3a77c93bc8b2226da6f4b56166ee3d9868de by Derek Mauro <dmauro@google.com>:
Internal change
PiperOrigin-RevId: 247482532
--
286bbb89e154d5424955b644edad5fe04be487f8 by Derek Mauro <dmauro@google.com>:
Add scripts to run ASAN and TSAN on CI.
PiperOrigin-RevId: 247479658
GitOrigin-RevId: ab1a58c85a462884413ec0022dc1fff19ccb8602
Change-Id: Ief4c5a62587d0c59d405735df469d498aa6bf101
Diffstat (limited to 'absl')
-rw-r--r-- | absl/base/BUILD.bazel | 2 | ||||
-rw-r--r-- | absl/container/BUILD.bazel | 12 | ||||
-rw-r--r-- | absl/container/CMakeLists.txt | 16 | ||||
-rw-r--r-- | absl/container/inlined_vector.h | 20 | ||||
-rw-r--r-- | absl/container/inlined_vector_benchmark.cc | 74 | ||||
-rw-r--r-- | absl/container/inlined_vector_exception_safety_test.cc | 55 | ||||
-rw-r--r-- | absl/container/internal/inlined_vector.h | 28 | ||||
-rw-r--r-- | absl/debugging/BUILD.bazel | 3 | ||||
-rw-r--r-- | absl/hash/BUILD.bazel | 2 | ||||
-rw-r--r-- | absl/strings/BUILD.bazel | 4 | ||||
-rw-r--r-- | absl/strings/str_format.h | 15 | ||||
-rw-r--r-- | absl/strings/str_format_test.cc | 22 | ||||
-rw-r--r-- | absl/synchronization/mutex.cc | 16 | ||||
-rw-r--r-- | absl/synchronization/mutex_test.cc | 15 | ||||
-rw-r--r-- | absl/types/compare.h | 6 | ||||
-rw-r--r-- | absl/types/compare_test.cc | 4 |
16 files changed, 258 insertions, 36 deletions
diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index 9fb1d8cd..317452da 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -444,7 +444,7 @@ cc_test( cc_test( name = "low_level_alloc_test", - size = "small", + size = "medium", srcs = ["internal/low_level_alloc_test.cc"], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 0488857e..99a72482 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -198,11 +198,23 @@ cc_test( deps = [ ":inlined_vector", "//absl/base", + "//absl/base:core_headers", "//absl/strings", "@com_github_google_benchmark//:benchmark_main", ], ) +cc_test( + name = "inlined_vector_exception_safety_test", + srcs = ["inlined_vector_exception_safety_test.cc"], + copts = ABSL_TEST_COPTS + ABSL_EXCEPTIONS_FLAG, + deps = [ + ":inlined_vector", + "//absl/base:exception_safety_testing", + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "test_instance_tracker", testonly = 1, diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 1e203dbf..526e37af 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -195,6 +195,22 @@ absl_cc_test( gmock_main ) +absl_cc_test( + NAME + inlined_vector_exception_safety_test + SRCS + "inlined_vector_exception_safety_test.cc" + COPTS + ${ABSL_TEST_COPTS} + ${ABSL_EXCEPTIONS_FLAG} + LINKOPTS + ${ABSL_EXCEPTIONS_FLAG_LINKOPTS} + DEPS + absl::inlined_vector + absl::exception_safety_testing + gmock_main +) + absl_cc_library( NAME test_instance_tracker diff --git a/absl/container/inlined_vector.h b/absl/container/inlined_vector.h index 16865272..61e0cfb4 100644 --- a/absl/container/inlined_vector.h +++ b/absl/container/inlined_vector.h @@ -784,16 +784,20 @@ class InlinedVector { // Destroys all elements in the inlined vector, sets the size of `0` and // deallocates the heap allocation if the inlined vector was allocated. void clear() noexcept { - size_type s = size(); - if (storage_.GetIsAllocated()) { - Destroy(storage_.GetAllocatedData(), storage_.GetAllocatedData() + s); - AllocatorTraits::deallocate(storage_.GetAllocator(), - storage_.GetAllocatedData(), + const bool is_allocated = storage_.GetIsAllocated(); + + pointer the_data = + is_allocated ? storage_.GetAllocatedData() : storage_.GetInlinedData(); + + inlined_vector_internal::DestroyElements(storage_.GetAllocator(), the_data, + storage_.GetSize()); + + if (is_allocated) { + AllocatorTraits::deallocate(storage_.GetAllocator(), the_data, storage_.GetAllocatedCapacity()); - } else if (s != 0) { // do nothing for empty vectors - Destroy(storage_.GetInlinedData(), storage_.GetInlinedData() + s); } - storage_.SetInlinedSize(0); + + storage_.SetInlinedSize(/* size = */ 0); } // `InlinedVector::reserve()` diff --git a/absl/container/inlined_vector_benchmark.cc b/absl/container/inlined_vector_benchmark.cc index 867a29ea..7bb3271b 100644 --- a/absl/container/inlined_vector_benchmark.cc +++ b/absl/container/inlined_vector_benchmark.cc @@ -1,4 +1,4 @@ -// Copyright 2017 The Abseil Authors. +// Copyright 2019 The Abseil Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "absl/container/inlined_vector.h" - #include <string> #include <vector> #include "benchmark/benchmark.h" #include "absl/base/internal/raw_logging.h" +#include "absl/base/macros.h" +#include "absl/container/inlined_vector.h" #include "absl/strings/str_cat.h" namespace { @@ -373,4 +373,72 @@ void BM_StdVectorEmpty(benchmark::State& state) { } BENCHMARK(BM_StdVectorEmpty); +constexpr size_t kInlineElements = 4; +constexpr size_t kSmallSize = kInlineElements / 2; +constexpr size_t kLargeSize = kInlineElements * 2; +constexpr size_t kBatchSize = 100; + +struct TrivialType { + size_t val; +}; + +using TrivialVec = absl::InlinedVector<TrivialType, kInlineElements>; + +class NontrivialType { + public: + ABSL_ATTRIBUTE_NOINLINE NontrivialType() : val_() {} + + ABSL_ATTRIBUTE_NOINLINE NontrivialType(const NontrivialType& other) + : val_(other.val_) {} + + ABSL_ATTRIBUTE_NOINLINE NontrivialType& operator=( + const NontrivialType& other) { + val_ = other.val_; + return *this; + } + + ABSL_ATTRIBUTE_NOINLINE ~NontrivialType() noexcept {} + + private: + size_t val_; +}; + +using NontrivialVec = absl::InlinedVector<NontrivialType, kInlineElements>; + +#define BENCHMARK_OPERATION(BM_Function) \ + BENCHMARK_TEMPLATE(BM_Function, TrivialVec, kSmallSize); \ + BENCHMARK_TEMPLATE(BM_Function, TrivialVec, kLargeSize); \ + BENCHMARK_TEMPLATE(BM_Function, NontrivialVec, kSmallSize); \ + BENCHMARK_TEMPLATE(BM_Function, NontrivialVec, kLargeSize) + +template <typename VecT, typename PrepareVec, typename TestVec> +void BatchedBenchmark(benchmark::State& state, PrepareVec prepare_vec, + TestVec test_vec) { + VecT vectors[kBatchSize]; + + while (state.KeepRunningBatch(kBatchSize)) { + // Prepare batch + state.PauseTiming(); + for (auto& vec : vectors) { + prepare_vec(&vec); + } + benchmark::DoNotOptimize(vectors); + state.ResumeTiming(); + + // Test batch + for (auto& vec : vectors) { + test_vec(&vec); + } + } +} + +template <typename VecT, size_t Size> +void BM_Clear(benchmark::State& state) { + BatchedBenchmark<VecT>( + state, + /* prepare_vec = */ [](VecT* vec) { vec->resize(Size); }, + /* test_vec = */ [](VecT* vec) { vec->clear(); }); +} +BENCHMARK_OPERATION(BM_Clear); + } // namespace diff --git a/absl/container/inlined_vector_exception_safety_test.cc b/absl/container/inlined_vector_exception_safety_test.cc new file mode 100644 index 00000000..0af048b1 --- /dev/null +++ b/absl/container/inlined_vector_exception_safety_test.cc @@ -0,0 +1,55 @@ +// Copyright 2019 The Abseil Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include <memory> + +#include "gtest/gtest.h" +#include "absl/base/internal/exception_safety_testing.h" +#include "absl/container/inlined_vector.h" + +namespace { + +constexpr size_t kInlined = 4; +constexpr size_t kSmallSize = kInlined / 2; +constexpr size_t kLargeSize = kInlined * 2; + +using Thrower = testing::ThrowingValue<>; +using ThrowerAlloc = testing::ThrowingAllocator<Thrower>; + +template <typename Allocator = std::allocator<Thrower>> +using InlVec = absl::InlinedVector<Thrower, kInlined, Allocator>; + +TEST(InlinedVector, DefaultConstructor) { + testing::TestThrowingCtor<InlVec<>>(); + + testing::TestThrowingCtor<InlVec<ThrowerAlloc>>(); +} + +TEST(InlinedVector, AllocConstructor) { + auto alloc = std::allocator<Thrower>(); + testing::TestThrowingCtor<InlVec<>>(alloc); + + auto throw_alloc = ThrowerAlloc(); + testing::TestThrowingCtor<InlVec<ThrowerAlloc>>(throw_alloc); +} + +TEST(InlinedVector, Clear) { + auto small_vec = InlVec<>(kSmallSize); + EXPECT_TRUE(testing::TestNothrowOp([&]() { small_vec.clear(); })); + + auto large_vec = InlVec<>(kLargeSize); + EXPECT_TRUE(testing::TestNothrowOp([&]() { large_vec.clear(); })); +} + +} // namespace diff --git a/absl/container/internal/inlined_vector.h b/absl/container/internal/inlined_vector.h index 6a5a75be..4589ce08 100644 --- a/absl/container/internal/inlined_vector.h +++ b/absl/container/internal/inlined_vector.h @@ -16,6 +16,7 @@ #define ABSL_CONTAINER_INTERNAL_INLINED_VECTOR_INTERNAL_H_ #include <cstddef> +#include <cstring> #include <iterator> #include <memory> #include <utility> @@ -31,6 +32,33 @@ using IsAtLeastForwardIterator = std::is_convertible< typename std::iterator_traits<Iterator>::iterator_category, std::forward_iterator_tag>; +template <typename AllocatorType, typename ValueType, typename SizeType> +void DestroyElements(AllocatorType alloc, ValueType* destroy_first, + SizeType destroy_size) { + using AllocatorTraits = std::allocator_traits<AllocatorType>; + + // Destroys `destroy_size` elements from `destroy_first`. + // + // Destroys the range + // [`destroy_first`, `destroy_first + destroy_size`). + // + // NOTE: We assume destructors do not throw and thus make no attempt to roll + // back. + for (SizeType i = 0; i < destroy_size; ++i) { + AllocatorTraits::destroy(alloc, destroy_first + i); + } + +#ifndef NDEBUG + // Overwrite unused memory with `0xab` so we can catch uninitialized usage. + // + // Cast to `void*` to tell the compiler that we don't care that we might be + // scribbling on a vtable pointer. + void* memory = reinterpret_cast<void*>(destroy_first); + size_t memory_size = sizeof(ValueType) * destroy_size; + std::memset(memory, 0xab, memory_size); +#endif // NDEBUG +} + template <typename T, size_t N, typename A> class Storage { public: diff --git a/absl/debugging/BUILD.bazel b/absl/debugging/BUILD.bazel index 0854314e..e4aed5e4 100644 --- a/absl/debugging/BUILD.bazel +++ b/absl/debugging/BUILD.bazel @@ -244,6 +244,7 @@ cc_test( "//conditions:default": [], }), linkopts = ABSL_LSAN_LINKOPTS + ABSL_DEFAULT_LINKOPTS, + tags = ["notsan"], deps = [ ":leak_check_api_enabled_for_testing", "//absl/base", @@ -256,6 +257,7 @@ cc_test( srcs = ["leak_check_test.cc"], copts = ["-UABSL_EXPECT_LEAK_SANITIZER"], linkopts = ABSL_DEFAULT_LINKOPTS, + tags = ["noasan"], deps = [ ":leak_check_api_disabled_for_testing", "//absl/base", # for raw_logging @@ -271,6 +273,7 @@ cc_test( name = "disabled_leak_check_test", srcs = ["leak_check_fail_test.cc"], linkopts = ABSL_LSAN_LINKOPTS + ABSL_DEFAULT_LINKOPTS, + tags = ["notsan"], deps = [ ":leak_check_api_enabled_for_testing", ":leak_check_disable", diff --git a/absl/hash/BUILD.bazel b/absl/hash/BUILD.bazel index e1e6eae8..8c2daf70 100644 --- a/absl/hash/BUILD.bazel +++ b/absl/hash/BUILD.bazel @@ -35,10 +35,10 @@ cc_library( copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ + ":city", "//absl/base:core_headers", "//absl/base:endian", "//absl/container:fixed_array", - "//absl/hash:city", "//absl/meta:type_traits", "//absl/numeric:int128", "//absl/strings", diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 2b194737..d6ce88da 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -404,7 +404,7 @@ cc_test( cc_test( name = "numbers_test", - size = "small", + size = "medium", srcs = [ "internal/numbers_test_common.h", "numbers_test.cc", @@ -628,7 +628,7 @@ cc_test( cc_test( name = "str_format_convert_test", - size = "small", + size = "medium", srcs = ["internal/str_format/convert_test.cc"], copts = ABSL_TEST_COPTS, visibility = ["//visibility:private"], diff --git a/absl/strings/str_format.h b/absl/strings/str_format.h index 539d9516..da3208e1 100644 --- a/absl/strings/str_format.h +++ b/absl/strings/str_format.h @@ -50,7 +50,7 @@ // * A `ParsedFormat` instance, which encapsulates a specific, pre-compiled // format string for a specific set of type(s), and which can be passed // between API boundaries. (The `FormatSpec` type should not be used -// directly.) +// directly except as an argument type for wrapper functions.) // // The `str_format` library provides the ability to output its format strings to // arbitrary sink types: @@ -157,10 +157,15 @@ class FormatCountCapture { // FormatSpec // // The `FormatSpec` type defines the makeup of a format string within the -// `str_format` library. You should not need to use or manipulate this type -// directly. A `FormatSpec` is a variadic class template that is evaluated at -// compile-time, according to the format string and arguments that are passed -// to it. +// `str_format` library. It is a variadic class template that is evaluated at +// compile-time, according to the format string and arguments that are passed to +// it. +// +// You should not need to manipulate this type directly. You should only name it +// if you are writing wrapper functions which accept format arguments that will +// be provided unmodified to functions in this library. Such a wrapper function +// might be a class method that provides format arguments and/or internally uses +// the result of formatting. // // For a `FormatSpec` to be valid at compile-time, it must be provided as // either: diff --git a/absl/strings/str_format_test.cc b/absl/strings/str_format_test.cc index d4cffa08..80830b36 100644 --- a/absl/strings/str_format_test.cc +++ b/absl/strings/str_format_test.cc @@ -13,7 +13,7 @@ namespace absl { namespace { using str_format_internal::FormatArgImpl; -class FormatEntryPointTest : public ::testing::Test { }; +using FormatEntryPointTest = ::testing::Test; TEST_F(FormatEntryPointTest, Format) { std::string sink; @@ -458,7 +458,7 @@ std::string SummarizeParsedFormat(const ParsedFormatBase& pc) { return out; } -class ParsedFormatTest : public testing::Test {}; +using ParsedFormatTest = ::testing::Test; TEST_F(ParsedFormatTest, SimpleChecked) { EXPECT_EQ("[ABC]{d:1$d}[DEF]", @@ -600,6 +600,24 @@ TEST_F(ParsedFormatTest, RegressionMixPositional) { EXPECT_FALSE((ExtendedParsedFormat<Conv::d, Conv::o>::New("%1$d %o"))); } +using FormatWrapperTest = ::testing::Test; + +// Plain wrapper for StrFormat. +template <typename... Args> +std::string WrappedFormat(const absl::FormatSpec<Args...>& format, + const Args&... args) { + return StrFormat(format, args...); +} + +TEST_F(FormatWrapperTest, ConstexprStringFormat) { + EXPECT_EQ(WrappedFormat("%s there", "hello"), "hello there"); +} + +TEST_F(FormatWrapperTest, ParsedFormat) { + ParsedFormat<'s'> format("%s there"); + EXPECT_EQ(WrappedFormat(format, "hello"), "hello there"); +} + } // namespace } // namespace absl diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 37ffff3d..3e8033a0 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -154,7 +154,7 @@ static int Delay(int32_t c, DelayMode mode) { if (c < limit) { c++; // spin } else { - ABSL_TSAN_MUTEX_PRE_DIVERT(0, 0); + ABSL_TSAN_MUTEX_PRE_DIVERT(nullptr, 0); if (c == limit) { // yield once AbslInternalMutexYield(); c++; @@ -162,7 +162,7 @@ static int Delay(int32_t c, DelayMode mode) { absl::SleepFor(absl::Microseconds(10)); c = 0; } - ABSL_TSAN_MUTEX_POST_DIVERT(0, 0); + ABSL_TSAN_MUTEX_POST_DIVERT(nullptr, 0); } return (c); } @@ -2583,7 +2583,7 @@ void CondVar::Wakeup(PerThreadSynch *w) { } void CondVar::Signal() { - ABSL_TSAN_MUTEX_PRE_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_PRE_SIGNAL(nullptr, 0); intptr_t v; int c = 0; for (v = cv_.load(std::memory_order_relaxed); v != 0; @@ -2612,17 +2612,17 @@ void CondVar::Signal() { if ((v & kCvEvent) != 0) { PostSynchEvent(this, SYNCH_EV_SIGNAL); } - ABSL_TSAN_MUTEX_POST_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_POST_SIGNAL(nullptr, 0); return; } else { c = Delay(c, GENTLE); } } - ABSL_TSAN_MUTEX_POST_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_POST_SIGNAL(nullptr, 0); } void CondVar::SignalAll () { - ABSL_TSAN_MUTEX_PRE_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_PRE_SIGNAL(nullptr, 0); intptr_t v; int c = 0; for (v = cv_.load(std::memory_order_relaxed); v != 0; @@ -2649,13 +2649,13 @@ void CondVar::SignalAll () { if ((v & kCvEvent) != 0) { PostSynchEvent(this, SYNCH_EV_SIGNALALL); } - ABSL_TSAN_MUTEX_POST_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_POST_SIGNAL(nullptr, 0); return; } else { c = Delay(c, GENTLE); // try again after a delay } } - ABSL_TSAN_MUTEX_POST_SIGNAL(0, 0); + ABSL_TSAN_MUTEX_POST_SIGNAL(nullptr, 0); } void ReleasableMutexLock::Release() { diff --git a/absl/synchronization/mutex_test.cc b/absl/synchronization/mutex_test.cc index 10211229..2e10f098 100644 --- a/absl/synchronization/mutex_test.cc +++ b/absl/synchronization/mutex_test.cc @@ -815,7 +815,12 @@ TEST(Mutex, MutexReaderDecrementBug) NO_THREAD_SAFETY_ANALYSIS { // Test that we correctly handle the situation when a lock is // held and then destroyed (w/o unlocking). +#ifdef THREAD_SANITIZER +// TSAN reports errors when locked Mutexes are destroyed. +TEST(Mutex, DISABLED_LockedMutexDestructionBug) NO_THREAD_SAFETY_ANALYSIS { +#else TEST(Mutex, LockedMutexDestructionBug) NO_THREAD_SAFETY_ANALYSIS { +#endif for (int i = 0; i != 10; i++) { // Create, lock and destroy 10 locks. const int kNumLocks = 10; @@ -1062,7 +1067,12 @@ class ScopedDisableBazelTestWarnings { const char ScopedDisableBazelTestWarnings::kVarName[] = "TEST_WARNINGS_OUTPUT_FILE"; +#ifdef THREAD_SANITIZER +// This test intentionally creates deadlocks to test the deadlock detector. +TEST(Mutex, DISABLED_DeadlockDetectorBazelWarning) { +#else TEST(Mutex, DeadlockDetectorBazelWarning) { +#endif absl::SetMutexDeadlockDetectionMode(absl::OnDeadlockCycle::kReport); // Cause deadlock detection to detect something, if it's @@ -1109,7 +1119,12 @@ TEST(Mutex, DeadlockDetectorStessTest) NO_THREAD_SAFETY_ANALYSIS { } } +#ifdef THREAD_SANITIZER +// TSAN reports errors when locked Mutexes are destroyed. +TEST(Mutex, DISABLED_DeadlockIdBug) NO_THREAD_SAFETY_ANALYSIS { +#else TEST(Mutex, DeadlockIdBug) NO_THREAD_SAFETY_ANALYSIS { +#endif // Test a scenario where a cached deadlock graph node id in the // list of held locks is not invalidated when the corresponding // mutex is deleted. diff --git a/absl/types/compare.h b/absl/types/compare.h index 7fed3081..50361d62 100644 --- a/absl/types/compare.h +++ b/absl/types/compare.h @@ -452,8 +452,10 @@ namespace compare_internal { // Helper functions to do a boolean comparison of two keys given a boolean // or three-way comparator. -constexpr bool compare_result_as_less_than(const bool r) { return r; } -constexpr bool compare_result_as_less_than(const int r) { return r < 0; } +// SFINAE prevents implicit conversions to bool (such as from int). +template <typename Bool, + absl::enable_if_t<std::is_same<bool, Bool>::value, int> = 0> +constexpr bool compare_result_as_less_than(const Bool r) { return r; } constexpr bool compare_result_as_less_than(const absl::weak_ordering r) { return r < 0; } diff --git a/absl/types/compare_test.cc b/absl/types/compare_test.cc index dd0388c1..3a855421 100644 --- a/absl/types/compare_test.cc +++ b/absl/types/compare_test.cc @@ -200,10 +200,6 @@ TEST(CompareResultAsLessThan, SanityTest) { EXPECT_FALSE(absl::compare_internal::compare_result_as_less_than(false)); EXPECT_TRUE(absl::compare_internal::compare_result_as_less_than(true)); - EXPECT_TRUE(absl::compare_internal::compare_result_as_less_than(-1)); - EXPECT_FALSE(absl::compare_internal::compare_result_as_less_than(0)); - EXPECT_FALSE(absl::compare_internal::compare_result_as_less_than(1)); - EXPECT_TRUE( absl::compare_internal::compare_result_as_less_than(weak_ordering::less)); EXPECT_FALSE(absl::compare_internal::compare_result_as_less_than( |