Span and Optional enhancements (project-chip#30345)

* Allow constructing Span<const T> from const std::array<T>

* Add converting constructor to Optional

* Make Optional conversion explicit if the underlying conversion is

src/lib/core/Optional.h

@@ -24,6 +24,7 @@
 #pragma once
 
 #include <new>
+#include <type_traits>
 
 #include <lib/core/CHIPCore.h>
 #include <lib/core/InPlace.h>
@@ -73,6 +74,28 @@ class Optional
         }
     }
 
+    // Converts an Optional of an implicitly convertible type
+    template <class U, std::enable_if_t<!std::is_same_v<T, U> && std::is_convertible_v<const U, T>, bool> = true>
+    constexpr Optional(const Optional<U> & other) : mHasValue(other.HasValue())
+    {
+        if (mHasValue)
+        {
+            new (&mValue.mData) T(other.Value());
+        }
+    }
+
+    // Converts an Optional of a type that requires explicit conversion
+    template <class U,
+              std::enable_if_t<!std::is_same_v<T, U> && !std::is_convertible_v<const U, T> && std::is_constructible_v<T, const U &>,
+                               bool> = true>
+    constexpr explicit Optional(const Optional<U> & other) : mHasValue(other.HasValue())
+    {
+        if (mHasValue)
+        {
+            new (&mValue.mData) T(other.Value());
+        }
+    }
+
     constexpr Optional(Optional && other) : mHasValue(other.mHasValue)
     {
         if (mHasValue)

src/lib/core/tests/TestOptional.cpp

@@ -23,11 +23,13 @@
  *
  */
 
-#include <inttypes.h>
-#include <stdint.h>
-#include <string.h>
+#include <array>
+#include <cinttypes>
+#include <cstdint>
+#include <cstring>
 
 #include <lib/core/Optional.h>
+#include <lib/support/Span.h>
 #include <lib/support/UnitTestRegistration.h>
 
 #include <nlunit-test.h>
@@ -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<const bool, 10>;
+    using WidgetStorage = std::array<bool, 10>;
+
+    auto optStorage                   = MakeOptional<WidgetStorage>();
+    auto const & constOptStorage      = optStorage;
+    auto optOtherStorage              = MakeOptional<WidgetStorage>();
+    auto const & constOptOtherStorage = optOtherStorage;
+
+    NL_TEST_ASSERT(inSuite, optStorage.HasValue());
+    NL_TEST_ASSERT(inSuite, optOtherStorage.HasValue());
+
+    Optional<WidgetView> 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<ExplicitBool> e(Optional<bool>(true)); // OK, explicitly constructing the optional
+
+    // The following should not compile
+    // e = Optional<bool>(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

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 <class U, size_t N, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
     constexpr Span(std::array<U, N> & arr) : mDataBuf(arr.data()), mDataLen(N)
     {}
 
+    template <class U, size_t N, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
+    constexpr Span(std::array<U, N> && arr) = delete; // would be a view of an rvalue
+
+    // Creates a Span view of an std::array
+    template <class U, size_t N, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<const U *, T *>::value>>
+    constexpr Span(const std::array<U, N> & arr) : mDataBuf(arr.data()), mDataLen(N)
+    {}
+
     template <size_t N>
     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 <class U, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
     constexpr FixedSpan(std::array<U, N> & arr) : mDataBuf(arr.data())
     {}
 
+    template <class U, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
+    constexpr FixedSpan(std::array<U, N> && arr) = delete; // would be a view of an rvalue
+
+    // Creates a FixedSpan view of an std::array
+    template <class U, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<const U *, T *>::value>>
+    constexpr FixedSpan(const std::array<U, N> & 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 <class U, size_t M, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>

src/lib/support/tests/TestSpan.cpp

@@ -27,6 +27,8 @@
 
 #include <nlunit-test.h>
 
+#include <array>
+
 using namespace chip;
 
 static void TestByteSpan(nlTestSuite * inSuite, void * inContext)
@@ -324,6 +326,22 @@ static void TestConversionConstructors(nlTestSuite * inSuite, void * inContext)
     Span<Foo> span6(testSpan2);
 
     FixedSpan<Foo, 2> span7(testSpan2);
+
+    std::array<Bar, 3> array;
+    const auto & constArray = array;
+    FixedSpan<Foo, 3> span9(array);
+    FixedSpan<const Foo, 3> span10(constArray);
+    Span<Foo> span11(array);
+    Span<const Foo> span12(constArray);
+
+    // Various places around the code base expect these conversions to be implicit
+    ([](FixedSpan<Foo, 3> f) {})(array);
+    ([](Span<Foo> f) {})(array);
+    ([](FixedSpan<const Foo, 3> f) {})(constArray);
+    ([](Span<const Foo> f) {})(constArray);
+
+    // The following should not compile
+    // Span<const Foo> error1 = std::array<Foo, 3>(); // Span would point into a temporary value
 }
 
 #define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn)