Revert "[base] Add missing methods to base::span"

This reverts commit 82d9545a.

Reason for revert: compile failures on ios-simulator-xcode-clang and ios-device-xcode-clang builders:
 * https://uberchromegw.corp.google.com/i/chromium.mac/builders/ios-simulator-xcode-clang/builds/46343
 * https://uberchromegw.corp.google.com/i/chromium.mac/builders/ios-device-xcode-clang/builds/57515

Original change's description:
> [base] Add missing methods to base::span
> 
> This change implements constructors for base::span that allow the
> construction from a pair of pointers, std::arrays and containers
> supporting base::data and base::size, e.g. std::initializer_list.
> In addition, this change adds operator().
> 
> Bug: 788913
> Change-Id: Ibc280eef1c7e47a5a27e92503dda3614ef5513a4
> Reviewed-on: https://chromium-review.googlesource.com/981139
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551313}

TBR=dcheng@chromium.org,jdoerrie@chromium.org

Change-Id: Ia967227caa3a24a10df2ab4c5fa4ee03dddf0f29
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 788913
Reviewed-on: https://chromium-review.googlesource.com/1014761Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551329}

base/containers/span.h

@@ -14,7 +14,6 @@
 #include <utility>
 
 #include "base/logging.h"
-#include "base/stl_util.h"
 
 namespace base {
 
@@ -41,36 +40,45 @@ struct IsStdArrayImpl<std::array<T, N>> : std::true_type {};
 template <typename T>
 using IsStdArray = IsStdArrayImpl<std::decay_t<T>>;
 
-template <typename T>
-using IsCArray = std::is_array<std::remove_reference_t<T>>;
-
 template <typename From, typename To>
-using IsLegalSpanConversion = std::is_convertible<From (*)[], To (*)[]>;
+using IsLegalSpanConversion = std::is_convertible<From*, To*>;
 
 template <typename Container, typename T>
 using ContainerHasConvertibleData = IsLegalSpanConversion<
-    std::remove_pointer_t<decltype(base::data(std::declval<Container>()))>,
+    std::remove_pointer_t<decltype(std::declval<Container>().data())>,
     T>;
-
 template <typename Container>
 using ContainerHasIntegralSize =
-    std::is_integral<decltype(base::size(std::declval<Container>()))>;
+    std::is_integral<decltype(std::declval<Container>().size())>;
 
 template <typename From, typename To>
 using EnableIfLegalSpanConversion =
     std::enable_if_t<IsLegalSpanConversion<From, To>::value>;
 
-// SFINAE check if Array can be converted to a span<T>.
-template <typename Array, typename T>
-using EnableIfSpanCompatibleArray =
-    std::enable_if_t<ContainerHasConvertibleData<Array, T>::value>;
-
-// SFINAE check if Container can be converted to a span<T>.
+// SFINAE check if Container can be converted to a span<T>. Note that the
+// implementation details of this check differ slightly from the requirements in
+// the working group proposal: in particular, the proposal also requires that
+// the container conversion constructor participate in overload resolution only
+// if two additional conditions are true:
+//
+//   1. Container implements operator[].
+//   2. Container::value_type matches remove_const_t<element_type>.
+//
+// The requirements are relaxed slightly here: in particular, not requiring (2)
+// means that an immutable span can be easily constructed from a mutable
+// container.
 template <typename Container, typename T>
 using EnableIfSpanCompatibleContainer =
     std::enable_if_t<!internal::IsSpan<Container>::value &&
                      !internal::IsStdArray<Container>::value &&
-                     !internal::IsCArray<Container>::value &&
+                     ContainerHasConvertibleData<Container, T>::value &&
+                     ContainerHasIntegralSize<Container>::value>;
+
+template <typename Container, typename T>
+using EnableIfConstSpanCompatibleContainer =
+    std::enable_if_t<std::is_const<T>::value &&
+                     !internal::IsSpan<Container>::value &&
+                     !internal::IsStdArray<Container>::value &&
                      ContainerHasConvertibleData<Container, T>::value &&
                      ContainerHasIntegralSize<Container>::value>;
 
@@ -125,7 +133,7 @@ using EnableIfSpanCompatibleContainer =
 // -------------------------------
 //
 // Const and pointers can get confusing. Here are vectors of pointers and their
-// corresponding spans:
+// corresponding spans (you can always make the span "more const" too):
 //
 //   const std::vector<int*>        =>  base::span<int* const>
 //   std::vector<const int*>        =>  base::span<const int*>
@@ -135,7 +143,7 @@ using EnableIfSpanCompatibleContainer =
 // -------------------------------------------
 //
 // https://wg21.link/P0122 is the latest working group proposal, Chromium
-// currently implements R7. The biggest difference is span does not support a
+// currently implements R6. The biggest difference is span does not support a
 // static extent template parameter. Other differences are documented in
 // subsections below.
 //
@@ -143,7 +151,7 @@ using EnableIfSpanCompatibleContainer =
 // - no dynamic_extent constant
 //
 // Differences from [span.objectrep]:
-// - as_bytes() and as_writable_bytes() return spans of uint8_t instead of
+// - as_bytes() and as_writeable_bytes() return spans of uint8_t instead of
 //   std::byte
 //
 // Differences in constants and types:
@@ -152,6 +160,10 @@ using EnableIfSpanCompatibleContainer =
 // - no different_type type alias
 // - no extent constant
 //
+// Differences from [span.cons]:
+// - no constructor from a pointer range
+// - no constructor from std::array
+//
 // Differences from [span.sub]:
 // - no templated first()
 // - no templated last()
@@ -162,6 +174,7 @@ using EnableIfSpanCompatibleContainer =
 // - using size_t instead of ptrdiff_t to represent size()
 //
 // Differences from [span.elem]:
+// - no operator ()()
 // - using size_t instead of ptrdiff_t for indexing
 
 // [span], class template span
@@ -179,39 +192,20 @@ class span {
   // [span.cons], span constructors, copy, assignment, and destructor
   constexpr span() noexcept : data_(nullptr), size_(0) {}
   constexpr span(T* data, size_t size) noexcept : data_(data), size_(size) {}
-  // Artificially templatized to break ambiguity for span(ptr, 0).
-  template <typename = void>
-  constexpr span(T* begin, T* end)
-      : data_(begin), size_(std::distance(begin, end)) {
-    CHECK_LE(begin, end);
-  }
-  template <size_t N,
-            typename = internal::EnableIfSpanCompatibleArray<T (&)[N], T>>
+  // TODO(dcheng): Implement construction from a |begin| and |end| pointer.
+  template <size_t N>
   constexpr span(T (&array)[N]) noexcept : span(array, N) {}
-
-  template <
-      size_t N,
-      typename =
-          internal::EnableIfSpanCompatibleArray<std::array<value_type, N>&, T>>
-  constexpr span(std::array<value_type, N>& array) noexcept
-      : span(array.data(), array.size()) {}
-  template <size_t N,
-            typename = internal::EnableIfSpanCompatibleArray<
-                const std::array<value_type, N>&,
-                T>>
-  constexpr span(const std::array<value_type, N>& array) noexcept
-      : span(array.data(), array.size()) {}
+  // TODO(dcheng): Implement construction from std::array.
   // Conversion from a container that provides |T* data()| and |integral_type
   // size()|.
   template <typename Container,
-            typename = internal::EnableIfSpanCompatibleContainer<Container&, T>>
+            typename = internal::EnableIfSpanCompatibleContainer<Container, T>>
   constexpr span(Container& container)
-      : span(base::data(container), container.size()) {}
+      : span(container.data(), container.size()) {}
   template <
       typename Container,
-      typename = internal::EnableIfSpanCompatibleContainer<const Container&, T>>
-  span(const Container& container)
-      : span(base::data(container), container.size()) {}
+      typename = internal::EnableIfConstSpanCompatibleContainer<Container, T>>
+  span(const Container& container) : span(container.data(), container.size()) {}
   constexpr span(const span& other) noexcept = default;
   // Conversions from spans of compatible types: this allows a span<T> to be
   // seamlessly used as a span<const T>, but not the other way around.
@@ -248,10 +242,6 @@ class span {
     CHECK(index < size_);
     return data_[index];
   }
-  constexpr T& operator()(size_t index) const noexcept {
-    CHECK(index < size_);
-    return data_[index];
-  }
   constexpr T* data() const noexcept { return data_; }
 
   // [span.iter], span iterator support
@@ -330,37 +320,22 @@ constexpr span<T> make_span(T* data, size_t size) noexcept {
   return span<T>(data, size);
 }
 
-template <typename T>
-constexpr span<T> make_span(T* begin, T* end) {
-  return span<T>(begin, end);
-}
-
 template <typename T, size_t N>
 constexpr span<T> make_span(T (&array)[N]) noexcept {
   return span<T>(array);
 }
 
-template <typename T, size_t N>
-constexpr span<T> make_span(std::array<T, N>& array) noexcept {
-  return span<T>(array);
-}
-
-template <typename T, size_t N>
-constexpr span<const T> make_span(const std::array<T, N>& array) noexcept {
-  return span<const T>(array);
-}
-
 template <typename Container,
           typename T = typename Container::value_type,
-          typename = internal::EnableIfSpanCompatibleContainer<Container&, T>>
+          typename = internal::EnableIfSpanCompatibleContainer<Container, T>>
 constexpr span<T> make_span(Container& container) {
   return span<T>(container);
 }
 
 template <
     typename Container,
-    typename T = const typename Container::value_type,
-    typename = internal::EnableIfSpanCompatibleContainer<const Container&, T>>
+    typename T = std::add_const_t<typename Container::value_type>,
+    typename = internal::EnableIfConstSpanCompatibleContainer<Container, T>>
 constexpr span<T> make_span(const Container& container) {
   return span<T>(container);
 }

base/containers/span_unittest.cc

@@ -8,7 +8,6 @@
 
 #include <algorithm>
 #include <memory>
-#include <string>
 #include <vector>
 
 #include "base/macros.h"
@@ -28,41 +27,14 @@ TEST(SpanTest, DefaultConstructor) {
 }
 
 TEST(SpanTest, ConstructFromDataAndSize) {
-  {
-    span<int> empty_span(nullptr, 0);
-    EXPECT_TRUE(empty_span.empty());
-    EXPECT_EQ(nullptr, empty_span.data());
-  }
-
-  {
-    std::vector<int> vector = {1, 1, 2, 3, 5, 8};
-
-    span<int> span(vector.data(), vector.size());
-    EXPECT_EQ(vector.data(), span.data());
-    EXPECT_EQ(vector.size(), span.size());
-
-    for (size_t i = 0; i < span.size(); ++i)
-      EXPECT_EQ(vector[i], span[i]);
-  }
-}
-
-TEST(SpanTest, ConstructFromPointerPair) {
-  {
-    span<int> empty_span(nullptr, nullptr);
-    EXPECT_TRUE(empty_span.empty());
-    EXPECT_EQ(nullptr, empty_span.data());
-  }
-
-  {
-    std::vector<int> vector = {1, 1, 2, 3, 5, 8};
+  std::vector<int> vector = {1, 1, 2, 3, 5, 8};
 
-    span<int> span(vector.data(), vector.data() + vector.size() / 2);
-    EXPECT_EQ(vector.data(), span.data());
-    EXPECT_EQ(vector.size() / 2, span.size());
+  span<int> span(vector.data(), vector.size());
+  EXPECT_EQ(vector.data(), span.data());
+  EXPECT_EQ(vector.size(), span.size());
 
-    for (size_t i = 0; i < span.size(); ++i)
-      EXPECT_EQ(vector[i], span[i]);
-  }
+  for (size_t i = 0; i < span.size(); ++i)
+    EXPECT_EQ(vector[i], span[i]);
 }
 
 TEST(SpanTest, ConstructFromConstexprArray) {
@@ -92,53 +64,6 @@ TEST(SpanTest, ConstructFromArray) {
     EXPECT_EQ(array[i], span[i]);
 }
 
-TEST(SpanTest, ConstructFromStdArray) {
-  // Note: Constructing a constexpr span from a constexpr std::array does not
-  // work prior to C++17 due to non-constexpr std::array::data.
-  std::array<int, 5> array = {5, 4, 3, 2, 1};
-
-  span<const int> const_span(array);
-  EXPECT_EQ(array.data(), const_span.data());
-  EXPECT_EQ(array.size(), const_span.size());
-  for (size_t i = 0; i < const_span.size(); ++i)
-    EXPECT_EQ(array[i], const_span[i]);
-
-  span<int> span(array);
-  EXPECT_EQ(array.data(), span.data());
-  EXPECT_EQ(array.size(), span.size());
-  for (size_t i = 0; i < span.size(); ++i)
-    EXPECT_EQ(array[i], span[i]);
-}
-
-TEST(SpanTest, ConstructFromInitializerList) {
-  std::initializer_list<int> il = {1, 1, 2, 3, 5, 8};
-
-  span<const int> const_span(il);
-  EXPECT_EQ(il.begin(), const_span.data());
-  EXPECT_EQ(il.size(), const_span.size());
-
-  for (size_t i = 0; i < const_span.size(); ++i)
-    EXPECT_EQ(il.begin()[i], const_span[i]);
-}
-
-TEST(SpanTest, ConstructFromStdString) {
-  std::string str = "foobar";
-
-  span<const char> const_span(str);
-  EXPECT_EQ(str.data(), const_span.data());
-  EXPECT_EQ(str.size(), const_span.size());
-
-  for (size_t i = 0; i < const_span.size(); ++i)
-    EXPECT_EQ(str[i], const_span[i]);
-
-  span<char> span(str);
-  EXPECT_EQ(str.data(), span.data());
-  EXPECT_EQ(str.size(), span.size());
-
-  for (size_t i = 0; i < span.size(); ++i)
-    EXPECT_EQ(str[i], span[i]);
-}
-
 TEST(SpanTest, ConstructFromConstContainer) {
   const std::vector<int> vector = {1, 1, 2, 3, 5, 8};
 
@@ -188,10 +113,10 @@ TEST(SpanTest, ConvertNonConstPointerToConst) {
   EXPECT_THAT(const_pointer_span, Pointwise(Eq(), non_const_pointer_span));
   // Note: no test for conversion from span<int> to span<const int*>, since that
   // would imply a conversion from int** to const int**, which is unsafe.
-  //
-  // Note: no test for conversion from span<int*> to span<const int* const>,
-  // due to CWG Defect 330:
-  // http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#330
+  span<const int* const> const_pointer_to_const_data_span(
+      non_const_pointer_span);
+  EXPECT_THAT(const_pointer_to_const_data_span,
+              Pointwise(Eq(), non_const_pointer_span));
 }
 
 TEST(SpanTest, ConvertBetweenEquivalentTypes) {
@@ -410,23 +335,6 @@ TEST(SpanTest, Empty) {
   }
 }
 
-TEST(SpanTest, OperatorAt) {
-  static constexpr int kArray[] = {1, 6, 1, 8, 0};
-  constexpr span<const int> span(kArray);
-
-  static_assert(kArray[0] == span[0], "span[0] does not equal kArray[0]");
-  static_assert(kArray[1] == span[1], "span[1] does not equal kArray[1]");
-  static_assert(kArray[2] == span[2], "span[2] does not equal kArray[2]");
-  static_assert(kArray[3] == span[3], "span[3] does not equal kArray[3]");
-  static_assert(kArray[4] == span[4], "span[4] does not equal kArray[4]");
-
-  static_assert(kArray[0] == span(0), "span(0) does not equal kArray[0]");
-  static_assert(kArray[1] == span(1), "span(1) does not equal kArray[1]");
-  static_assert(kArray[2] == span(2), "span(2) does not equal kArray[2]");
-  static_assert(kArray[3] == span(3), "span(3) does not equal kArray[3]");
-  static_assert(kArray[4] == span(4), "span(4) does not equal kArray[4]");
-}
-
 TEST(SpanTest, Iterator) {
   static constexpr int kArray[] = {1, 6, 1, 8, 0};
   constexpr span<const int> span(kArray);
@@ -597,39 +505,17 @@ TEST(SpanTest, AsWritableBytes) {
 }
 
 TEST(SpanTest, MakeSpanFromDataAndSize) {
-  int* nullint = nullptr;
-  auto empty_span = make_span(nullint, 0);
-  EXPECT_TRUE(empty_span.empty());
-  EXPECT_EQ(nullptr, empty_span.data());
-
   std::vector<int> vector = {1, 1, 2, 3, 5, 8};
   span<int> span(vector.data(), vector.size());
   EXPECT_EQ(span, make_span(vector.data(), vector.size()));
 }
 
-TEST(SpanTest, MakeSpanFromPointerPair) {
-  int* nullint = nullptr;
-  auto empty_span = make_span(nullint, nullint);
-  EXPECT_TRUE(empty_span.empty());
-  EXPECT_EQ(nullptr, empty_span.data());
-
-  std::vector<int> vector = {1, 1, 2, 3, 5, 8};
-  span<int> span(vector.data(), vector.size());
-  EXPECT_EQ(span, make_span(vector.data(), vector.data() + vector.size()));
-}
-
 TEST(SpanTest, MakeSpanFromConstexprArray) {
   static constexpr int kArray[] = {1, 2, 3, 4, 5};
   constexpr span<const int> span(kArray);
   EXPECT_EQ(span, make_span(kArray));
 }
 
-TEST(SpanTest, MakeSpanFromStdArray) {
-  const std::array<int, 5> kArray = {1, 2, 3, 4, 5};
-  span<const int> span(kArray);
-  EXPECT_EQ(span, make_span(kArray));
-}
-
 TEST(SpanTest, MakeSpanFromConstContainer) {
   const std::vector<int> vector = {-1, -2, -3, -4, -5};
   span<const int> span(vector);

base/containers/span_unittest.nc

@@ -42,6 +42,14 @@ void WontCompile() {
   span<const int*> const_span(non_const_span);
 }
 
+#elif defined(NCTEST_STD_ARRAY_CONVERSION_DISALLOWED)  // [r"fatal error: no matching constructor for initialization of 'span<int>'"]
+
+// This isn't implemented today. Maybe it will be some day.
+void WontCompile() {
+  std::array<int, 3> array;
+  span<int> span(array);
+}
+
 #elif defined(NCTEST_CONST_CONTAINER_TO_MUTABLE_CONVERSION_DISALLOWED)  // [r"fatal error: no matching constructor for initialization of 'span<int>'"]
 
 // A const container should not be convertible to a mutable span.