From bd122a20ab2e8a39ece1937e900bc68c57bc4a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Wilken=20D=C3=B6rrie?= Date: Thu, 5 Mar 2020 12:05:42 +0000 Subject: [PATCH] [base] Remove const iterator member functions from span MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change implements the resolution of LWG issue 3320 by removing the members cbegin(), cend(), crbegin() and crend() from base::span. This was done because std::cbegin(std::span()) and std::span().cbegin() returned different results if T was non-const. Note that the nested const_iterator type is not yet removed from base::span, as this breaks using it in gMock matchers like ElementsAre. This will be addressed once gMock no longer requires the presence of this nested type. References: * https://wg21.link/LWG3320 * https://github.com/cplusplus/draft/commit/6faa9c TBR=dcheng Bug: 828324 Change-Id: I21f8ffaa0c7183d6e63a087b73f271b677d13f3a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2086231 Commit-Queue: Jan Wilken Dörrie Reviewed-by: Chris Palmer Reviewed-by: Daniel Cheng Cr-Commit-Position: refs/heads/master@{#747216} --- base/containers/span.h | 17 ++++-------- base/containers/span_unittest.cc | 27 +++++-------------- components/cbor/reader_unittest.cc | 2 +- device/fido/opaque_public_key.cc | 5 ++-- .../client/image_decode_accelerator_proxy.cc | 3 +-- ...image_decode_accelerator_proxy_unittest.cc | 3 +-- ...mage_decode_accelerator_worker_unittest.cc | 23 ++++++++-------- 7 files changed, 27 insertions(+), 53 deletions(-) diff --git a/base/containers/span.h b/base/containers/span.h index fa62e6c38a6cff..01697a8cf69006 100644 --- a/base/containers/span.h +++ b/base/containers/span.h @@ -227,9 +227,10 @@ class span : public internal::ExtentStorage { using pointer = T*; using reference = T&; using iterator = CheckedContiguousIterator; - using const_iterator = CheckedContiguousConstIterator; + // TODO(https://crbug.com/828324): Drop the const_iterator typedef once gMock + // supports containers without this nested type. + using const_iterator = iterator; using reverse_iterator = std::reverse_iterator; - using const_reverse_iterator = std::reverse_iterator; static constexpr size_t extent = Extent; // [span.cons], span constructors, copy, assignment, and destructor @@ -395,27 +396,19 @@ class span : public internal::ExtentStorage { constexpr iterator begin() const noexcept { return iterator(data_, data_ + size()); } + constexpr 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(); } - constexpr reverse_iterator rbegin() const noexcept { return reverse_iterator(end()); } + constexpr reverse_iterator rend() const noexcept { return reverse_iterator(begin()); } - constexpr const_reverse_iterator crbegin() const noexcept { - return const_reverse_iterator(cend()); - } - constexpr const_reverse_iterator crend() const noexcept { - return const_reverse_iterator(cbegin()); - } - private: T* data_; }; diff --git a/base/containers/span_unittest.cc b/base/containers/span_unittest.cc index 94e6b455193d86..0f1526ac6779e8 100644 --- a/base/containers/span_unittest.cc +++ b/base/containers/span_unittest.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -965,7 +966,7 @@ TEST(SpanTest, ReverseIterator) { EXPECT_TRUE(std::equal(std::rbegin(kArray), std::rend(kArray), span.rbegin(), span.rend())); EXPECT_TRUE(std::equal(std::crbegin(kArray), std::crend(kArray), - span.crbegin(), span.crend())); + std::crbegin(span), std::crend(span))); } TEST(SpanTest, AsBytes) { @@ -1259,11 +1260,6 @@ TEST(SpanTest, IteratorIsRangeMoveSafe) { CheckedContiguousIterator( span.data() + dest_start_index, span.data() + dest_start_index + kNumElements))); - EXPECT_FALSE(CheckedContiguousConstIterator::IsRangeMoveSafe( - span.cbegin(), span.cend(), - CheckedContiguousConstIterator( - span.data() + dest_start_index, - span.data() + dest_start_index + kNumElements))); } // Non-overlapping ranges. @@ -1273,28 +1269,17 @@ TEST(SpanTest, IteratorIsRangeMoveSafe) { CheckedContiguousIterator( span.data() + dest_start_index, span.data() + dest_start_index + kNumElements))); - EXPECT_TRUE(CheckedContiguousConstIterator::IsRangeMoveSafe( - span.cbegin(), span.cend(), - CheckedContiguousConstIterator( - span.data() + dest_start_index, - span.data() + dest_start_index + kNumElements))); } // IsRangeMoveSafe is true if the length to be moved is 0. EXPECT_TRUE(CheckedContiguousIterator::IsRangeMoveSafe( span.begin(), span.begin(), CheckedContiguousIterator(span.data(), span.data()))); - EXPECT_TRUE(CheckedContiguousConstIterator::IsRangeMoveSafe( - span.cbegin(), span.cbegin(), - CheckedContiguousConstIterator(span.data(), span.data()))); // IsRangeMoveSafe is false if end < begin. EXPECT_FALSE(CheckedContiguousIterator::IsRangeMoveSafe( span.end(), span.begin(), CheckedContiguousIterator(span.data(), span.data()))); - EXPECT_FALSE(CheckedContiguousConstIterator::IsRangeMoveSafe( - span.cend(), span.cbegin(), - CheckedContiguousConstIterator(span.data(), span.data()))); } TEST(SpanTest, Sort) { @@ -1336,12 +1321,12 @@ TEST(SpanTest, SpanExtentConversions) { TEST(SpanTest, IteratorConversions) { static_assert(std::is_convertible::iterator, - span::const_iterator>::value, - "Error: iterator should be convertible to const_iterator"); + span::iterator>::value, + "Error: iterator should be convertible to const iterator"); - static_assert(!std::is_convertible::const_iterator, + static_assert(!std::is_convertible::iterator, span::iterator>::value, - "Error: const_iterator should not be convertible to iterator"); + "Error: const iterator should not be convertible to iterator"); } } // namespace base diff --git a/components/cbor/reader_unittest.cc b/components/cbor/reader_unittest.cc index fd61c9ac76e225..cfee33d71f7c67 100644 --- a/components/cbor/reader_unittest.cc +++ b/components/cbor/reader_unittest.cc @@ -19,7 +19,7 @@ namespace cbor { namespace { std::vector WithExtraneousData(base::span original) { - std::vector ret(original.cbegin(), original.cend()); + std::vector ret(original.begin(), original.end()); // Add a valid one byte long CBOR data item, namely, an unsigned integer // with value "1". ret.push_back(0x01); diff --git a/device/fido/opaque_public_key.cc b/device/fido/opaque_public_key.cc index c8a687402d9722..498338a397323e 100644 --- a/device/fido/opaque_public_key.cc +++ b/device/fido/opaque_public_key.cc @@ -8,9 +8,8 @@ namespace device { OpaquePublicKey::OpaquePublicKey( base::span cose_encoded_public_key) - : PublicKey(), - cose_encoding_(std::vector(cose_encoded_public_key.cbegin(), - cose_encoded_public_key.cend())) {} + : cose_encoding_(cose_encoded_public_key.begin(), + cose_encoded_public_key.end()) {} OpaquePublicKey::~OpaquePublicKey() = default; diff --git a/gpu/ipc/client/image_decode_accelerator_proxy.cc b/gpu/ipc/client/image_decode_accelerator_proxy.cc index 630cd3ddb45eb2..cfd6e613a1c3e6 100644 --- a/gpu/ipc/client/image_decode_accelerator_proxy.cc +++ b/gpu/ipc/client/image_decode_accelerator_proxy.cc @@ -195,8 +195,7 @@ SyncToken ImageDecodeAcceleratorProxy::ScheduleImageDecode( ChannelIdFromCommandBufferId(raster_decoder_command_buffer_id)); GpuChannelMsg_ScheduleImageDecode_Params params; - params.encoded_data = - std::vector(encoded_data.cbegin(), encoded_data.cend()); + params.encoded_data.assign(encoded_data.begin(), encoded_data.end()); params.output_size = output_size; params.raster_decoder_route_id = RouteIdFromCommandBufferId(raster_decoder_command_buffer_id); diff --git a/gpu/ipc/client/image_decode_accelerator_proxy_unittest.cc b/gpu/ipc/client/image_decode_accelerator_proxy_unittest.cc index 7dd7730bc4f369..01fe16772b0a1a 100644 --- a/gpu/ipc/client/image_decode_accelerator_proxy_unittest.cc +++ b/gpu/ipc/client/image_decode_accelerator_proxy_unittest.cc @@ -111,8 +111,7 @@ TEST_F(ImageDecodeAcceleratorProxyTest, ScheduleImageDecodeSendsMessage) { const gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB(); GpuChannelMsg_ScheduleImageDecode_Params expected_params; - expected_params.encoded_data = - std::vector(encoded_data.cbegin(), encoded_data.cend()); + expected_params.encoded_data.assign(encoded_data.begin(), encoded_data.end()); expected_params.output_size = kOutputSize; expected_params.raster_decoder_route_id = kRasterCmdBufferRouteId; expected_params.transfer_cache_entry_id = 1u; diff --git a/media/gpu/vaapi/vaapi_image_decode_accelerator_worker_unittest.cc b/media/gpu/vaapi/vaapi_image_decode_accelerator_worker_unittest.cc index d7bcd05f105d27..31f840b5695b22 100644 --- a/media/gpu/vaapi/vaapi_image_decode_accelerator_worker_unittest.cc +++ b/media/gpu/vaapi/vaapi_image_decode_accelerator_worker_unittest.cc @@ -65,11 +65,10 @@ constexpr uint8_t kLossyWebPFileHeader[] = { }; // clang-format on -constexpr base::span kJpegEncodedData(kJpegPFileHeader, 3u); +constexpr base::span kJpegEncodedData = kJpegPFileHeader; -constexpr base::span kLossyWebPEncodedData( - kLossyWebPFileHeader, - kWebPFileAndVp8ChunkHeaderSizeInBytes); +constexpr base::span + kLossyWebPEncodedData = kLossyWebPFileHeader; class MockNativePixmapDmaBuf : public gfx::NativePixmapDmaBuf { public: @@ -185,10 +184,10 @@ ACTION_P2(ExportAsNativePixmapDmaBufSuccessfully, } TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeSucceeds) { - std::vector jpeg_encoded_data(kJpegEncodedData.cbegin(), - kJpegEncodedData.cend()); - std::vector webp_encoded_data(kLossyWebPEncodedData.cbegin(), - kLossyWebPEncodedData.cend()); + std::vector jpeg_encoded_data(kJpegEncodedData.begin(), + kJpegEncodedData.end()); + std::vector webp_encoded_data(kLossyWebPEncodedData.begin(), + kLossyWebPEncodedData.end()); { InSequence sequence; MockVaapiImageDecoder* jpeg_decoder = GetJpegDecoder(); @@ -235,10 +234,10 @@ TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeSucceeds) { } TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeFails) { - std::vector jpeg_encoded_data(kJpegEncodedData.cbegin(), - kJpegEncodedData.cend()); - std::vector webp_encoded_data(kLossyWebPEncodedData.cbegin(), - kLossyWebPEncodedData.cend()); + std::vector jpeg_encoded_data(kJpegEncodedData.begin(), + kJpegEncodedData.end()); + std::vector webp_encoded_data(kLossyWebPEncodedData.begin(), + kLossyWebPEncodedData.end()); { InSequence sequence; MockVaapiImageDecoder* jpeg_decoder = GetJpegDecoder();