From ca1d7cb497beb850a7193bd071c2a87718af31cf Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 13 May 2026 13:43:25 -0700 Subject: [PATCH] Add container overloads for absl::c_copy and absl::c_copy_n These versions accept a container as the output destination. The primary motivation for these overloads is to add bounds checking. We determine if an object is a container by checking if they support `std::begin`/`std::end`. PiperOrigin-RevId: 915035028 Change-Id: Ibeb17b784ec313215ba8216a6085628d11eac102 --- absl/algorithm/BUILD.bazel | 2 + absl/algorithm/CMakeLists.txt | 2 + absl/algorithm/container.h | 141 ++++++++++++++++- absl/algorithm/container_test.cc | 262 +++++++++++++++++++++++++++++++ 4 files changed, 399 insertions(+), 8 deletions(-) diff --git a/absl/algorithm/BUILD.bazel b/absl/algorithm/BUILD.bazel index b56ba66a..d482afe3 100644 --- a/absl/algorithm/BUILD.bazel +++ b/absl/algorithm/BUILD.bazel @@ -70,6 +70,7 @@ cc_library( ":algorithm", "//absl/base:config", "//absl/base:core_headers", + "//absl/base:iterator_traits_internal", "//absl/meta:type_traits", ], ) @@ -85,6 +86,7 @@ cc_test( "//absl/base:config", "//absl/base:core_headers", "//absl/memory", + "//absl/meta:type_traits", "//absl/random", "//absl/types:span", "@googletest//:gtest", diff --git a/absl/algorithm/CMakeLists.txt b/absl/algorithm/CMakeLists.txt index 2fb484da..df6ae832 100644 --- a/absl/algorithm/CMakeLists.txt +++ b/absl/algorithm/CMakeLists.txt @@ -50,6 +50,7 @@ absl_cc_library( DEPS absl::algorithm absl::config + absl::iterator_traits_internal absl::core_headers absl::meta PUBLIC @@ -67,6 +68,7 @@ absl_cc_test( absl::config absl::core_headers absl::memory + absl::meta absl::random_random absl::span GTest::gmock_main diff --git a/absl/algorithm/container.h b/absl/algorithm/container.h index a823ee15..236983e1 100644 --- a/absl/algorithm/container.h +++ b/absl/algorithm/container.h @@ -52,6 +52,7 @@ #include "absl/algorithm/algorithm.h" #include "absl/base/config.h" +#include "absl/base/internal/iterator_traits.h" #include "absl/base/macros.h" #include "absl/meta/type_traits.h" @@ -106,6 +107,45 @@ ABSL_INTERNAL_CONSTEXPR_SINCE_CXX17 ContainerIter c_end(C& c) { return end(c); } +// Helper to check that the `OutputRange` has enough space. +// Only performs the check if the iterators are ForwardIterators or better. +template +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX17 bool CheckCopyNSize(InputSequence& input, + Size n, + OutputRange& output) { + using InputIter = ContainerIter; + using OutputIter = ContainerIter; + + if constexpr (base_internal::IsAtLeastForwardIterator::value) { + if (n > std::distance(container_algorithm_internal::c_begin(input), + container_algorithm_internal::c_end(input))) { + return false; + } + } + if constexpr (base_internal::IsAtLeastForwardIterator::value) { + if (n > std::distance(container_algorithm_internal::c_begin(output), + container_algorithm_internal::c_end(output))) { + return false; + } + } + return true; +} + +template +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX17 bool CheckCopySize(InputSequence& input, + OutputRange& output) { + using InputIter = ContainerIter; + using OutputIter = ContainerIter; + if constexpr (base_internal::IsAtLeastForwardIterator::value && + base_internal::IsAtLeastForwardIterator::value) { + return std::distance(container_algorithm_internal::c_begin(input), + container_algorithm_internal::c_end(input)) <= + std::distance(container_algorithm_internal::c_begin(output), + container_algorithm_internal::c_end(output)); + } + return true; +} + template struct IsUnorderedContainer : std::false_type {}; @@ -117,6 +157,27 @@ template struct IsUnorderedContainer> : std::true_type {}; +template +struct HasBeginEnd : std::false_type {}; + +template +struct HasBeginEnd()())), + decltype(container_algorithm_internal::end( + std::declval()()))>> + : std::true_type {}; + +// We don't support multidimensional arrays yet +template +using IsMultidimensionalArray = std::is_array>; + +template +struct IsIterator : std::false_type {}; + +template +struct IsIterator< + Iter, std::void_t::iterator_category>> + : std::true_type {}; } // namespace container_algorithm_internal // PUBLIC API @@ -521,10 +582,42 @@ ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 // Container-based version of the `std::copy()` function to copy a // container's elements into an iterator. template -ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 OutputIterator -c_copy(const InputSequence& input, OutputIterator output) { +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 + std::enable_if_t>::value && + !container_algorithm_internal::IsMultidimensionalArray< + InputSequence>::value, + std::decay_t> + c_copy(const InputSequence& input, OutputIterator&& output) { return std::copy(container_algorithm_internal::c_begin(input), - container_algorithm_internal::c_end(input), output); + container_algorithm_internal::c_end(input), + std::forward(output)); +} + +// Copies elements from `input` to `output`. `absl::c_copy(input, output)` is +// equivalent to `std::copy(std::begin(input), std::end(input), +// std::begin(output))`. +// +// The `output` container must be large enough to hold all elements of `input`; +// this function does not resize `output`. + +// If `std::size(input) > std::size(output)`, behavior is undefined. +// If `std::size(output) > std::size(input)`, only `std::size(input)` elements +// are copied, and `output` is not truncated. +template +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 + std::enable_if_t>::value && + !container_algorithm_internal::IsMultidimensionalArray< + std::remove_reference_t>::value && + !container_algorithm_internal::IsMultidimensionalArray< + InputSequence>::value, + void> + c_copy(const InputSequence& input, OutputRange&& output) { + ABSL_HARDENING_ASSERT( + container_algorithm_internal::CheckCopySize(input, output)); + absl::c_copy(input, container_algorithm_internal::c_begin( + std::forward(output))); } // c_copy_n() @@ -532,9 +625,41 @@ c_copy(const InputSequence& input, OutputIterator output) { // Container-based version of the `std::copy_n()` function to copy a // container's first N elements into an iterator. template -ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 OutputIterator -c_copy_n(const C& input, Size n, OutputIterator output) { - return std::copy_n(container_algorithm_internal::c_begin(input), n, output); +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 std::enable_if_t< + container_algorithm_internal::IsIterator< + absl::remove_cvref_t>::value && + !container_algorithm_internal::IsMultidimensionalArray::value, + std::decay_t> +c_copy_n(const C& input, Size n, OutputIterator&& output) { + return std::copy_n(container_algorithm_internal::c_begin(input), n, + std::forward(output)); +} + +// Copies the first `n` elements from `input` to `output`. +// `absl::c_copy_n(input, n, output)` is equivalent to +// `std::copy_n(std::begin(input), n, std::begin(output))`. +// +// The `output` container must be large enough to hold N elements; this function +// does not resize `output`. +// +// If `n > std::size(output)` or `n > std::size(input)`, behavior is +// undefined. +// If `std::size(output) > n`, only `n` elements are copied, and `output` is not +// truncated. +template +ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 std::enable_if_t< + container_algorithm_internal::HasBeginEnd< + std::add_lvalue_reference_t>::value && + !container_algorithm_internal::IsMultidimensionalArray< + std::remove_reference_t>::value && + !container_algorithm_internal::IsMultidimensionalArray::value, + void> +c_copy_n(const C& input, Size n, OutputRange&& output) { + ABSL_HARDENING_ASSERT( + container_algorithm_internal::CheckCopyNSize(input, n, output)); + absl::c_copy_n( + input, n, + container_algorithm_internal::c_begin(std::forward(output))); } // c_copy_if() @@ -819,8 +944,8 @@ template > ABSL_INTERNAL_CONSTEXPR_SINCE_CXX20 Iterator c_rotate(C& sequence, Iterator middle) { - return std::rotate(container_algorithm_internal::c_begin(sequence), middle, - container_algorithm_internal::c_end(sequence)); + return absl::rotate(container_algorithm_internal::c_begin(sequence), middle, + container_algorithm_internal::c_end(sequence)); } // c_rotate_copy() diff --git a/absl/algorithm/container_test.cc b/absl/algorithm/container_test.cc index 8a898be8..fcb84925 100644 --- a/absl/algorithm/container_test.cc +++ b/absl/algorithm/container_test.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +38,7 @@ #include "absl/base/config.h" #include "absl/base/macros.h" #include "absl/memory/memory.h" +#include "absl/meta/type_traits.h" #include "absl/random/random.h" #include "absl/types/span.h" @@ -712,6 +715,190 @@ TEST(MutatingTest, CopyN) { EXPECT_EQ(expected, actual); } +TEST(MutatingTest, CopyNWithNegativeN) { +#ifdef _LIBCPP_VERSION + GTEST_SKIP() << "libc++ does not handle negative counts correctly"; +#else + const std::vector input = {1, 2, 3}; + std::vector actual = {0, 0, 0}; + absl::c_copy_n(input, -1, actual.begin()); + EXPECT_THAT(actual, ElementsAre(0, 0, 0)); +#endif +} + +TEST(MutatingTest, CopyToContainer) { + const std::vector input = {1, 2, 3}; + std::vector actual = {0, 0, 0, 4, 5}; + absl::c_copy(input, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 3, 4, 5)); +} + +TEST(MutatingTest, CopyNToContainer) { + const std::vector input = {1, 2, 3, 4, 5}; + std::vector actual = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 2, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 0, 0, 0)); +} + +TEST(MutatingTest, CopyNToContainerWithZeroN) { + const std::vector input = {1, 2, 3, 4, 5}; + std::vector actual = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 0, actual); + EXPECT_THAT(actual, ElementsAre(0, 0, 0, 0, 0)); +} + +TEST(MutatingTest, CopyNToContainerWithNegativeN) { +#ifdef _LIBCPP_VERSION + GTEST_SKIP() << "libc++ does not handle negative counts correctly"; +#else + const std::vector input = {1, 2, 3}; + std::vector actual = {0, 0, 0}; + absl::c_copy_n(input, -1, actual); + EXPECT_THAT(actual, ElementsAre(0, 0, 0)); +#endif +} + +TEST(MutatingTest, CopyToDifferentContainerType) { + const std::list input = {1, 2, 3}; + std::array actual = {0, 0, 0, 4, 5}; + absl::c_copy(input, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 3, 4, 5)); +} + +TEST(MutatingTest, CopyNToDifferentContainerType) { + const std::list input = {1, 2, 3, 4, 5}; + std::array actual = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 2, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 0, 0, 0)); +} + +TEST(MutatingTest, CopyToCArray) { + const std::vector input = {1, 2, 3}; + int actual[5] = {0, 0, 0, 4, 5}; + absl::c_copy(input, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 3, 4, 5)); +} + +TEST(MutatingTest, CopyNToCArray) { + const std::vector input = {1, 2, 3, 4, 5}; + int actual[5] = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 2, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 0, 0, 0)); +} + +TEST(MutatingTest, CopyFromCArray) { + const int input[5] = {1, 2, 3, 4, 5}; + std::vector actual = {0, 0, 0, 0, 0}; + absl::c_copy(input, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 3, 4, 5)); +} + +TEST(MutatingTest, CopyNFromCArray) { + const int input[5] = {1, 2, 3, 4, 5}; + std::vector actual = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 2, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 0, 0, 0)); +} + +TEST(MutatingTest, CopyContainerWithNoSizeMethod) { + const std::forward_list input = {1, 2, 3}; + std::forward_list actual = {0, 0, 0, 4, 5}; + absl::c_copy(input, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 3, 4, 5)); +} + +TEST(MutatingTest, CopyNContainerWithNoSizeMethod) { + const std::forward_list input = {1, 2, 3, 4, 5}; + std::forward_list actual = {0, 0, 0, 0, 0}; + absl::c_copy_n(input, 2, actual); + EXPECT_THAT(actual, ElementsAre(1, 2, 0, 0, 0)); +} + +#if GTEST_HAS_DEATH_TEST + +bool IsHardened() { + bool hardened = false; + ABSL_HARDENING_ASSERT([&hardened]() { + hardened = true; + return true; + }()); + return hardened; +} + +TEST(MutatingTest, CopyToCArrayInvalidSize) { + const std::vector input = {1, 2, 3}; + int actual[2] = {0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy(input, actual), ""); + } +} + +TEST(MutatingTest, CopyNToCArrayInvalidSize) { + const std::vector input = {1, 2, 3}; + int actual[2] = {0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 3, actual), ""); + } +} + +TEST(MutatingTest, CopyNToCArrayNGreaterThanInput) { + const std::vector input = {1, 2, 3}; + int actual[4] = {0, 0, 0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 4, actual), ""); + } +} + +TEST(MutatingTest, CopyToContainerInvalidSize) { + const std::list input = {1, 2, 3, 4, 5}; + std::list actual = {0, 0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy(input, actual), ""); + } +} + +TEST(MutatingTest, CopyNToContainerNGreaterThanInput) { + const std::vector input = {1, 2, 3}; + std::vector actual = {0, 0, 0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 4, actual), ""); + } +} + +TEST(MutatingTest, CopyNToContainerNGreaterThanOutput) { + const std::vector input = {1, 2, 3}; + std::vector actual = {0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 3, actual), ""); + } +} + +TEST(MutatingTest, CopyToForwardListInvalidSize) { + const std::forward_list input = {1, 2, 3, 4, 5}; + std::forward_list actual = {0, 0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy(input, actual), ""); + } +} + +TEST(MutatingTest, CopyNToForwardListNGreaterThanInput) { + const std::forward_list input = {1, 2, 3}; + std::forward_list actual = {0, 0, 0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 4, actual), ""); + } +} + +TEST(MutatingTest, CopyNToForwardListNGreaterThanOutput) { + const std::forward_list input = {1, 2, 3}; + std::forward_list actual = {0, 0}; + if (IsHardened()) { + EXPECT_DEATH(absl::c_copy_n(input, 3, actual), ""); + } +} + +#endif // GTEST_HAS_DEATH_TEST + TEST(MutatingTest, CopyIf) { const std::list input = {1, 2, 3}; std::vector output; @@ -2229,4 +2416,79 @@ TEST(ConstexprTest, PartialSumWithPredicate) { #endif // defined(ABSL_INTERNAL_CPLUSPLUS_LANG) && // ABSL_INTERNAL_CPLUSPLUS_LANG >= 202002L +// A type that acts as both a container and an iterator. +struct AmbiguousType { + // Container requirements + int* begin() { return nullptr; } + int* end() { return nullptr; } + + // Iterator requirements + using iterator_category = std::input_iterator_tag; + using value_type = int; + using difference_type = std::ptrdiff_t; + using pointer = int*; + using reference = int&; + + int& operator*() { + static int x; + return x; + } + AmbiguousType& operator++() { return *this; } + AmbiguousType operator++(int) { return *this; } + friend bool operator==(const AmbiguousType&, const AmbiguousType&) { + return true; + } + friend bool operator!=(const AmbiguousType&, const AmbiguousType&) { + return false; + } +}; + +template +struct CanCopy : std::false_type {}; +template +struct CanCopy(), + std::declval()))>> + : std::true_type {}; + +template +struct CanCopyN : std::false_type {}; +template +struct CanCopyN(), std::declval(), + std::declval()))>> : std::true_type {}; + +TEST(CanCopyTest, CopyToMultiDimArray) { + static_assert(CanCopy, int (&)[10]>::value); + static_assert(!CanCopy, int (&)[2][2]>::value); + static_assert(CanCopyN, int (&)[10]>::value); + static_assert(!CanCopyN, int (&)[2][2]>::value); + + static_assert(CanCopy::value); + static_assert(!CanCopy::value); + static_assert(CanCopyN::value); + static_assert(!CanCopyN::value); + static_assert(!CanCopy::value); + static_assert(!CanCopy::value); + static_assert(!CanCopyN::value); + static_assert(!CanCopyN::value); +} + +TEST(CanCopyTest, BlockNonWritableIterators) { + using Vec = std::vector; + + static_assert(CanCopy::value); + static_assert(CanCopy>::value); +} + +TEST(CanCopyTest, AmbiguousTypeFailsToCompile) { + using Vec = std::vector; + // Because AmbiguousType is both an iterator and a container, + // the compiler should fail to resolve the c_copy overload. + static_assert(!CanCopy::value, + "Ambiguous types should not compile!"); + static_assert(!CanCopyN::value, + "Ambiguous types should not compile!"); +} } // namespace