From dbd6d1d64a916c4a2358cca4e4e0b4fb9485ef54 Mon Sep 17 00:00:00 2001 From: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Date: Fri, 10 Nov 2023 13:05:33 +1300 Subject: [PATCH] Span and Optional enhancements (#30345) * Allow constructing Span from const std::array * Add converting constructor to Optional * Make Optional conversion explicit if the underlying conversion is --- src/lib/core/Optional.h | 23 +++++++++++++++ src/lib/core/tests/TestOptional.cpp | 43 ++++++++++++++++++++++++++--- src/lib/support/Span.h | 18 ++++++++++++ src/lib/support/tests/TestSpan.cpp | 18 ++++++++++++ 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/src/lib/core/Optional.h b/src/lib/core/Optional.h index 3b673e36fa7071..7278484a6e6be9 100644 --- a/src/lib/core/Optional.h +++ b/src/lib/core/Optional.h @@ -24,6 +24,7 @@ #pragma once #include +#include #include #include @@ -73,6 +74,28 @@ class Optional } } + // Converts an Optional of an implicitly convertible type + template && std::is_convertible_v, bool> = true> + constexpr Optional(const Optional & other) : mHasValue(other.HasValue()) + { + if (mHasValue) + { + new (&mValue.mData) T(other.Value()); + } + } + + // Converts an Optional of a type that requires explicit conversion + template && !std::is_convertible_v && std::is_constructible_v, + bool> = true> + constexpr explicit Optional(const Optional & other) : mHasValue(other.HasValue()) + { + if (mHasValue) + { + new (&mValue.mData) T(other.Value()); + } + } + constexpr Optional(Optional && other) : mHasValue(other.mHasValue) { if (mHasValue) diff --git a/src/lib/core/tests/TestOptional.cpp b/src/lib/core/tests/TestOptional.cpp index 607829834a717f..b54d8de1120a4c 100644 --- a/src/lib/core/tests/TestOptional.cpp +++ b/src/lib/core/tests/TestOptional.cpp @@ -23,11 +23,13 @@ * */ -#include -#include -#include +#include +#include +#include +#include #include +#include #include #include @@ -184,6 +186,39 @@ static void TestMove(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, Count::created == 4 && Count::destroyed == 4); } +static void TestConversion(nlTestSuite * inSuite, void * inContext) +{ + // FixedSpan is implicitly convertible from std::array + using WidgetView = FixedSpan; + using WidgetStorage = std::array; + + auto optStorage = MakeOptional(); + auto const & constOptStorage = optStorage; + auto optOtherStorage = MakeOptional(); + auto const & constOptOtherStorage = optOtherStorage; + + NL_TEST_ASSERT(inSuite, optStorage.HasValue()); + NL_TEST_ASSERT(inSuite, optOtherStorage.HasValue()); + + Optional optView(constOptStorage); + NL_TEST_ASSERT(inSuite, optView.HasValue()); + NL_TEST_ASSERT(inSuite, &optView.Value()[0] == &optStorage.Value()[0]); + + optView = optOtherStorage; + optView = constOptOtherStorage; + NL_TEST_ASSERT(inSuite, optView.HasValue()); + NL_TEST_ASSERT(inSuite, &optView.Value()[0] == &optOtherStorage.Value()[0]); + + struct ExplicitBool + { + explicit ExplicitBool(bool) {} + }; + Optional e(Optional(true)); // OK, explicitly constructing the optional + + // The following should not compile + // e = Optional(false); // relies on implicit conversion +} + /** * Test Suite. It lists all the test functions. */ @@ -195,7 +230,7 @@ static const nlTest sTests[] = NL_TEST_DEF("OptionalMake", TestMake), NL_TEST_DEF("OptionalCopy", TestCopy), NL_TEST_DEF("OptionalMove", TestMove), - + NL_TEST_DEF("OptionalConversion", TestConversion), NL_TEST_SENTINEL() }; // clang-format on diff --git a/src/lib/support/Span.h b/src/lib/support/Span.h index aad5ffe4bfa5fe..7b7bcb8cd48b1f 100644 --- a/src/lib/support/Span.h +++ b/src/lib/support/Span.h @@ -62,10 +62,19 @@ class Span constexpr explicit Span(U (&databuf)[N]) : mDataBuf(databuf), mDataLen(N) {} + // Creates a (potentially mutable) Span view of an std::array template ::value>> constexpr Span(std::array & arr) : mDataBuf(arr.data()), mDataLen(N) {} + template ::value>> + constexpr Span(std::array && arr) = delete; // would be a view of an rvalue + + // Creates a Span view of an std::array + template ::value>> + constexpr Span(const std::array & arr) : mDataBuf(arr.data()), mDataLen(N) + {} + template constexpr Span & operator=(T (&databuf)[N]) { @@ -265,10 +274,19 @@ class FixedSpan static_assert(M >= N, "Passed-in buffer too small for FixedSpan"); } + // Creates a (potentially mutable) FixedSpan view of an std::array template ::value>> constexpr FixedSpan(std::array & arr) : mDataBuf(arr.data()) {} + template ::value>> + constexpr FixedSpan(std::array && arr) = delete; // would be a view of an rvalue + + // Creates a FixedSpan view of an std::array + template ::value>> + constexpr FixedSpan(const std::array & arr) : mDataBuf(arr.data()) + {} + // Allow implicit construction from a FixedSpan of sufficient size over a // type that has the same size as ours, as long as the pointers are convertible. template ::value>> diff --git a/src/lib/support/tests/TestSpan.cpp b/src/lib/support/tests/TestSpan.cpp index 303eb3f596637b..6490755ca57d7d 100644 --- a/src/lib/support/tests/TestSpan.cpp +++ b/src/lib/support/tests/TestSpan.cpp @@ -27,6 +27,8 @@ #include +#include + using namespace chip; static void TestByteSpan(nlTestSuite * inSuite, void * inContext) @@ -324,6 +326,22 @@ static void TestConversionConstructors(nlTestSuite * inSuite, void * inContext) Span span6(testSpan2); FixedSpan span7(testSpan2); + + std::array array; + const auto & constArray = array; + FixedSpan span9(array); + FixedSpan span10(constArray); + Span span11(array); + Span span12(constArray); + + // Various places around the code base expect these conversions to be implicit + ([](FixedSpan f) {})(array); + ([](Span f) {})(array); + ([](FixedSpan f) {})(constArray); + ([](Span f) {})(constArray); + + // The following should not compile + // Span error1 = std::array(); // Span would point into a temporary value } #define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn)