Skip to content

Commit

Permalink
Revert "[base] Remove base::span's comparison operators"
Browse files Browse the repository at this point in the history
This reverts commit 73564b6.

Reason for revert: <Fails build  Google Chrome Win Build #40337 on "chrome/browser/conflicts/module_blacklist_cache_util_win.cc(242,61)">

Original change's description:
> [base] Remove base::span's comparison operators
> 
> At the recent ISO C++ Committee meeting in San Diego it was decided [1]
> to make std::span SemiRegular and drop its comparison operators [2]. For
> further info see also this blog post [3].
> 
> This change updates the base::span implemention appropriately and
> replaces prior usages of the comparison operators with their STL
> algorithm equivalent.
> 
> [1] http://redd.it/9vwvbz
> [2] https://wg21.link/p1085
> [3] https://abseil.io/blog/20180531-regular-types#evaluating-span
> 
> Bug: 828324
> Change-Id: I94e94450df5f233b6de81da81f309386e0dcf4a7
> Reviewed-on: https://chromium-review.googlesource.com/c/1337628
> Reviewed-by: Ken Rockot <rockot@google.com>
> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608702}

TBR=rsleevi@chromium.org,dcheng@chromium.org,rockot@google.com,jrummell@chromium.org,mek@chromium.org,haraken@chromium.org,jdoerrie@chromium.org,hongjunchoi@chromium.org

Change-Id: Id39b32be29dd80b6d299fc99a2efade8c2246fe6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 828324
Reviewed-on: https://chromium-review.googlesource.com/c/1339999
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608711}
  • Loading branch information
Jun Choi authored and Commit Bot committed Nov 16, 2018
1 parent 0bdd840 commit b0bcb96
Show file tree
Hide file tree
Showing 16 changed files with 249 additions and 146 deletions.
33 changes: 33 additions & 0 deletions base/containers/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,39 @@ class span : public internal::ExtentStorage<Extent> {
template <class T, size_t Extent>
constexpr size_t span<T, Extent>::extent;

// [span.comparison], span comparison operators
// Relational operators. Equality is a element-wise comparison.
template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator==(span<T, X> lhs, span<U, Y> rhs) noexcept {
return std::equal(lhs.cbegin(), lhs.cend(), rhs.cbegin(), rhs.cend());
}

template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator!=(span<T, X> lhs, span<U, Y> rhs) noexcept {
return !(lhs == rhs);
}

template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator<(span<T, X> lhs, span<U, Y> rhs) noexcept {
return std::lexicographical_compare(lhs.cbegin(), lhs.cend(), rhs.cbegin(),
rhs.cend());
}

template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator<=(span<T, X> lhs, span<U, Y> rhs) noexcept {
return !(rhs < lhs);
}

template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator>(span<T, X> lhs, span<U, Y> rhs) noexcept {
return rhs < lhs;
}

template <typename T, size_t X, typename U, size_t Y>
constexpr bool operator>=(span<T, X> lhs, span<U, Y> rhs) noexcept {
return !(lhs < rhs);
}

// [span.objectrep], views of object representation
template <typename T, size_t X>
span<const uint8_t, (X == dynamic_extent ? dynamic_extent : sizeof(T) * X)>
Expand Down
146 changes: 126 additions & 20 deletions base/containers/span_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,11 @@ TEST(SpanTest, ConvertBetweenEquivalentTypes) {

span<int32_t> int32_t_span(vector);
span<int> converted_span(int32_t_span);
EXPECT_EQ(int32_t_span.data(), converted_span.data());
EXPECT_EQ(int32_t_span.size(), converted_span.size());
EXPECT_EQ(int32_t_span, converted_span);

span<int32_t, 5> static_int32_t_span(vector);
span<int, 5> static_converted_span(static_int32_t_span);
EXPECT_EQ(static_int32_t_span.data(), static_converted_span.data());
EXPECT_EQ(static_int32_t_span.size(), static_converted_span.size());
EXPECT_EQ(static_int32_t_span, static_converted_span);
}

TEST(SpanTest, TemplatedFirst) {
Expand Down Expand Up @@ -908,6 +906,122 @@ TEST(SpanTest, ReverseIterator) {
span.crbegin(), span.crend()));
}

TEST(SpanTest, Equality) {
static constexpr int kArray1[] = {3, 1, 4, 1, 5};
static constexpr int kArray2[] = {3, 1, 4, 1, 5};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 5> span2(kArray2);

EXPECT_EQ(span1, span2);

static constexpr int kArray3[] = {2, 7, 1, 8, 3};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 == span3);

static double kArray4[] = {2.0, 7.0, 1.0, 8.0, 3.0};
span<double, 5> span4(kArray4);

EXPECT_EQ(span3, span4);
}

TEST(SpanTest, Inequality) {
static constexpr int kArray1[] = {2, 3, 5, 7, 11};
static constexpr int kArray2[] = {1, 4, 6, 8, 9};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 5> span2(kArray2);

EXPECT_NE(span1, span2);

static constexpr int kArray3[] = {2, 3, 5, 7, 11};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 != span3);

