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

This is a reland of 82d9545a

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

Bug: 788913
Change-Id: I2e02879ddd2839ce0c26c8b18e565d555e08cc7b
Reviewed-on: https://chromium-review.googlesource.com/1013468Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551627}

base/containers/span.h

@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "base/logging.h"
+#include "base/stl_util.h"
 
 namespace base {
 
@@ -40,45 +41,36 @@ 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(std::declval<Container>().data())>,
+    std::remove_pointer_t<decltype(base::data(std::declval<Container>()))>,
     T>;
+
 template <typename Container>
 using ContainerHasIntegralSize =
-    std::is_integral<decltype(std::declval<Container>().size())>;
+    std::is_integral<decltype(base::size(std::declval<Container>()))>;
 
 template <typename From, typename To>
 using EnableIfLegalSpanConversion =
     std::enable_if_t<IsLegalSpanConversion<From, To>::value>;
 
-// 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.
+// 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>.
 template <typename Container, typename T>
 using EnableIfSpanCompatibleContainer =
     std::enable_if_t<!internal::IsSpan<Container>::value &&
                      !internal::IsStdArray<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 &&
+                     !internal::IsCArray<Container>::value &&
                      ContainerHasConvertibleData<Container, T>::value &&
                      ContainerHasIntegralSize<Container>::value>;
 
@@ -133,7 +125,7 @@ using EnableIfConstSpanCompatibleContainer =
 // -------------------------------
 //
 // Const and pointers can get confusing. Here are vectors of pointers and their
-// corresponding spans (you can always make the span "more const" too):
+// corresponding spans:
 //
 //   const std::vector<int*>        =>  base::span<int* const>
 //   std::vector<const int*>        =>  base::span<const int*>
@@ -143,7 +135,7 @@ using EnableIfConstSpanCompatibleContainer =
 // -------------------------------------------
 //
 // https://wg21.link/P0122 is the latest working group proposal, Chromium
-// currently implements R6. The biggest difference is span does not support a
+// currently implements R7. The biggest difference is span does not support a
 // static extent template parameter. Other differences are documented in
 // subsections below.
 //
@@ -151,7 +143,7 @@ using EnableIfConstSpanCompatibleContainer =
 // - no dynamic_extent constant
 //
 // Differences from [span.objectrep]:
-// - as_bytes() and as_writeable_bytes() return spans of uint8_t instead of
+// - as_bytes() and as_writable_bytes() return spans of uint8_t instead of
 //   std::byte
 //
 // Differences in constants and types:
@@ -160,10 +152,6 @@ using EnableIfConstSpanCompatibleContainer =
 // - 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()
@@ -174,7 +162,6 @@ using EnableIfConstSpanCompatibleContainer =
 // - 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
@@ -192,20 +179,39 @@ 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) {}
-  // TODO(dcheng): Implement construction from a |begin| and |end| pointer.
-  template <size_t N>
+  // 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>>
   constexpr span(T (&array)[N]) noexcept : span(array, N) {}
-  // TODO(dcheng): Implement construction from std::array.
+
+  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()) {}
   // 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(container.data(), container.size()) {}
+      : span(base::data(container), container.size()) {}
   template <
       typename Container,
-      typename = internal::EnableIfConstSpanCompatibleContainer<Container, T>>
-  span(const Container& container) : span(container.data(), container.size()) {}
+      typename = internal::EnableIfSpanCompatibleContainer<const Container&, T>>
+  span(const Container& container)
+      : span(base::data(container), 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.
@@ -242,6 +248,10 @@ 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
@@ -320,22 +330,37 @@ 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 = std::add_const_t<typename Container::value_type>,
-    typename = internal::EnableIfConstSpanCompatibleContainer<Container, T>>
+    typename T = const typename Container::value_type,
+    typename = internal::EnableIfSpanCompatibleContainer<const Container&, T>>
 constexpr span<T> make_span(const Container& container) {
   return span<T>(container);
 }

base/containers/span_unittest.cc

@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
@@ -27,14 +28,41 @@ TEST(SpanTest, DefaultConstructor) {
 }
 
 TEST(SpanTest, ConstructFromDataAndSize) {
-  std::vector<int> vector = {1, 1, 2, 3, 5, 8};
+  {
+    span<int> empty_span(nullptr, 0);
+    EXPECT_TRUE(empty_span.empty());
+    EXPECT_EQ(nullptr, empty_span.data());
+  }
 
-  span<int> span(vector.data(), vector.size());
-  EXPECT_EQ(vector.data(), span.data());
-  EXPECT_EQ(vector.size(), span.size());
+  {
+    std::vector<int> vector = {1, 1, 2, 3, 5, 8};
 
-  for (size_t i = 0; i < span.size(); ++i)
-    EXPECT_EQ(vector[i], span[i]);
+    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};
+
+    span<int> span(vector.data(), vector.data() + vector.size() / 2);
+    EXPECT_EQ(vector.data(), span.data());
+    EXPECT_EQ(vector.size() / 2, span.size());
+
+    for (size_t i = 0; i < span.size(); ++i)
+      EXPECT_EQ(vector[i], span[i]);
+  }
 }
 
 TEST(SpanTest, ConstructFromConstexprArray) {
@@ -64,6 +92,53 @@ 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};
 
@@ -113,10 +188,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.
-  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));
+  //
+  // 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
 }
 
 TEST(SpanTest, ConvertBetweenEquivalentTypes) {
@@ -335,6 +410,23 @@ 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);
@@ -505,17 +597,39 @@ 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,14 +42,6 @@ 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.