Slight span helper improvements.

* Use CTAD rather than `make_span()` in some span helpers.
* Combine two `make_span()` overrides into one that better matches the
  actual span constructors, to make further removal more mechanical.
* Add a few constraints that were already checked by underlying helpers,
  but not by those that called them; won't change what compiles, but
  hopefully will make errors give the underlying issue sooner.
* Stop supporting rvalue arrays for as_writable_byte_span(). It doesn't
  make much sense to want a writable span created from an rvalue, and we
  don't really support it elsewhere.
* Fix oversight in SpanReader deduction guide that didn't support
  raw_spans.

Bug: 364987728
Change-Id: If8b7f151fee4b5416cce8f3d6e66e83c30bf1741
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5962112
Auto-Submit: Peter Kasting <[email protected]>
Commit-Queue: Peter Kasting <[email protected]>
Reviewed-by: danakj <[email protected]>
Code-Coverage: [email protected] <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1375433}


CrOS-Libchrome-Original-Commit: 866b397d72d22bbc587896fd98c9a281cc915580

Author: pkasting

base/containers/span.h

@@ -74,6 +74,14 @@ concept LegalDataConversion =
     std::is_convertible_v<std::remove_reference_t<From> (*)[],
                           std::remove_reference_t<To> (*)[]>;
 
+// Akin to `std::constructible_from<span, T>`, but meant to be used in a
+// type-deducing context where we don't know what args would be deduced;
+// `std::constructible_from` can't be directly used in such a case since the
+// type parameters must be fully-specified (e.g. `span<int>`), requiring us to
+// have that knowledge already.
+template <typename T>
+concept SpanConstructibleFrom = requires(const T& t) { span(t); };
+
 template <typename T, typename It>
 concept CompatibleIter = std::contiguous_iterator<It> &&
                          LegalDataConversion<std::iter_reference_t<It>, T>;
@@ -1488,42 +1496,20 @@ auto as_writable_chars(span<T, X, InternalPtrType> s) noexcept {
       span<char, N>(reinterpret_cast<char*>(s.data()), s.size_bytes()));
 }
 
-// Type-deducing helper for constructing a span.
-//
-// # Safety
-// The contiguous iterator `it` must point to the first element of at least
-// `size` many elements or Undefined Behaviour may result as the span may give
-// access beyond the bounds of the collection pointed to by `it`.
-template <int&... ExplicitArgumentBarrier, typename It>
-UNSAFE_BUFFER_USAGE constexpr auto make_span(
-    It it,
-    StrictNumeric<size_t> size) noexcept {
-  using T = std::remove_reference_t<std::iter_reference_t<It>>;
-  // SAFETY: The caller guarantees that `it` is the first of at least `size`
-  // many elements.
-  return UNSAFE_BUFFERS(span<T>(it, size));
-}
-
 // Type-deducing helper for constructing a span.
 // Deprecated: Use CTAD (i.e. use `span()` directly without template arguments).
 // TODO(crbug.com/341907909): Remove.
 //
