Reland "Add `CheckedRandomAccess[Const]Iterator`."

This reverts commit 5f80b9a8.

Reason for revert: Revert was wrong; it was OK the first time.

Original change's description:
> Revert "Add `CheckedRandomAccess[Const]Iterator`."
> 
> This reverts commit 383049f2.
> 
> Reason for revert: Landed wrong patchset!
> 
> Original change's description:
> > Add `CheckedRandomAccess[Const]Iterator`.
> > 
> > And add a first call site.
> > 
> > Bug: 817982
> > Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.linux:linux_mojo
> > Change-Id: I2db298494e4f562ffac1f68b87c7a617e592f196
> > Reviewed-on: https://chromium-review.googlesource.com/963726
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Reviewed-by: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#589244}
> 
> TBR=ajwong@chromium.org,palmer@chromium.org,dcheng@chromium.org,jyasskin@chromium.org,mmenke@chromium.org,tsepez@chromium.org,nharper@chromium.org
> 
> Change-Id: Icf99f8409054d853a15b8d27cdb232ff727f8a42
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 817982
> Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.linux:linux_mojo
> Reviewed-on: https://chromium-review.googlesource.com/1211743
> Reviewed-by: Chris Palmer <palmer@chromium.org>
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589245}

TBR=ajwong@chromium.org,palmer@chromium.org,dcheng@chromium.org,jyasskin@chromium.org,mmenke@chromium.org,tsepez@chromium.org,nharper@chromium.org

Change-Id: Ieba0af822a2287583cf37a109538f6ceeb8e4f98
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 817982
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1211902Reviewed-by: Chris Palmer <palmer@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589278}

base/BUILD.gn