static double kArray4[] = {1.0, 4.0, 6.0, 8.0, 9.0};
span<double, 5> span4(kArray4);

EXPECT_NE(span3, span4);
}

TEST(SpanTest, LessThan) {
static constexpr int kArray1[] = {2, 3, 5, 7, 11};
static constexpr int kArray2[] = {2, 3, 5, 7, 11, 13};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 6> span2(kArray2);

EXPECT_LT(span1, span2);

static constexpr int kArray3[] = {2, 3, 5, 7, 11};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 < span3);

static double kArray4[] = {2.0, 3.0, 5.0, 7.0, 11.0, 13.0};
span<double, 6> span4(kArray4);

EXPECT_LT(span3, span4);
}

TEST(SpanTest, LessEqual) {
static constexpr int kArray1[] = {2, 3, 5, 7, 11};
static constexpr int kArray2[] = {2, 3, 5, 7, 11, 13};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 6> span2(kArray2);

EXPECT_LE(span1, span1);
EXPECT_LE(span1, span2);

static constexpr int kArray3[] = {2, 3, 5, 7, 10};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 <= span3);

static double kArray4[] = {2.0, 3.0, 5.0, 7.0, 11.0, 13.0};
span<double, 6> span4(kArray4);

EXPECT_LE(span3, span4);
}

TEST(SpanTest, GreaterThan) {
static constexpr int kArray1[] = {2, 3, 5, 7, 11, 13};
static constexpr int kArray2[] = {2, 3, 5, 7, 11};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 5> span2(kArray2);

EXPECT_GT(span1, span2);

static constexpr int kArray3[] = {2, 3, 5, 7, 11, 13};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 > span3);

static double kArray4[] = {2.0, 3.0, 5.0, 7.0, 11.0};
span<double, 5> span4(kArray4);

EXPECT_GT(span3, span4);
}

TEST(SpanTest, GreaterEqual) {
static constexpr int kArray1[] = {2, 3, 5, 7, 11, 13};
static constexpr int kArray2[] = {2, 3, 5, 7, 11};
constexpr span<const int> span1(kArray1);
constexpr span<const int, 5> span2(kArray2);

EXPECT_GE(span1, span1);
EXPECT_GE(span1, span2);

static constexpr int kArray3[] = {2, 3, 5, 7, 12};
constexpr span<const int> span3(kArray3);

EXPECT_FALSE(span1 >= span3);

static double kArray4[] = {2.0, 3.0, 5.0, 7.0, 11.0};
span<double, 5> span4(kArray4);

EXPECT_GE(span3, span4);
}

TEST(SpanTest, AsBytes) {
{
constexpr int kArray[] = {2, 3, 5, 7, 11, 13};
Expand Down Expand Up @@ -951,8 +1065,7 @@ TEST(SpanTest, MakeSpanFromDataAndSize) {
std::vector<int> vector = {1, 1, 2, 3, 5, 8};
span<int> span(vector.data(), vector.size());
auto made_span = make_span(vector.data(), vector.size());
EXPECT_EQ(span.data(), made_span.data());
EXPECT_EQ(span.size(), made_span.size());
EXPECT_EQ(span, made_span);
static_assert(decltype(made_span)::extent == dynamic_extent, "");
}

Expand All @@ -965,56 +1078,49 @@ TEST(SpanTest, MakeSpanFromPointerPair) {
std::vector<int> vector = {1, 1, 2, 3, 5, 8};
span<int> span(vector.data(), vector.size());
auto made_span = make_span(vector.data(), vector.data() + vector.size());
EXPECT_EQ(span.data(), made_span.data());
EXPECT_EQ(span.size(), made_span.size());
EXPECT_EQ(span, made_span);
static_assert(decltype(made_span)::extent == dynamic_extent, "");
}

TEST(SpanTest, MakeSpanFromConstexprArray) {
static constexpr int kArray[] = {1, 2, 3, 4, 5};
constexpr span<const int> span(kArray);
EXPECT_EQ(span.data(), make_span(kArray).data());
EXPECT_EQ(span.size(), make_span(kArray).size());
EXPECT_EQ(span, make_span(kArray));
static_assert(decltype(make_span(kArray))::extent == 5, "");
}

TEST(SpanTest, MakeSpanFromStdArray) {
const std::array<int, 5> kArray = {{1, 2, 3, 4, 5}};
span<const int> span(kArray);
EXPECT_EQ(span.data(), make_span(kArray).data());
EXPECT_EQ(span.size(), make_span(kArray).size());
EXPECT_EQ(span, make_span(kArray));
static_assert(decltype(make_span(kArray))::extent == 5, "");
}

TEST(SpanTest, MakeSpanFromConstContainer) {
const std::vector<int> vector = {-1, -2, -3, -4, -5};
span<const int> span(vector);
EXPECT_EQ(span.data(), make_span(vector).data());
EXPECT_EQ(span.size(), make_span(vector).size());
EXPECT_EQ(span, make_span(vector));
static_assert(decltype(make_span(vector))::extent == dynamic_extent, "");
}

TEST(SpanTest, MakeStaticSpanFromConstContainer) {
const std::vector<int> vector = {-1, -2, -3, -4, -5};
span<const int, 5> span(vector);
EXPECT_EQ(span.data(), make_span<5>(vector).data());
EXPECT_EQ(span.size(), make_span<5>(vector).size());
EXPECT_EQ(span, make_span<5>(vector));
static_assert(decltype(make_span<5>(vector))::extent == 5, "");
}

TEST(SpanTest, MakeSpanFromContainer) {
std::vector<int> vector = {-1, -2, -3, -4, -5};
span<int> span(vector);
EXPECT_EQ(span.data(), make_span(vector).data());
EXPECT_EQ(span.size(), make_span(vector).size());
EXPECT_EQ(span, make_span(vector));
static_assert(decltype(make_span(vector))::extent == dynamic_extent, "");
}

TEST(SpanTest, MakeStaticSpanFromContainer) {
std::vector<int> vector = {-1, -2, -3, -4, -5};
span<int, 5> span(vector);
EXPECT_EQ(span.data(), make_span<5>(vector).data());
EXPECT_EQ(span.size(), make_span<5>(vector).size());
EXPECT_EQ(span, make_span<5>(vector));
static_assert(decltype(make_span<5>(vector))::extent == 5, "");
}

Expand Down
9 changes: 2 additions & 7 deletions device/fido/ble/fido_ble_frames_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "device/fido/ble/fido_ble_frames.h"

#include <algorithm>
#include <vector>

#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -39,9 +38,7 @@ TEST(FidoBleFramesTest, InitializationFragment) {
FidoBleFrameInitializationFragment::Parse(buffer, &parsed_fragment));

EXPECT_EQ(kDataLength, parsed_fragment.data_length());
EXPECT_TRUE(std::equal(data.begin(), data.end(),
parsed_fragment.fragment().begin(),
parsed_fragment.fragment().end()));
EXPECT_EQ(base::make_span(data), parsed_fragment.fragment());
EXPECT_EQ(FidoBleDeviceCommand::kMsg, parsed_fragment.command());
}

