Implement `span::to_fixed_extent<N>(...)` to replace `make_span<N>(...)`
`span::to_fixed_extent<N>()` is a helper that converts a dynamic-extent
span to a fixed-extent span at runtime. In theory, this isn't needed
since there are already several ways to do this conversion:
- `first<N>(...)`, `last<N>(...)`, and `subspan<0, N>(...)` all extract
a fixed-extent span from a dynamic-extent span. However, none of these
express the intent "this span is exactly size N" without the caller
writing additional checks.
- `make_span<N>(...)` makes a runtime-checked fixed-extent span from a
container with dynamic length. However:
- In general, the `make_span<...>(...)` family of functions will
migrate to CTAD in the future.
- As a free function, it can be used with any type of spannable
object even when it's not necessary.
- Using an explicit `span<T, N>` construction. However, this requires
writing `T` again, which can sometimes be clunky.
The introduction of this helper allows the removal of
`as_byte_span<N>()` and `as_writable_byte_span<N>()`, and will
facilitate the future removal of `make_span<N>()`.
Finally, remove redundant constraints from `as_byte_span()` and
`as_writable_byte_span()` as part of cleaning up the overloads.
Bug: 341907909, 355603418
Change-Id: I6191eb9191371e32db3a618c77888d773a12fa8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5763092
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1338855}
diff --git a/base/containers/span.h b/base/containers/span.h
index 7927bf6..51db5c3 100644
--- a/base/containers/span.h
+++ b/base/containers/span.h
@@ -16,6 +16,7 @@
#include <iterator>
#include <limits>
#include <memory>
+#include <optional>
#include <span>
#include <type_traits>
#include <utility>
@@ -246,6 +247,7 @@
// - byte_span_from_cstring() function.
// - byte_span_with_nul_from_cstring() function.
// - split_at() method.
+// - to_fixed_extent() method.
// - operator==() comparator function.
// - operator<=>() comparator function.
// - operator<<() printing function.
@@ -1038,6 +1040,15 @@
return UNSAFE_BUFFERS({data() + offset, new_extent});
}
+ // Convert a dynamic-extent span to a fixed-extent span. Returns a
+ // `span<T, Extent>` iff `size() == Extent`; otherwise, returns
+ // `std::nullopt`.
+ template <size_t Extent>
+ constexpr std::optional<span<T, Extent>> to_fixed_extent() const {
+ return size() == Extent ? std::optional(span<T, Extent>(*this))
+ : std::nullopt;
+ }
+
// Splits a span into two at the given `offset`, returning two spans that
// cover the full range of the original span.
//
@@ -1679,41 +1690,16 @@
// to convert std::string or string-objects holding chars, or std::vector
// or vector-like objects holding other scalar types, prior to passing them
// into an API that requires byte spans.
-template <typename T>
- requires requires(const T& arg) {
- requires !std::is_array_v<std::remove_reference_t<T>>;
- make_span(arg);
- }
-constexpr span<const uint8_t> as_byte_span(const T& arg) {
+template <int&... ExplicitArgumentBarrier, typename Spannable>
+ requires requires(const Spannable& arg) { make_span(arg); }
+constexpr auto as_byte_span(const Spannable& arg) {
return as_bytes(make_span(arg));
}
-// This overload for arrays preserves the compile-time size N of the array in
-// the span type signature span<uint8_t, N>.
-template <typename T, size_t N>
+template <int&... ExplicitArgumentBarrier, typename T, size_t N>
constexpr span<const uint8_t, N * sizeof(T)> as_byte_span(
const T (&arr ABSL_ATTRIBUTE_LIFETIME_BOUND)[N]) {
- return as_bytes(make_span<N>(arr));
-}
-
-// This overload adds a compile-time size that must be explicitly given,
-// checking that the size is correct at runtime. The template argument `N` is
-// the number of _bytes_ in the input range, not the number of elements.
-//
-// This is sugar for `base::span<const uint8_t, N>(base::as_byte_span(x))`.
-//
-// Example:
-// ```
-// std::string foo = "hello";
-// base::span<const uint8_t, 5> s = base::as_byte_span<5>(foo);
-// ```
-template <size_t N, typename T>
- requires requires(const T& arg) {
- requires !std::is_array_v<std::remove_reference_t<T>>;
- make_span(arg);
- }
-constexpr span<const uint8_t, N> as_byte_span(const T& arg) {
- return span<const uint8_t, N>(as_byte_span(arg));
+ return as_bytes(make_span(arr));
}
// Convenience function for converting an object which is itself convertible
@@ -1721,50 +1707,27 @@
// to convert std::string or string-objects holding chars, or std::vector
// or vector-like objects holding other scalar types, prior to passing them
// into an API that requires mutable byte spans.
-template <typename T>
- requires requires(T&& arg) {
- requires !std::is_array_v<std::remove_reference_t<T>>;
+template <int&... ExplicitArgumentBarrier, typename Spannable>
+ requires requires(Spannable&& arg) {
make_span(arg);
requires !std::is_const_v<typename decltype(make_span(arg))::element_type>;
}
-constexpr span<uint8_t> as_writable_byte_span(T&& arg) {
- return as_writable_bytes(make_span(arg));
+constexpr auto as_writable_byte_span(Spannable&& arg) {
+ return as_writable_bytes(make_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 <typename T, size_t N>
- requires(!std::is_const_v<T>)
+template <int&... ExplicitArgumentBarrier, typename T, size_t N>
constexpr span<uint8_t, N * sizeof(T)> as_writable_byte_span(
T (&arr ABSL_ATTRIBUTE_LIFETIME_BOUND)[N]) {
- return as_writable_bytes(make_span<N>(arr));
-}
-template <typename T, size_t N>
- requires(!std::is_const_v<T>)
-constexpr span<uint8_t, N * sizeof(T)> as_writable_byte_span(
- T (&&arr ABSL_ATTRIBUTE_LIFETIME_BOUND)[N]) {
- return as_writable_bytes(make_span<N>(arr));
+ return as_writable_bytes(make_span(arr));
}
-// This overload adds a compile-time size that must be explicitly given,
-// checking that the size is correct at runtime. The template argument `N` is
-// the number of _bytes_ in the input range, not the number of elements.
-//
-// This is sugar for `base::span<const uint8_t, N>(base::as_byte_span(x))`.
-//
-// Example:
-// ```
-// std::string foo = "hello";
-// base::span<const uint8_t, 5> s = base::as_writable_byte_span<5>(foo);
-// ```
-template <size_t N, typename T>
- requires requires(T&& arg) {
- requires !std::is_array_v<std::remove_reference_t<T>>;
- make_span(arg);
- requires !std::is_const_v<typename decltype(make_span(arg))::element_type>;
- }
-constexpr span<uint8_t, N> as_writable_byte_span(T&& arg) {
- return span<uint8_t, N>(as_writable_byte_span(arg));
+template <int&... ExplicitArgumentBarrier, typename T, size_t N>
+constexpr span<uint8_t, N * sizeof(T)> as_writable_byte_span(
+ T (&&arr ABSL_ATTRIBUTE_LIFETIME_BOUND)[N]) {
+ return as_writable_bytes(make_span(arr));
}
namespace internal {
diff --git a/base/containers/span_unittest.cc b/base/containers/span_unittest.cc
index 5ac626ef..e749156 100644
--- a/base/containers/span_unittest.cc
+++ b/base/containers/span_unittest.cc
@@ -1506,6 +1506,22 @@
}
}
+TEST(SpanTest, ToFixedExtent) {
+ {
+ const int kArray[] = {1, 2, 3};
+ const span<const int> s(kArray);
+
+ auto static_span = s.to_fixed_extent<3>();
+ ASSERT_TRUE(static_span.has_value());
+ static_assert(std::same_as<typename decltype(static_span)::value_type,
+ span<const int, 3>>);
+ EXPECT_EQ(s.data(), static_span->data());
+ EXPECT_EQ(s.size(), static_span->size());
+
+ EXPECT_EQ(std::nullopt, s.to_fixed_extent<4>());
+ }
+}
+
TEST(SpanTest, Size) {
{
span<int> span;
@@ -1754,14 +1770,6 @@
EXPECT_EQ(byte_span.size(), kVec.size() * sizeof(int));
}
{
- const std::vector<int> kVec({2, 3, 5, 7, 11, 13});
- auto byte_span = as_byte_span<6u * sizeof(int)>(kVec);
- static_assert(std::is_same_v<decltype(byte_span),
- span<const uint8_t, 6u * sizeof(int)>>);
- EXPECT_EQ(byte_span.data(), reinterpret_cast<const uint8_t*>(kVec.data()));
- EXPECT_EQ(byte_span.size(), kVec.size() * sizeof(int));
- }
- {
int kMutArray[] = {2, 3, 5, 7};
auto byte_span = as_byte_span(kMutArray);
static_assert(std::is_same_v<decltype(byte_span),
@@ -1777,15 +1785,6 @@
reinterpret_cast<const uint8_t*>(kMutVec.data()));
EXPECT_EQ(byte_span.size(), kMutVec.size() * sizeof(int));
}
- {
- std::vector<int> kMutVec({2, 3, 5, 7});
- auto byte_span = as_byte_span<4u * sizeof(int)>(kMutVec);
- static_assert(std::is_same_v<decltype(byte_span),
- span<const uint8_t, 4u * sizeof(int)>>);
- EXPECT_EQ(byte_span.data(),
- reinterpret_cast<const uint8_t*>(kMutVec.data()));
- EXPECT_EQ(byte_span.size(), kMutVec.size() * sizeof(int));
- }
// Rvalue input.
{
[](auto byte_span) {
@@ -1798,15 +1797,6 @@
}
}
-TEST(SpanDeathTest, AsByteSpan) {
- // Constructing a fixed-size span of the wrong size will terminate.
- const std::vector<int> kVec({2, 3, 5, 7, 11, 13});
- EXPECT_CHECK_DEATH({
- auto byte_span = as_byte_span<6u>(kVec); // 6 bytes is the wrong size.
- base::debug::Alias(&byte_span);
- });
-}
-
TEST(SpanTest, AsWritableByteSpan) {
{
int kMutArray[] = {2, 3, 5, 7};
@@ -1823,14 +1813,6 @@
EXPECT_EQ(byte_span.data(), reinterpret_cast<uint8_t*>(kMutVec.data()));
EXPECT_EQ(byte_span.size(), kMutVec.size() * sizeof(int));
}
- {
- std::vector<int> kMutVec({2, 3, 5, 7});
- auto byte_span = as_writable_byte_span<4u * sizeof(int)>(kMutVec);
- static_assert(
- std::is_same_v<decltype(byte_span), span<uint8_t, 4u * sizeof(int)>>);
- EXPECT_EQ(byte_span.data(), reinterpret_cast<uint8_t*>(kMutVec.data()));
- EXPECT_EQ(byte_span.size(), kMutVec.size() * sizeof(int));
- }
// Rvalue input.
{
[](auto byte_span) {
@@ -1843,16 +1825,6 @@
}
}
-TEST(SpanDeathTest, AsWritableByteSpan) {
- // Constructing a fixed-size span of the wrong size will terminate.
- std::vector<int> kVec({2, 3, 5, 7, 11, 13});
- EXPECT_CHECK_DEATH({
- auto byte_span =
- as_writable_byte_span<6u>(kVec); // 6 bytes is the wrong size.
- base::debug::Alias(&byte_span);
- });
-}
-
TEST(SpanTest, AsStringView) {
{
constexpr uint8_t kArray[] = {'h', 'e', 'l', 'l', 'o'};
diff --git a/gpu/command_buffer/service/memory_program_cache.cc b/gpu/command_buffer/service/memory_program_cache.cc
index 99424b1..e43349d 100644
--- a/gpu/command_buffer/service/memory_program_cache.cc
+++ b/gpu/command_buffer/service/memory_program_cache.cc
@@ -461,81 +461,91 @@
const std::string& program) {
std::unique_ptr<GpuProgramProto> proto(
GpuProgramProto::default_instance().New());
- if (proto->ParseFromString(program)) {
- AttributeMap vertex_attribs;
- UniformMap vertex_uniforms;
- VaryingMap vertex_varyings;
- OutputVariableList vertex_output_variables;
- InterfaceBlockMap vertex_interface_blocks;
- for (int i = 0; i < proto->vertex_shader().attribs_size(); i++) {
- RetrieveShaderAttributeInfo(proto->vertex_shader().attribs(i),
- &vertex_attribs);
- }
- for (int i = 0; i < proto->vertex_shader().uniforms_size(); i++) {
- RetrieveShaderUniformInfo(proto->vertex_shader().uniforms(i),
- &vertex_uniforms);
- }
- for (int i = 0; i < proto->vertex_shader().varyings_size(); i++) {
- RetrieveShaderVaryingInfo(proto->vertex_shader().varyings(i),
- &vertex_varyings);
- }
- for (int i = 0; i < proto->vertex_shader().output_variables_size(); i++) {
- RetrieveShaderOutputVariableInfo(
- proto->vertex_shader().output_variables(i), &vertex_output_variables);
- }
- for (int i = 0; i < proto->vertex_shader().interface_blocks_size(); i++) {
- RetrieveShaderInterfaceBlockInfo(
- proto->vertex_shader().interface_blocks(i), &vertex_interface_blocks);
- }
-
- AttributeMap fragment_attribs;
- UniformMap fragment_uniforms;
- VaryingMap fragment_varyings;
- OutputVariableList fragment_output_variables;
- InterfaceBlockMap fragment_interface_blocks;
- for (int i = 0; i < proto->fragment_shader().attribs_size(); i++) {
- RetrieveShaderAttributeInfo(proto->fragment_shader().attribs(i),
- &fragment_attribs);
- }
- for (int i = 0; i < proto->fragment_shader().uniforms_size(); i++) {
- RetrieveShaderUniformInfo(proto->fragment_shader().uniforms(i),
- &fragment_uniforms);
- }
- for (int i = 0; i < proto->fragment_shader().varyings_size(); i++) {
- RetrieveShaderVaryingInfo(proto->fragment_shader().varyings(i),
- &fragment_varyings);
- }
- for (int i = 0; i < proto->fragment_shader().output_variables_size(); i++) {
- RetrieveShaderOutputVariableInfo(
- proto->fragment_shader().output_variables(i),
- &fragment_output_variables);
- }
- for (int i = 0; i < proto->fragment_shader().interface_blocks_size(); i++) {
- RetrieveShaderInterfaceBlockInfo(
- proto->fragment_shader().interface_blocks(i),
- &fragment_interface_blocks);
- }
-
- std::vector<uint8_t> binary(proto->program().length());
- memcpy(binary.data(), proto->program().c_str(), proto->program().length());
-
- store_.Put(proto->sha(),
- new ProgramCacheValue(
- proto->format(), std::move(binary),
- proto->has_program_is_compressed() &&
- proto->program_is_compressed(),
- proto->program_decompressed_length(), proto->sha(),
- base::as_byte_span<ProgramCache::kHashLength>(
- proto->vertex_shader().sha()),
- vertex_attribs, vertex_uniforms, vertex_varyings,
- vertex_output_variables, vertex_interface_blocks,
- base::as_byte_span<ProgramCache::kHashLength>(
- proto->fragment_shader().sha()),
- fragment_attribs, fragment_uniforms, fragment_varyings,
- fragment_output_variables, fragment_interface_blocks, this));
- } else {
+ if (!proto->ParseFromString(program)) {
DVLOG(2) << "Failed to parse proto file.";
+ return;
}
+
+ AttributeMap vertex_attribs;
+ UniformMap vertex_uniforms;
+ VaryingMap vertex_varyings;
+ OutputVariableList vertex_output_variables;
+ InterfaceBlockMap vertex_interface_blocks;
+ for (int i = 0; i < proto->vertex_shader().attribs_size(); i++) {
+ RetrieveShaderAttributeInfo(proto->vertex_shader().attribs(i),
+ &vertex_attribs);
+ }
+ for (int i = 0; i < proto->vertex_shader().uniforms_size(); i++) {
+ RetrieveShaderUniformInfo(proto->vertex_shader().uniforms(i),
+ &vertex_uniforms);
+ }
+ for (int i = 0; i < proto->vertex_shader().varyings_size(); i++) {
+ RetrieveShaderVaryingInfo(proto->vertex_shader().varyings(i),
+ &vertex_varyings);
+ }
+ for (int i = 0; i < proto->vertex_shader().output_variables_size(); i++) {
+ RetrieveShaderOutputVariableInfo(proto->vertex_shader().output_variables(i),
+ &vertex_output_variables);
+ }
+ for (int i = 0; i < proto->vertex_shader().interface_blocks_size(); i++) {
+ RetrieveShaderInterfaceBlockInfo(proto->vertex_shader().interface_blocks(i),
+ &vertex_interface_blocks);
+ }
+
+ AttributeMap fragment_attribs;
+ UniformMap fragment_uniforms;
+ VaryingMap fragment_varyings;
+ OutputVariableList fragment_output_variables;
+ InterfaceBlockMap fragment_interface_blocks;
+ for (int i = 0; i < proto->fragment_shader().attribs_size(); i++) {
+ RetrieveShaderAttributeInfo(proto->fragment_shader().attribs(i),
+ &fragment_attribs);
+ }
+ for (int i = 0; i < proto->fragment_shader().uniforms_size(); i++) {
+ RetrieveShaderUniformInfo(proto->fragment_shader().uniforms(i),
+ &fragment_uniforms);
+ }
+ for (int i = 0; i < proto->fragment_shader().varyings_size(); i++) {
+ RetrieveShaderVaryingInfo(proto->fragment_shader().varyings(i),
+ &fragment_varyings);
+ }
+ for (int i = 0; i < proto->fragment_shader().output_variables_size(); i++) {
+ RetrieveShaderOutputVariableInfo(
+ proto->fragment_shader().output_variables(i),
+ &fragment_output_variables);
+ }
+ for (int i = 0; i < proto->fragment_shader().interface_blocks_size(); i++) {
+ RetrieveShaderInterfaceBlockInfo(
+ proto->fragment_shader().interface_blocks(i),
+ &fragment_interface_blocks);
+ }
+
+ std::vector<uint8_t> binary(proto->program().length());
+ memcpy(binary.data(), proto->program().c_str(), proto->program().length());
+
+ auto vertex_shader_sha = base::as_byte_span(proto->vertex_shader().sha())
+ .to_fixed_extent<kHashLength>();
+ auto fragment_shader_sha = base::as_byte_span(proto->fragment_shader().sha())
+ .to_fixed_extent<kHashLength>();
+ if (!vertex_shader_sha.has_value()) {
+ DVLOG(2) << "Ill-formed vertex shader hash";
+ return;
+ }
+ if (!fragment_shader_sha.has_value()) {
+ DVLOG(2) << "Ill-formed fragment shader hash";
+ return;
+ }
+ store_.Put(
+ proto->sha(),
+ new ProgramCacheValue(
+ proto->format(), std::move(binary),
+ proto->has_program_is_compressed() && proto->program_is_compressed(),
+ proto->program_decompressed_length(), proto->sha(),
+ vertex_shader_sha.value(), vertex_attribs, vertex_uniforms,
+ vertex_varyings, vertex_output_variables, vertex_interface_blocks,
+ fragment_shader_sha.value(), fragment_attribs, fragment_uniforms,
+ fragment_varyings, fragment_output_variables,
+ fragment_interface_blocks, this));
}
size_t MemoryProgramCache::Trim(size_t limit) {
diff --git a/gpu/command_buffer/service/program_cache.cc b/gpu/command_buffer/service/program_cache.cc
index 8b1f29a1..9ae7e77 100644
--- a/gpu/command_buffer/service/program_cache.cc
+++ b/gpu/command_buffer/service/program_cache.cc
@@ -49,8 +49,9 @@
bool ProgramCache::HasSuccessfullyCompiledShader(
const std::string& shader_signature) const {
std::string sha(kHashLength, '\0');
- ComputeShaderHash(shader_signature,
- base::as_writable_byte_span<kHashLength>(sha));
+ ComputeShaderHash(
+ shader_signature,
+ base::as_writable_byte_span(sha).to_fixed_extent<kHashLength>().value());
return base::Contains(compiled_shaders_, sha);
}
