From 7c8071e847097fad3e7b8769bea8ed16e47b7536 Mon Sep 17 00:00:00 2001 From: hai007 Date: Fri, 25 Oct 2024 10:48:40 -0700 Subject: [PATCH] Add begin()/end() to ByteArray. This allows this type to meet the requirements of e.g. std::ranges::range, which is necessary for it to work with the std::span range constructor, or the "non-legacy" constructor for Chromium's base::span. Fixes a number of IWYU problems along the way, to silence checks. Bug: chromium:364987728 PiperOrigin-RevId: 689841844 --- .../mediums/ble_v2/ble_utils.cc | 12 +++ .../implementation/mediums/wifi_lan.cc | 7 +- internal/platform/BUILD | 1 + internal/platform/byte_array.h | 16 +++- internal/platform/byte_array_test.cc | 81 +++++++++++++++++++ internal/weave/packetizer.cc | 2 + 6 files changed, 115 insertions(+), 4 deletions(-) diff --git a/connections/implementation/mediums/ble_v2/ble_utils.cc b/connections/implementation/mediums/ble_v2/ble_utils.cc index e7606227..3cbb1806 100644 --- a/connections/implementation/mediums/ble_v2/ble_utils.cc +++ b/connections/implementation/mediums/ble_v2/ble_utils.cc @@ -14,9 +14,21 @@ #include "connections/implementation/mediums/ble_v2/ble_utils.h" +#include +#include #include +#include "absl/base/attributes.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include "absl/types/optional.h" +#include "connections/implementation/mediums/ble_v2/ble_advertisement.h" +#include "connections/implementation/mediums/ble_v2/ble_advertisement_header.h" +#include "connections/implementation/mediums/ble_v2/ble_packet.h" +#include "connections/implementation/mediums/utils.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/prng.h" +#include "internal/platform/uuid.h" namespace nearby { namespace connections { diff --git a/connections/implementation/mediums/wifi_lan.cc b/connections/implementation/mediums/wifi_lan.cc index 2c3e262e..e336e5f8 100644 --- a/connections/implementation/mediums/wifi_lan.cc +++ b/connections/implementation/mediums/wifi_lan.cc @@ -14,14 +14,19 @@ #include "connections/implementation/mediums/wifi_lan.h" -#include +#include #include #include +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "connections/implementation/mediums/utils.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/cancellation_flag.h" #include "internal/platform/logging.h" #include "internal/platform/mutex_lock.h" +#include "internal/platform/nsd_service_info.h" +#include "internal/platform/wifi_lan.h" namespace nearby { namespace connections { diff --git a/internal/platform/BUILD b/internal/platform/BUILD index f4e3bad8..f47c0de5 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -392,6 +392,7 @@ cc_test( "//internal/platform/implementation/g3", # build_cleaner: keep "@com_github_protobuf_matchers//protobuf-matchers", "@com_google_absl//absl/hash:hash_testing", + "@com_google_absl//absl/strings:string_view", "@com_google_absl//absl/synchronization", "@com_google_googletest//:gtest_main", ], diff --git a/internal/platform/byte_array.h b/internal/platform/byte_array.h index 2212c0f5..78d44436 100644 --- a/internal/platform/byte_array.h +++ b/internal/platform/byte_array.h @@ -17,19 +17,19 @@ #include #include -#include #include #include -#include #include -#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" namespace nearby { class ByteArray { public: + using iterator = std::string::iterator; + using const_iterator = std::string::const_iterator; + // Create an empty ByteArray ByteArray() = default; template @@ -84,6 +84,16 @@ class ByteArray { size_t size() const { return data_.size(); } bool Empty() const { return data_.empty(); } + // Iterators. These allow `ByteArray` to meet the requirements of + // `std::ranges::contiguous_range`, which in turn make it implicitly + // convertible to e.g. `std::span`. + iterator begin() { return data_.begin(); } + const_iterator begin() const { return data_.begin(); } + const_iterator cbegin() const { return data_.cbegin(); } + iterator end() { return data_.end(); } + const_iterator end() const { return data_.end(); } + const_iterator cend() const { return data_.cend(); } + friend bool operator==(const ByteArray& lhs, const ByteArray& rhs); friend bool operator!=(const ByteArray& lhs, const ByteArray& rhs); friend bool operator<(const ByteArray& lhs, const ByteArray& rhs); diff --git a/internal/platform/byte_array_test.cc b/internal/platform/byte_array_test.cc index 2397ad3b..7b8a8fa5 100644 --- a/internal/platform/byte_array_test.cc +++ b/internal/platform/byte_array_test.cc @@ -14,11 +14,17 @@ #include "internal/platform/byte_array.h" +#include + +#include +#include #include #include +#include #include "gtest/gtest.h" #include "absl/hash/hash_testing.h" +#include "absl/strings/string_view.h" namespace { @@ -97,6 +103,81 @@ TEST(ByteArrayTest, CreateFromAbslStringReturnsTheSame) { EXPECT_EQ(bytes.AsStringView(), kTestString); } +TEST(ByteArrayTest, IteratorTypes) { + static_assert(std::same_as().begin()), + ByteArray::iterator>); + static_assert(std::same_as().cbegin()), + ByteArray::const_iterator>); + static_assert(std::same_as().end()), + ByteArray::iterator>); + static_assert(std::same_as().cend()), + ByteArray::const_iterator>); + + static_assert(std::same_as().begin()), + ByteArray::const_iterator>); + static_assert(std::same_as().end()), + ByteArray::const_iterator>); +} + +TEST(ByteArrayTest, Iterators) { + ByteArray bytes("12345"); + const ByteArray const_bytes("12345"); + static constexpr auto kExpected = std::to_array({'1', '2', '3', '4', '5'}); + + // Check manual iteration. + { + size_t i = 0; + for (auto it = bytes.begin(); it != bytes.end(); ++it) { + ASSERT_LT(i, kExpected.size()); + EXPECT_EQ(*it, kExpected[i++]); + } + EXPECT_EQ(i, kExpected.size()); + } + { + size_t i = 0; + for (auto it = bytes.cbegin(); it != bytes.cend(); ++it) { + ASSERT_LT(i, kExpected.size()); + EXPECT_EQ(*it, kExpected[i++]); + } + EXPECT_EQ(i, kExpected.size()); + } + { + size_t i = 0; + for (auto it = const_bytes.begin(); it != const_bytes.end(); ++it) { + ASSERT_LT(i, kExpected.size()); + EXPECT_EQ(*it, kExpected[i++]); + } + EXPECT_EQ(i, kExpected.size()); + } + + // Check range-for loops. + { + size_t i = 0; + for (auto c : bytes) { + ASSERT_LT(i, kExpected.size()); + EXPECT_EQ(c, kExpected[i++]); + } + EXPECT_EQ(i, kExpected.size()); + } + { + size_t i = 0; + for (auto c : const_bytes) { + ASSERT_LT(i, kExpected.size()); + EXPECT_EQ(c, kExpected[i++]); + } + EXPECT_EQ(i, kExpected.size()); + } +} + +TEST(ByteArrayTest, EmptyArrayIterators) { + // It should be legal to call begin()/end() etc. on empty arrays. + ByteArray bytes; + const ByteArray const_bytes; + EXPECT_EQ(bytes.begin(), bytes.end()); + EXPECT_EQ(bytes.cbegin(), bytes.cend()); + EXPECT_EQ(const_bytes.begin(), const_bytes.end()); +} + TEST(ByteArrayTest, Hash) { EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly({ ByteArray(), diff --git a/internal/weave/packetizer.cc b/internal/weave/packetizer.cc index 81519190..a7ad3883 100644 --- a/internal/weave/packetizer.cc +++ b/internal/weave/packetizer.cc @@ -17,6 +17,8 @@ #include #include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" #include "internal/platform/byte_array.h" #include "internal/platform/mutex_lock.h" #include "internal/weave/packet.h"