Expand All @@ -61,9 +58,7 @@ TEST(FidoBleFramesTest, ContinuationFragment) {
ASSERT_TRUE(
FidoBleFrameContinuationFragment::Parse(buffer, &parsed_fragment));

EXPECT_TRUE(std::equal(data.begin(), data.end(),
parsed_fragment.fragment().begin(),
parsed_fragment.fragment().end()));
EXPECT_EQ(base::make_span(data), parsed_fragment.fragment());
EXPECT_EQ(kSequence, parsed_fragment.sequence());
}

Expand Down
8 changes: 3 additions & 5 deletions device/fido/cable/fido_cable_discovery_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,8 @@ MATCHER_P2(IsAdvertisementContent,
manufacturer_data_payload[1] == 0x15 && // Manufacturer Data Type
manufacturer_data_payload[2] == 0x20 && // Cable Flags
manufacturer_data_payload[3] == kTestCableVersionNumber &&
std::equal(manufacturer_data_payload.begin() + 4,
manufacturer_data_payload.end(),
expected_client_eid.begin(), expected_client_eid.end());
base::make_span(manufacturer_data_payload).subspan(4) ==
expected_client_eid;

#elif defined(OS_LINUX) || defined(OS_CHROMEOS)
const auto service_data = arg->service_data();
Expand All @@ -132,8 +131,7 @@ MATCHER_P2(IsAdvertisementContent,
return (service_data_value[0] >> 5 & 1) &&
service_data_value[1] == kTestCableVersionNumber &&
service_data_value.size() == 18u &&
std::equal(service_data_value.begin() + 2, service_data_value.end(),
expected_client_eid.begin(), expected_client_eid.end());
base::make_span(service_data_value).subspan(2) == expected_client_eid;

#endif

Expand Down
15 changes: 6 additions & 9 deletions device/fido/fido_parsing_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include <algorithm>
#include <array>
#include <iterator>
#include <utility>
#include <vector>

Expand All @@ -23,16 +22,14 @@
namespace device {
namespace fido_parsing_utils {

// Comparator object that calls std::lexicographical_compare on the begin and
// end iterators of the passed in ranges. Useful when comparing sequence
// containers that are of different types, but have similar semantics.
struct RangeLess {
// Comparator object that calls base::make_span on its arguments before
// comparing them with operator<. Useful when comparing sequence containers that
// are of different types, but have similar semantics.
struct SpanLess {
template <typename T, typename U>
constexpr bool operator()(T&& lhs, U&& rhs) const {
using std::begin;
using std::end;
return std::lexicographical_compare(begin(lhs), end(lhs), begin(rhs),
end(rhs));
return base::make_span(std::forward<T>(lhs)) <
base::make_span(std::forward<U>(rhs));
}

using is_transparent = void;
Expand Down
Loading

0 comments on commit b0bcb96

Please sign in to comment.