Be a little more flexible about Span constructors. (#26701)

Before this change, this code:

    struct X {
      int member;
    };

    struct Y : public X {
    };

    Y object;
    Span<X> span(&Y, 1);

would not compile, because we had std::is_same checks on the types (X and Y in
this case).  But this code is in fact safe as long as sizeof(Y) == sizeof(X), so
we don't get confused about our object boundaries.

This change replaces the std::is_same checks with sizeof() checks.  But that
leads to some situations being ambiguous, because the "can this pointer actually
work for us?" check happens after overload resolution, so we explicitly do the
std::is_convertible checks alongside sizeof to make sure that the pointer passed
to our Span constructor will work.

Author: bzbarsky-apple

src/lib/support/Span.h

@@ -43,11 +43,11 @@ class Span
 
     constexpr Span() : mDataBuf(nullptr), mDataLen(0) {}
     constexpr Span(pointer databuf, size_t datalen) : mDataBuf(databuf), mDataLen(datalen) {}
-    template <size_t N>
-    constexpr explicit Span(T (&databuf)[N]) : Span(databuf, N)
+    template <class U, size_t N, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
+    constexpr explicit Span(U (&databuf)[N]) : Span(databuf, N)
     {}
 
-    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>>
+    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)
     {}
 
@@ -60,14 +60,20 @@ class Span
     }
 
     // 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>>
+    // type's size, if a pointer to the other type can be treated as a pointer
+    // to our type (e.g. other type is same as ours, or is a same-size
+    // subclass).  The size check is really important to make sure we don't get
+    // confused about where our object boundaries are.
+    template <class U, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::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>>
+    // type's size, if a pointer to the other type can be treated as a pointer
+    // to our type (e.g. other type is same as ours, or is a same-size
+    // subclass).  The size check is really important to make sure we don't get
+    // confused about where our object boundaries are.
+    template <class U, size_t N, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
     constexpr inline Span(const FixedSpan<U, N> & other);
 
     constexpr pointer data() const { return mDataBuf; }
@@ -169,28 +175,25 @@ class FixedSpan
     //
     // To do that we have a template constructor enabled only when the type
     // 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>>
+    // matches T's size and can convert to T*.
+    template <class U,
+              typename = std::enable_if_t<std::is_pointer<U>::value && sizeof(std::remove_pointer_t<U>) == sizeof(T) &&
+                                          std::is_convertible<U, T *>::value>>
     constexpr explicit FixedSpan(U databuf) : mDataBuf(databuf)
     {}
-    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>>
+    template <class U, size_t M, typename = std::enable_if_t<sizeof(U) == sizeof(T) && std::is_convertible<U *, T *>::value>>
     constexpr explicit FixedSpan(U (&databuf)[M]) : mDataBuf(databuf)
     {
         static_assert(M >= N, "Passed-in buffer too small for FixedSpan");
     }
 
-    template <class U, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
+    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())
     {}
 
     // 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>>
+    // 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>>
     constexpr FixedSpan(FixedSpan<U, M> const & other) : mDataBuf(other.data())
     {
         static_assert(M >= N, "Passed-in FixedSpan is smaller than we are");

src/lib/support/tests/TestSpan.cpp

@@ -302,14 +302,48 @@ static void TestFromCharString(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, s1.data_equal(CharSpan(str, 3)));
 }
 
+static void TestConversionConstructors(nlTestSuite * inSuite, void * inContext)
+{
+    struct Foo
+    {
+        int member = 0;
+    };
+    struct Bar : public Foo
+    {
+    };
+
+    Bar objects[2];
+
+    // Check that various things here compile.
+    Span<Foo> span1(objects);
+    Span<Foo> span2(&objects[0], 1);
+    FixedSpan<Foo, 2> span3(objects);
+    FixedSpan<Foo, 1> span4(objects);
+
+    Span<Bar> testSpan1(objects);
+    FixedSpan<Bar, 2> testSpan2(objects);
+
+    Span<Foo> span5(testSpan1);
+    Span<Foo> span6(testSpan2);
+
+    FixedSpan<Foo, 2> span7(testSpan2);
+}
+
 #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_DEF_FN(TestSpanOfPointers),
-                                 NL_TEST_DEF_FN(TestSubSpan),        NL_TEST_DEF_FN(TestFromZclString),
-                                 NL_TEST_DEF_FN(TestFromCharString), NL_TEST_SENTINEL() };
+static const nlTest sTests[] = {
+    NL_TEST_DEF_FN(TestByteSpan),
+    NL_TEST_DEF_FN(TestMutableByteSpan),
+    NL_TEST_DEF_FN(TestFixedByteSpan),
+    NL_TEST_DEF_FN(TestSpanOfPointers),
+    NL_TEST_DEF_FN(TestSubSpan),
+    NL_TEST_DEF_FN(TestFromZclString),
+    NL_TEST_DEF_FN(TestFromCharString),
+    NL_TEST_DEF_FN(TestConversionConstructors),
+    NL_TEST_SENTINEL(),
+};
 
 int TestSpan()
 {