Skip to content

Commit

Permalink
Allow passing FixedSpan to TLVWriter::Put. (#8294)
Browse files Browse the repository at this point in the history 
Two main changes here:

1. Narrow down the existing Put template for enums to only get matched
for enum types, so it does not try to get compiled for a FixedSpan.

2. Introduce more inter-conversions between Span and FixedSpan, where
possible, via implicit constructors.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Sep 16, 2021
1 parent 27a3359 commit b6d18b4
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/lib/core/CHIPTLV.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ class DLL_EXPORT TLVWriter
/**
* static_cast to enumerations' underlying type when data is an enumeration.
*/
template <typename T>
template <typename T, typename = std::enable_if_t<std::is_enum<T>::value>>
CHIP_ERROR Put(uint64_t tag, T data)
{
return Put(tag, to_underlying(data));
Expand Down
65 changes: 48 additions & 17 deletions src/lib/support/Span.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@

namespace chip {

template <class T, size_t N>
class FixedSpan;

/**
* @brief A wrapper class for holding objects and its length, without the ownership of it.
* We can use C++20 std::span once we support it, the data() and size() come from C++20 std::span.
Expand All @@ -42,6 +45,17 @@ class Span
constexpr explicit Span(T (&databuf)[N]) : Span(databuf, N)
{}

// Allow implicit construction from a Span over a type that matches our
// type, up to const-ness.
template <class U, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr Span(const Span<U> & other) : Span(other.data(), other.size())
{}

// Allow implicit construction from a FixedSpan over a type that matches our
// type, up to const-ness.
template <class U, size_t N, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr inline Span(const FixedSpan<U, N> & other);

constexpr pointer data() const { return mDataBuf; }
constexpr size_t size() const { return mDataLen; }
constexpr bool empty() const { return size() == 0; }
Expand All @@ -52,6 +66,8 @@ class Span
{
return (size() == other.size()) && (empty() || (memcmp(data(), other.data(), size() * sizeof(T)) == 0));
}
template <class U, size_t N, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
inline bool data_equal(const FixedSpan<U, N> & other) const;

Span SubSpan(size_t offset, size_t length) const
{
Expand All @@ -60,13 +76,6 @@ class Span
return Span(mDataBuf + offset, length);
}

// Allow converting a span with non-const T into a span with const T.
template <class U = T>
operator typename std::enable_if_t<!std::is_const<U>::value, Span<const T>>() const
{
return Span<const T>(data(), size());
}

// Allow reducing the size of a span.
void reduce_size(size_t new_size)
{
Expand Down Expand Up @@ -96,19 +105,30 @@ class FixedSpan
// template).
//
// To do that we have a template constructor enabled only when the type
// passed to it is a pointer type, and rely on our assignment of that
// pointer type to mDataBuf to verify that the pointer is to a type
// compatible enough with T (T itself, a subclass, a non-const version if T
// is const, etc).
template <class U, typename = std::enable_if_t<std::is_pointer<U>::value>>
// passed to it is a pointer type, and that pointer is to a type that
// matches T up to const-ness. Importantly, we do NOT want to allow
// subclasses of T here, because they would have a different size and our
// Span would not work right.
template <
class U,
typename = std::enable_if_t<std::is_pointer<U>::value &&
std::is_same<std::remove_const_t<T>, std::remove_const_t<std::remove_pointer_t<U>>>::value>>
constexpr explicit FixedSpan(U databuf) : mDataBuf(databuf)
{}
template <class U, size_t M>
template <class U, size_t M, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr explicit FixedSpan(U (&databuf)[M]) : mDataBuf(databuf)
{
static_assert(M >= N, "Passed-in buffer too small for FixedSpan");
}

// Allow implicit construction from a FixedSpan of sufficient size over a
// type that matches our type, up to const-ness.
template <class U, size_t M, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr FixedSpan(FixedSpan<U, M> const & other) : mDataBuf(other.data())
{
static_assert(M >= N, "Passed-in FixedSpan is smaller than we are");
}

constexpr pointer data() const { return mDataBuf; }
constexpr size_t size() const { return N; }
constexpr bool empty() const { return data() == nullptr; }
Expand All @@ -126,17 +146,28 @@ class FixedSpan
(!empty() && !other.empty() && (memcmp(data(), other.data(), size() * sizeof(T)) == 0));
}

// Allow converting a span with non-const T into a span with const T.
template <class U = T>
operator typename std::enable_if_t<!std::is_const<U>::value, FixedSpan<const T, N>>() const
template <class U, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
bool data_equal(const Span<U> & other) const
{
return FixedSpan<const T, N>(data());
return (size() == other.size() && (empty() || (memcmp(data(), other.data(), size() * sizeof(T)) == 0)));
}

private:
pointer mDataBuf;
};

template <class T>
template <class U, size_t N, typename>
constexpr Span<T>::Span(const FixedSpan<U, N> & other) : Span(other.data(), other.size())
{}

template <class T>
template <class U, size_t N, typename>
inline bool Span<T>::data_equal(const FixedSpan<U, N> & other) const
{
return (size() == other.size()) && (empty() || (memcmp(data(), other.data(), size() * sizeof(T)) == 0));
}

using ByteSpan = Span<const uint8_t>;
using MutableByteSpan = Span<uint8_t>;
template <size_t N>
Expand Down
61 changes: 59 additions & 2 deletions src/lib/support/tests/TestSpan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ static void TestMutableByteSpan(nlTestSuite * inSuite, void * inContext)
ByteSpan s9 = s8;
NL_TEST_ASSERT(inSuite, s9.data_equal(s8));
NL_TEST_ASSERT(inSuite, s8.data_equal(s9));

// Not mutable span on purpose.
ByteSpan s10(s8);
NL_TEST_ASSERT(inSuite, s10.data_equal(s8));
NL_TEST_ASSERT(inSuite, s8.data_equal(s10));
}

static void TestFixedByteSpan(nlTestSuite * inSuite, void * inContext)
Expand Down Expand Up @@ -171,7 +176,7 @@ static void TestFixedByteSpan(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, s3.data_equal(s2));

uint8_t arr2[] = { 3, 2, 1 };
FixedByteSpan<3> s4(arr2);
FixedSpan<uint8_t, 3> s4(arr2);
NL_TEST_ASSERT(inSuite, !s4.data_equal(s2));

size_t idx = 0;
Expand All @@ -180,14 +185,66 @@ static void TestFixedByteSpan(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, entry == arr2[idx++]);
}
NL_TEST_ASSERT(inSuite, idx == 3);

FixedByteSpan<3> s5(arr2);
NL_TEST_ASSERT(inSuite, s5.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s5));

FixedByteSpan<2> s6(s4);
idx = 0;
for (auto & entry : s6)
{
NL_TEST_ASSERT(inSuite, entry == arr2[idx++]);
}
NL_TEST_ASSERT(inSuite, idx == 2);

// Not fixed, to test conversion.
ByteSpan s7(s4);
NL_TEST_ASSERT(inSuite, s7.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s7));

MutableByteSpan s8(s4);
NL_TEST_ASSERT(inSuite, s8.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s8));
}

static void TestSpanOfPointers(nlTestSuite * inSuite, void * inContext)
{
uint8_t x = 5;
uint8_t * ptrs[] = { &x, &x };
Span<uint8_t *> s1(ptrs);
Span<uint8_t * const> s2(s1);
NL_TEST_ASSERT(inSuite, s1.data_equal(s2));
NL_TEST_ASSERT(inSuite, s2.data_equal(s1));

FixedSpan<uint8_t *, 2> s3(ptrs);
FixedSpan<uint8_t * const, 2> s4(s3);
NL_TEST_ASSERT(inSuite, s1.data_equal(s3));
NL_TEST_ASSERT(inSuite, s3.data_equal(s1));

NL_TEST_ASSERT(inSuite, s2.data_equal(s3));
NL_TEST_ASSERT(inSuite, s3.data_equal(s2));

NL_TEST_ASSERT(inSuite, s1.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s1));

NL_TEST_ASSERT(inSuite, s2.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s2));

NL_TEST_ASSERT(inSuite, s3.data_equal(s4));
NL_TEST_ASSERT(inSuite, s4.data_equal(s3));

Span<uint8_t *> s5(s3);
NL_TEST_ASSERT(inSuite, s5.data_equal(s3));
NL_TEST_ASSERT(inSuite, s3.data_equal(s5));
}

#define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn)
/**
* Test Suite. It lists all the test functions.
*/
static const nlTest sTests[] = { NL_TEST_DEF_FN(TestByteSpan), NL_TEST_DEF_FN(TestMutableByteSpan),
NL_TEST_DEF_FN(TestFixedByteSpan), NL_TEST_SENTINEL() };
NL_TEST_DEF_FN(TestFixedByteSpan), NL_TEST_DEF_FN(TestSpanOfPointers), NL_TEST_SENTINEL() };

int TestSpan(void)
{
Expand Down

0 comments on commit b6d18b4

Please sign in to comment.