@@ -197,6 +197,7 @@ jumbo_component("base") {
     "compiler_specific.h",
     "component_export.h",
     "containers/adapters.h",
+    "containers/checked_iterators.h",
     "containers/circular_deque.h",
     "containers/flat_map.h",
     "containers/flat_set.h",

base/containers/checked_iterators.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_CONTAINERS_CHECKED_ITERATORS_H_
+#define BASE_CONTAINERS_CHECKED_ITERATORS_H_
+
+#include <iterator>
+#include <memory>
+
+#include "base/logging.h"
+
+namespace base {
+
+template <typename T>
+class CheckedRandomAccessConstIterator;
+
+template <typename T>
+class CheckedRandomAccessIterator {
+ public:
+  using difference_type = std::ptrdiff_t;
+  using value_type = typename std::iterator_traits<T*>::value_type;
+  using pointer = T*;
+  using reference = T&;
+  using iterator_category = std::random_access_iterator_tag;
+
+  friend class CheckedRandomAccessConstIterator<T>;
+
+  CheckedRandomAccessIterator() = default;
+  CheckedRandomAccessIterator(T* start, const T* end)
+      : CheckedRandomAccessIterator(start, start, end) {}
+  CheckedRandomAccessIterator(T* start, T* current, const T* end)
+      : start_(start), current_(current), end_(end) {
+    CHECK(start <= current);
+    CHECK(current <= end);
+  }
+  CheckedRandomAccessIterator(const CheckedRandomAccessIterator& other) =
+      default;
+  ~CheckedRandomAccessIterator() = default;
+
+  CheckedRandomAccessIterator& operator=(
+      const CheckedRandomAccessIterator& other) = default;
+
+  bool operator==(const CheckedRandomAccessIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ == other.current_;
+  }
+
+  bool operator!=(const CheckedRandomAccessIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ != other.current_;
+  }
+
+  bool operator<(const CheckedRandomAccessIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ < other.current_;
+  }
+
+  CheckedRandomAccessIterator& operator++() {
+    CHECK(current_ != end_);
+    ++current_;
+    return *this;
+  }
+
+  CheckedRandomAccessIterator operator++(int) {
+    CheckedRandomAccessIterator old = *this;
+    ++*this;
+    return old;
+  }
+
+  CheckedRandomAccessIterator& operator--() {
+    CHECK(current_ != start_);
+    --current_;
+    return *this;
+  }
+
+  CheckedRandomAccessIterator& operator--(int) {
+    CheckedRandomAccessIterator old = *this;
+    --*this;
+    return old;
+  }
+
+  CheckedRandomAccessIterator& operator+=(difference_type rhs) {
+    if (rhs > 0) {
+      CHECK_LE(rhs, end_ - current_);
+    } else {
+      CHECK_LE(-rhs, current_ - start_);
+    }
+    current_ += rhs;
+    return *this;
+  }
+
+  CheckedRandomAccessIterator operator+(difference_type rhs) const {
+    CheckedRandomAccessIterator it = *this;
+    it += rhs;
+    return it;
+  }
+
+  CheckedRandomAccessIterator& operator-=(difference_type rhs) {
+    if (rhs < 0) {
+      CHECK_LE(rhs, end_ - current_);
+    } else {
+      CHECK_LE(-rhs, current_ - start_);
+    }
+    current_ -= rhs;
+    return *this;
+  }
+
+  CheckedRandomAccessIterator operator-(difference_type rhs) const {
+    CheckedRandomAccessIterator it = *this;
+    it -= rhs;
+    return it;
+  }
+
+  friend difference_type operator-(const CheckedRandomAccessIterator& lhs,
+                                   const CheckedRandomAccessIterator& rhs) {
+    CHECK(lhs.start_ == rhs.start_);
+    CHECK(lhs.end_ == rhs.end_);
+    return lhs.current_ - rhs.current_;
+  }
+
+  reference operator*() const {
+    CHECK(current_ != end_);
+    return *current_;
+  }
+
+  pointer operator->() const {
+    CHECK(current_ != end_);
+    return current_;
+  }
+
+ private:
+  const T* start_ = nullptr;
+  T* current_ = nullptr;
+  const T* end_ = nullptr;
+};
+
+template <typename T>
+class CheckedRandomAccessConstIterator {
+ public:
+  using difference_type = std::ptrdiff_t;
+  using value_type = typename std::iterator_traits<T*>::value_type;
+  using pointer = const T*;
+  using reference = const T&;
+  using iterator_category = std::random_access_iterator_tag;
+
+  CheckedRandomAccessConstIterator() = default;
+  CheckedRandomAccessConstIterator(T* start, const T* end)
+      : CheckedRandomAccessConstIterator(start, start, end) {}
+  CheckedRandomAccessConstIterator(T* start, T* current, const T* end)
+      : start_(start), current_(current), end_(end) {
+    CHECK(start <= current);
+    CHECK(current <= end);
+  }
+  CheckedRandomAccessConstIterator(
+      const CheckedRandomAccessConstIterator& other) = default;
+  CheckedRandomAccessConstIterator(const CheckedRandomAccessIterator<T>& other)
+      : start_(other.start_), current_(other.current_), end_(other.end_) {
+    // We explicitly don't delegate to the 3-argument constructor here. Its
+    // CHECKs would be redundant, since we expect |other| to maintain its own
+    // invariant. However, DCHECKs never hurt anybody. Presumably.
+    DCHECK(other.start_ <= other.current_);
+    DCHECK(other.current_ <= other.end_);
+  }
+  ~CheckedRandomAccessConstIterator() = default;
+
+  CheckedRandomAccessConstIterator& operator=(
+      const CheckedRandomAccessConstIterator& other) = default;
+
+  CheckedRandomAccessConstIterator& operator=(
+      CheckedRandomAccessConstIterator& other) = default;
+
+  bool operator==(const CheckedRandomAccessConstIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ == other.current_;
+  }
+
+  bool operator!=(const CheckedRandomAccessConstIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ != other.current_;
+  }
+
+  bool operator<(const CheckedRandomAccessConstIterator& other) const {
+    CHECK_EQ(start_, other.start_);
+    CHECK_EQ(end_, other.end_);
+    return current_ < other.current_;
+  }
+
+  CheckedRandomAccessConstIterator& operator++() {
+    CHECK(current_ != end_);
+    ++current_;
+    return *this;
+  }
+
+  CheckedRandomAccessConstIterator operator++(int) {
+    CheckedRandomAccessConstIterator old = *this;
+    ++*this;
+    return old;
+  }
+
+  CheckedRandomAccessConstIterator& operator--() {
+    CHECK(current_ != start_);
+    --current_;
+    return *this;
+  }
+
+  CheckedRandomAccessConstIterator& operator--(int) {
+    CheckedRandomAccessConstIterator old = *this;
+    --*this;
+    return old;
+  }
+
+  CheckedRandomAccessConstIterator& operator+=(difference_type rhs) {
+    if (rhs > 0) {
+      CHECK_LE(rhs, end_ - current_);
+    } else {
+      CHECK_LE(-rhs, current_ - start_);
+    }
+    current_ += rhs;
+    return *this;
+  }
+
+  CheckedRandomAccessConstIterator operator+(difference_type rhs) const {
+    CheckedRandomAccessConstIterator it = *this;
+    it += rhs;
+    return it;
+  }
+
+  CheckedRandomAccessConstIterator& operator-=(difference_type rhs) {
+    if (rhs < 0) {
+      CHECK_LE(rhs, end_ - current_);
+    } else {
+      CHECK_LE(-rhs, current_ - start_);
+    }
+    current_ -= rhs;
+    return *this;
+  }
+
+  CheckedRandomAccessConstIterator operator-(difference_type rhs) const {
+    CheckedRandomAccessConstIterator it = *this;
+    it -= rhs;
+    return it;
+  }
+
+  friend difference_type operator-(
+      const CheckedRandomAccessConstIterator& lhs,
+      const CheckedRandomAccessConstIterator& rhs) {
+    CHECK(lhs.start_ == rhs.start_);
+    CHECK(lhs.end_ == rhs.end_);
+    return lhs.current_ - rhs.current_;
+  }
+
+  reference operator*() const {
+    CHECK(current_ != end_);
+    return *current_;
+  }
+
+  pointer operator->() const {
+    CHECK(current_ != end_);
+    return current_;
+  }
+
+ private:
+  const T* start_ = nullptr;
+  const T* current_ = nullptr;
+  const T* end_ = nullptr;
+};
+
+}  // namespace base
+
+#endif  // BASE_CONTAINERS_CHECKED_ITERATORS_H_

base/containers/span.h

@@ -13,6 +13,7 @@
 #include <type_traits>
 #include <utility>
 
+#include "base/containers/checked_iterators.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 
@@ -221,8 +222,8 @@ class span : public internal::ExtentStorage<Extent> {
   using difference_type = ptrdiff_t;
   using pointer = T*;
   using reference = T&;
-  using iterator = T*;
-  using const_iterator = const T*;
+  using iterator = CheckedRandomAccessIterator<T>;
+  using const_iterator = CheckedRandomAccessConstIterator<T>;
   using reverse_iterator = std::reverse_iterator<iterator>;
   using const_reverse_iterator = std::reverse_iterator<const_iterator>;
   static constexpr index_type extent = Extent;
@@ -392,8 +393,10 @@ class span : public internal::ExtentStorage<Extent> {
   constexpr T* data() const noexcept { return data_; }
 
   // [span.iter], span iterator support
-  constexpr iterator begin() const noexcept { return data(); }
-  constexpr iterator end() const noexcept { return data() + size(); }
+  iterator begin() const noexcept { return iterator(data_, data_ + size()); }
+  iterator end() const noexcept {
+    return iterator(data_, data_ + size(), data_ + size());
+  }
 
   constexpr const_iterator cbegin() const noexcept { return begin(); }
   constexpr const_iterator cend() const noexcept { return end(); }

base/containers/span_unittest.cc

@@ -236,6 +236,8 @@ TEST(SpanTest, ConvertNonConstIntegralToConst) {
 
   span<int> int_span(vector.data(), vector.size());
   span<const int> const_span(int_span);
+  EXPECT_EQ(int_span.size(), const_span.size());
+
   EXPECT_THAT(const_span, Pointwise(Eq(), int_span));
 
   span<int, 6> static_int_span(vector.data(), vector.size());

base/values.cc

@@ -374,12 +374,12 @@ Value* Value::SetPath(std::initializer_list<StringPiece> path, Value value) {
 }
 
 Value* Value::SetPath(span<const StringPiece> path, Value value) {
-  DCHECK_NE(path.begin(), path.end());  // Can't be empty path.
+  DCHECK(path.begin() != path.end());  // Can't be empty path.
 
   // Walk/construct intermediate dictionaries. The last element requires
   // special handling so skip it in this loop.
   Value* cur = this;
-  const StringPiece* cur_path = path.begin();
+  auto cur_path = path.begin();
   for (; (cur_path + 1) < path.end(); ++cur_path) {
     if (!cur->is_dict())
       return nullptr;

net/socket/socket_test_util.cc

@@ -352,8 +352,8 @@ SequencedSocketData::SequencedSocketData(base::span<const MockRead> reads,
   int next_sequence_number = 0;
   bool last_event_was_pause = false;
 
-  auto* next_read = reads.begin();
-  auto* next_write = writes.begin();
+  auto next_read = reads.begin();
+  auto next_write = writes.begin();
   while (next_read != reads.end() || next_write != writes.end()) {
     if (next_read != reads.end() &&
         next_read->sequence_number == next_sequence_number) {
@@ -407,8 +407,8 @@ SequencedSocketData::SequencedSocketData(base::span<const MockRead> reads,
   // ERR_IO_PENDING.
   CHECK(!last_event_was_pause);
 
-  CHECK_EQ(next_read, reads.end());
-  CHECK_EQ(next_write, writes.end());
+  CHECK(next_read == reads.end());
+  CHECK(next_write == writes.end());
 }
 
 SequencedSocketData::SequencedSocketData(const MockConnect& connect,

services/network/udp_socket.cc

@@ -362,7 +362,7 @@ void UDPSocket::DoSendToOrWrite(
   // |data| points to a range of bytes in the received message and will be
   // freed when this method returns, so copy out the bytes now.
   auto buffer = base::MakeRefCounted<net::IOBufferWithSize>(data.size());
-  memcpy(buffer.get()->data(), data.begin(), data.size());
+  memcpy(buffer.get()->data(), data.data(), data.size());
 
   if (send_buffer_.get()) {
     auto request = std::make_unique<PendingSendRequest>();