-// # Checks
-// The function CHECKs that `it <= end` and will terminate otherwise.
-//
-// # Safety
-// The contiguous iterator `it` and its end sentinel `end` must be for the same
-// allocation or Undefined Behaviour may result as the span may give access
-// beyond the bounds of the collection pointed to by `it`.
-template <int&... ExplicitArgumentBarrier,
-          typename It,
-          typename End,
-          typename = std::enable_if_t<!std::is_convertible_v<End, size_t>>>
-UNSAFE_BUFFER_USAGE constexpr auto make_span(It it, End end) noexcept {
-  using T = std::remove_reference_t<std::iter_reference_t<It>>;
-  // SAFETY: The caller guarantees that `it` and `end` are iterators of the
-  // same allocation.
-  return UNSAFE_BUFFERS(span<T>(it, end));
+// SAFETY: `it` must point to the first of a (possibly-empty) series of
+// contiguous valid elements. If `end_or_size` is a size, the series must
+// contain at least that many valid elements; if it is an iterator or sentinel,
+// it must refer to the same allocation, and all elements in the range [it,
+// end_or_size) must be valid. Otherwise, the span will allow access to invalid
+// elements, resulting in UB.
+template <int&... ExplicitArgumentBarrier, typename It, typename EndOrSize>
+  requires(std::contiguous_iterator<It>)
+UNSAFE_BUFFER_USAGE constexpr auto make_span(It it, EndOrSize end_or_size) {
+  return UNSAFE_BUFFERS(span(it, end_or_size));
 }
 
 // make_span utility function that deduces both the span's value_type and extent
@@ -1533,6 +1519,7 @@ UNSAFE_BUFFER_USAGE constexpr auto make_span(It it, End end) noexcept {
 // Deprecated: Use CTAD (i.e. use `span()` directly without template arguments).
 // TODO(crbug.com/341907909): Remove.
 template <int&... ExplicitArgumentBarrier, typename Container>
+  requires(internal::SpanConstructibleFrom<Container>)
 constexpr auto make_span(Container&& container) noexcept {
   return span(std::forward<Container>(container));
 }
@@ -1664,9 +1651,9 @@ constexpr span<const uint8_t, N> byte_span_with_nul_from_cstring(
 // or vector-like objects holding other scalar types, prior to passing them
 // into an API that requires byte spans.
 template <int&... ExplicitArgumentBarrier, typename Spannable>
-  requires requires(const Spannable& arg) { make_span(arg); }
+  requires(internal::SpanConstructibleFrom<Spannable>)
 constexpr auto as_byte_span(const Spannable& arg) {
-  return as_bytes(make_span(arg));
+  return as_bytes(span(arg));
 }
 
 template <int&... ExplicitArgumentBarrier, typename T, size_t N>
@@ -1681,26 +1668,20 @@ constexpr span<const uint8_t, N * sizeof(T)> as_byte_span(
 // or vector-like objects holding other scalar types, prior to passing them
 // into an API that requires mutable byte spans.
 template <int&... ExplicitArgumentBarrier, typename Spannable>
-  requires requires(Spannable&& arg) {
-    make_span(arg);
-    requires !std::is_const_v<typename decltype(make_span(arg))::element_type>;
-  }
+  requires(internal::SpanConstructibleFrom<Spannable> &&
+           !std::is_const_v<typename decltype(span(
+               std::declval<Spannable>()))::element_type>)
 constexpr auto as_writable_byte_span(Spannable&& arg) {
-  return as_writable_bytes(make_span(std::forward<Spannable>(arg)));
+  return as_writable_bytes(span(std::forward<Spannable>(arg)));
 }
 
 // This overload for arrays preserves the compile-time size N of the array in
 // the span type signature span<uint8_t, N>.
 template <int&... ExplicitArgumentBarrier, typename T, size_t N>
+  requires(!std::is_const_v<T>)
 constexpr span<uint8_t, N * sizeof(T)> as_writable_byte_span(
     T (&arr LIFETIME_BOUND)[N]) {
-  return as_writable_bytes(make_span(arr));
-}
-
-template <int&... ExplicitArgumentBarrier, typename T, size_t N>
-constexpr span<uint8_t, N * sizeof(T)> as_writable_byte_span(
-    T (&&arr LIFETIME_BOUND)[N]) {
-  return as_writable_bytes(make_span(arr));
+  return as_writable_bytes(span(arr));
 }
 
 namespace internal {

base/containers/span_nocompile.nc

@@ -93,9 +93,9 @@ void StdSetConversionDisallowed() {
   span<int> span1(set.begin(), 0u);                // expected-error {{no matching constructor for initialization of 'span<int>'}}
   span<int> span2(set.begin(), set.end());         // expected-error {{no matching constructor for initialization of 'span<int>'}}
   span<int> span3(set);                            // expected-error {{no matching constructor for initialization of 'span<int>'}}
-  auto span4 = make_span(set.begin(), 0u);         // expected-error@*:* {{no matching constructor for initialization of 'span<T>' (aka 'span<const int>')}}
-  auto span5 = make_span(set.begin(), set.end());  // expected-error@*:* {{no matching constructor for initialization of 'span<T>' (aka 'span<const int>')}}
-  auto span6 = make_span(set);                     // expected-error@*:* {{no viable constructor or deduction guide for deduction of template arguments of 'span'}}
+  auto span4 = make_span(set.begin(), 0u);         // expected-error@*:* {{no matching function for call to 'make_span'}}
+  auto span5 = make_span(set.begin(), set.end());  // expected-error@*:* {{no matching function for call to 'make_span'}}
+  auto span6 = make_span(set);                     // expected-error@*:* {{no matching function for call to 'make_span'}}
 }
 
 // Static views of spans with static extent must not exceed the size.

base/containers/span_reader.h

@@ -281,8 +281,8 @@ class SpanReader {
   size_t original_size_;
 };
 
-template <class T, size_t N>
-SpanReader(span<T, N>) -> SpanReader<T>;
+template <typename T, size_t N, typename InternalPtrType>
+SpanReader(span<T, N, InternalPtrType>) -> SpanReader<T>;
 
 }  // namespace base
 

base/containers/span_unittest.cc

@@ -1940,15 +1940,16 @@ TEST(SpanTest, AsWritableByteSpan) {
     EXPECT_EQ(byte_span.data(), reinterpret_cast<uint8_t*>(kMutVec.data()));
     EXPECT_EQ(byte_span.size(), kMutVec.size() * sizeof(int));
   }
-  // Rvalue input.
+  // Result can be passed as rvalue.
   {
+    int kMutArray[] = {2, 3, 5, 7, 11, 13};
     [](auto byte_span) {
       static_assert(
           std::is_same_v<decltype(byte_span), span<uint8_t, 6u * sizeof(int)>>);
       EXPECT_EQ(byte_span.size(), 6u * sizeof(int));
       // Little endian puts the low bits in the first byte.
       EXPECT_EQ(byte_span[0u], 2);
-    }(as_writable_byte_span({2, 3, 5, 7, 11, 13}));
+    }(as_writable_byte_span(kMutArray));
   }
 }