Skip to content

Commit

Permalink
Allow C++20 forward iterators to use fast paths
Browse files Browse the repository at this point in the history
Some forward iterators (by C++20's definition) are only input iterators in C++17 or before, because they cannot have a language reference for their `reference` type, e.g. a zip iterator. However, we can still use the fast paths for these iterators. E.g. this makes `absl::InlinedVector::insert(iterator, InputIt, InputIt)` faster for this category of forward iterator.

`iterator_concept` was added in C++20, but it's reasonable to support this for pre-C++20 code as well.

PiperOrigin-RevId: 725791745
Change-Id: I7dd02eef0b94ea6d4bfbda8f7f72db4abea10440
  • Loading branch information
Quincunx271 authored and copybara-github committed Feb 11, 2025
1 parent 9e764b4 commit ae4b0c5
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 3 deletions.
14 changes: 13 additions & 1 deletion absl/base/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,10 @@ cc_library(
hdrs = ["internal/iterator_traits.h"],
copts = ABSL_DEFAULT_COPTS,
linkopts = ABSL_DEFAULT_LINKOPTS,
deps = [":config"],
deps = [
":config",
"//absl/meta:type_traits",
],
)

cc_test(
Expand All @@ -971,6 +974,7 @@ cc_test(
deps = [
":config",
":iterator_traits_internal",
":iterator_traits_test_helper",
"@googletest//:gtest",
"@googletest//:gtest_main",
],
Expand All @@ -991,6 +995,14 @@ cc_library(
],
)

cc_library(
name = "iterator_traits_test_helper",
hdrs = ["internal/iterator_traits_test_helper.h"],
copts = ABSL_DEFAULT_COPTS,
linkopts = ABSL_DEFAULT_LINKOPTS,
deps = [":config"],
)

cc_test(
name = "tracing_internal_weak_test",
srcs = ["internal/tracing_weak_test.cc"],
Expand Down
15 changes: 15 additions & 0 deletions absl/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ absl_cc_library(
${ABSL_DEFAULT_COPTS}
DEPS
absl::config
absl::type_traits
PUBLIC
)

Expand All @@ -858,5 +859,19 @@ absl_cc_test(
DEPS
absl::config
absl::iterator_traits_internal
absl::iterator_traits_test_helper_internal
GTest::gtest_main
)

# Internal-only target, do not depend on directly.
absl_cc_library(
NAME
iterator_traits_test_helper_internal
HDRS
"internal/iterator_traits_test_helper.h"
COPTS
${ABSL_DEFAULT_COPTS}
DEPS
absl::config
PUBLIC
)
30 changes: 28 additions & 2 deletions absl/base/internal/iterator_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,40 @@
#include <type_traits>

#include "absl/base/config.h"
#include "absl/meta/type_traits.h"

namespace absl {
ABSL_NAMESPACE_BEGIN
namespace base_internal {

template <typename Iterator, typename = void>
struct IteratorCategory {};

template <typename Iterator>
struct IteratorCategory<
Iterator,
absl::void_t<typename std::iterator_traits<Iterator>::iterator_category>> {
using type = typename std::iterator_traits<Iterator>::iterator_category;
};

template <typename Iterator, typename = void>
struct IteratorConceptImpl : IteratorCategory<Iterator> {};

template <typename Iterator>
struct IteratorConceptImpl<
Iterator,
absl::void_t<typename std::iterator_traits<Iterator>::iterator_concept>> {
using type = typename std::iterator_traits<Iterator>::iterator_concept;
};

// The newer `std::iterator_traits<Iterator>::iterator_concept` if available,
// else `std::iterator_traits<Iterator>::iterator_category`.
template <typename Iterator>
using IteratorConcept = typename IteratorConceptImpl<Iterator>::type;

template <typename IteratorTag, typename Iterator>
using IsAtLeastIterator = std::is_convertible<
typename std::iterator_traits<Iterator>::iterator_category, IteratorTag>;
using IsAtLeastIterator =
std::is_convertible<IteratorConcept<Iterator>, IteratorTag>;

template <typename Iterator>
using IsAtLeastForwardIterator =
Expand Down
10 changes: 10 additions & 0 deletions absl/base/internal/iterator_traits_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "gtest/gtest.h"
#include "absl/base/config.h"
#include "absl/base/internal/iterator_traits_test_helper.h"

namespace absl {
ABSL_NAMESPACE_BEGIN
Expand Down Expand Up @@ -67,6 +68,15 @@ TEST(IsAtLeastIteratorTest, IsAtLeastIterator) {
std::istream_iterator<int>>()));
EXPECT_FALSE((IsAtLeastIterator<std::random_access_iterator_tag,
std::istream_iterator<int>>()));

EXPECT_TRUE((IsAtLeastIterator<std::input_iterator_tag,
Cpp20ForwardZipIterator<int*>>()));
EXPECT_TRUE((IsAtLeastIterator<std::forward_iterator_tag,
Cpp20ForwardZipIterator<int*>>()));
EXPECT_FALSE((IsAtLeastIterator<std::bidirectional_iterator_tag,
Cpp20ForwardZipIterator<int*>>()));
EXPECT_FALSE((IsAtLeastIterator<std::random_access_iterator_tag,
Cpp20ForwardZipIterator<int*>>()));
}

} // namespace
Expand Down
97 changes: 97 additions & 0 deletions absl/base/internal/iterator_traits_test_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Copyright 2025 The Abseil Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef ABSL_BASE_INTERNAL_ITERATOR_TRAITS_TEST_HELPER_H_
#define ABSL_BASE_INTERNAL_ITERATOR_TRAITS_TEST_HELPER_H_

#include <iterator>
#include <utility>

#include "absl/base/config.h"

namespace absl {
ABSL_NAMESPACE_BEGIN
namespace base_internal {

// This would be a forward_iterator in C++20, but it's only an input iterator
// before that, since it has a non-reference `reference`.
template <typename Iterator>
class Cpp20ForwardZipIterator {
using IteratorReference = typename std::iterator_traits<Iterator>::reference;

public:
Cpp20ForwardZipIterator() = default;
explicit Cpp20ForwardZipIterator(Iterator left, Iterator right)
: left_(left), right_(right) {}

Cpp20ForwardZipIterator& operator++() {
++left_;
++right_;
return *this;
}

Cpp20ForwardZipIterator operator++(int) {
Cpp20ForwardZipIterator tmp(*this);
++*this;
return *this;
}

std::pair<IteratorReference, IteratorReference> operator*() const {
return {*left_, *right_};
}

// C++17 input iterators require `operator->`, but this isn't possible to
// implement. C++20 dropped the requirement.

friend bool operator==(const Cpp20ForwardZipIterator& lhs,
const Cpp20ForwardZipIterator& rhs) {
return lhs.left_ == rhs.left_ && lhs.right_ == rhs.right_;
}

friend bool operator!=(const Cpp20ForwardZipIterator& lhs,
const Cpp20ForwardZipIterator& rhs) {
return !(lhs == rhs);
}

private:
Iterator left_{};
Iterator right_{};
};

} // namespace base_internal
ABSL_NAMESPACE_END
} // namespace absl

template <typename Iterator>
struct std::iterator_traits<
absl::base_internal::Cpp20ForwardZipIterator<Iterator>> {
private:
using IteratorReference = typename std::iterator_traits<Iterator>::reference;

public:
using iterator_category = std::input_iterator_tag;
using iterator_concept = std::forward_iterator_tag;
using value_type = std::pair<IteratorReference, IteratorReference>;
using difference_type =
typename std::iterator_traits<Iterator>::difference_type;
using reference = value_type;
using pointer = void;
};

#if defined(__cpp_lib_concepts)
static_assert(
std::forward_iterator<absl::base_internal::Cpp20ForwardZipIterator<int*>>);
#endif // defined(__cpp_lib_concepts)

#endif // ABSL_BASE_INTERNAL_ITERATOR_TRAITS_TEST_HELPER_H_
2 changes: 2 additions & 0 deletions absl/container/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ cc_test(
":test_allocator",
"//absl/base:config",
"//absl/base:exception_testing",
"//absl/base:iterator_traits_test_helper",
"//absl/hash:hash_testing",
"//absl/memory",
"@googletest//:gtest",
Expand Down Expand Up @@ -173,6 +174,7 @@ cc_test(
"//absl/base:config",
"//absl/base:core_headers",
"//absl/base:exception_testing",
"//absl/base:iterator_traits_test_helper",
"//absl/hash:hash_testing",
"//absl/log:check",
"//absl/memory",
Expand Down
1 change: 1 addition & 0 deletions absl/container/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ absl_cc_library(
absl::core_headers
absl::dynamic_annotations
absl::iterator_traits_internal
absl::iterator_traits_test_helper_internal
absl::throw_delegate
absl::memory
PUBLIC
Expand Down
17 changes: 17 additions & 0 deletions absl/container/fixed_array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,21 @@
#include <stdio.h>

#include <cstring>
#include <forward_list>
#include <list>
#include <memory>
#include <numeric>
#include <scoped_allocator>
#include <stdexcept>
#include <string>
#include <utility>
#include <vector>

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/base/config.h"
#include "absl/base/internal/exception_testing.h"
#include "absl/base/internal/iterator_traits_test_helper.h"
#include "absl/base/options.h"
#include "absl/container/internal/test_allocator.h"
#include "absl/hash/hash_testing.h"
Expand Down Expand Up @@ -409,6 +412,20 @@ TEST(IteratorConstructorTest, FromBidirectionalIteratorRange) {
EXPECT_THAT(fixed, testing::ElementsAreArray(kInput));
}

TEST(IteratorConstructorTest, FromCpp20ForwardIteratorRange) {
std::forward_list<int> const kUnzippedInput = {2, 3, 5, 7, 11, 13, 17};
absl::base_internal::Cpp20ForwardZipIterator<
std::forward_list<int>::const_iterator> const
begin(std::begin(kUnzippedInput), std::begin(kUnzippedInput));
absl::base_internal::
Cpp20ForwardZipIterator<std::forward_list<int>::const_iterator> const end(
std::end(kUnzippedInput), std::end(kUnzippedInput));

std::forward_list<std::pair<int, int>> const items(begin, end);
absl::FixedArray<std::pair<int, int>> const fixed(begin, end);
EXPECT_THAT(fixed, testing::ElementsAreArray(items));
}

TEST(InitListConstructorTest, InitListConstruction) {
absl::FixedArray<int> fixed = {1, 2, 3};
EXPECT_THAT(fixed, testing::ElementsAreArray({1, 2, 3}));
Expand Down
28 changes: 28 additions & 0 deletions absl/container/inlined_vector_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "gtest/gtest.h"
#include "absl/base/attributes.h"
#include "absl/base/internal/exception_testing.h"
#include "absl/base/internal/iterator_traits_test_helper.h"
#include "absl/base/macros.h"
#include "absl/base/options.h"
#include "absl/container/internal/test_allocator.h"
Expand Down Expand Up @@ -649,6 +650,33 @@ TEST(IntVec, Insert) {
}
}

TEST(IntPairVec, Insert) {
for (size_t len = 0; len < 20; len++) {
for (ptrdiff_t pos = 0; pos <= static_cast<ptrdiff_t>(len); pos++) {
// Iterator range (C++20 forward iterator)
const std::forward_list<int> unzipped_input = {9999, 8888, 7777};
const absl::base_internal::Cpp20ForwardZipIterator<
std::forward_list<int>::const_iterator>
begin(unzipped_input.cbegin(), unzipped_input.cbegin());
const absl::base_internal::Cpp20ForwardZipIterator<
std::forward_list<int>::const_iterator>
end(unzipped_input.cend(), unzipped_input.cend());

std::vector<std::pair<int, int>> std_v;
absl::InlinedVector<std::pair<int, int>, 8> v;
for (size_t i = 0; i < len; ++i) {
std_v.emplace_back(i, i);
v.emplace_back(i, i);
}

std_v.insert(std_v.begin() + pos, begin, end);
auto it = v.insert(v.cbegin() + pos, begin, end);
EXPECT_THAT(v, ElementsAreArray(std_v));
EXPECT_EQ(it, v.cbegin() + pos);
}
}
}

TEST(RefCountedVec, InsertConstructorDestructor) {
// Make sure the proper construction/destruction happen during insert
// operations.
Expand Down

0 comments on commit ae4b0c5

Please sign in to comment.