From c94a66f4684e38ad599d5de15a5f6ac319395286 Mon Sep 17 00:00:00 2001
From: Neil MacIntosh <neilmac@microsoft.com>
Date: Sun, 12 Jun 2016 18:28:19 -0700
Subject: [PATCH] Tightened SFINAE for span<U> to span<T> conversions.

---
 include/span.h       |  91 ++++++++++++++++++++++--------------
 tests/span_tests.cpp | 109 +++++++++++++++++++------------------------
 2 files changed, 104 insertions(+), 96 deletions(-)

diff --git a/include/span.h b/include/span.h
index 7152c2d..676fdf2 100644
--- a/include/span.h
+++ b/include/span.h
@@ -77,6 +77,11 @@ template <class ElementType, std::ptrdiff_t Extent = dynamic_extent>
 class span;
 
 
+// [views.constants], constants  
+constexpr const std::ptrdiff_t dynamic_extent = -1;
+
+
+// implementation details
 namespace details
 {
 template <class T>
@@ -96,45 +101,55 @@ struct is_span : is_span_oracle<std::remove_cv_t<T>>
 
 template <class From, class To>
 struct is_allowed_pointer_conversion
-    : std::integral_constant<bool,
-        std::is_pointer<From>::value &&
-        std::is_pointer<To>::value &&
-        std::is_convertible<From, To>::value
+    : std::bool_constant<
+    std::is_pointer<From>::value &&
+    std::is_pointer<To>::value &&
+    std::is_convertible<From, To>::value
     >
 {
 };
 
 template <class From, class To>
 struct is_allowed_integral_conversion
-    : std::integral_constant<bool,
-        std::is_integral<From>::value &&
-        std::is_integral<To>::value &&
-        sizeof(From) == sizeof(To) && 
-        alignof(From) == alignof(To) &&
-        std::is_convertible<From, To>::value
+    : std::bool_constant<
+    std::is_integral<From>::value &&
+    std::is_integral<To>::value &&
+    sizeof(From) == sizeof(To) &&
+    alignof(From) == alignof(To) &&
+    std::is_convertible<From, To>::value
+    >
+{
+};
+
+template <std::ptrdiff_t From, std::ptrdiff_t To>
+struct is_allowed_extent_conversion
+    : std::bool_constant<
+    From == To ||
+    From == gsl::dynamic_extent ||
+    To == gsl::dynamic_extent
     >
 {
 };
 
 template <class From, class To>
 struct is_allowed_element_type_conversion
-    : std::integral_constant<bool,        
-        std::is_same<From, std::remove_cv_t<To>>::value ||
-        is_allowed_pointer_conversion<From, To>::value ||
-        is_allowed_integral_conversion<From, To>::value
+    : std::bool_constant<
+    std::is_same<From, std::remove_cv_t<To>>::value ||
+    is_allowed_pointer_conversion<From, To>::value ||
+    is_allowed_integral_conversion<From, To>::value
     >
 {
 };
 
 template <class From>
 struct is_allowed_element_type_conversion<From, byte>
-    : std::integral_constant<bool, !std::is_const<From>::value>
+    : std::bool_constant<!std::is_const<From>::value>
 {
 };
 
 template <class From>
 struct is_allowed_element_type_conversion<From, const byte>
-    : std::integral_constant<bool, true>
+    : std::true_type
 {
 };
 
@@ -191,7 +206,7 @@ public:
         auto ret{*this};
         return ret += n;
     }
-    
+
     span_iterator& operator+=(difference_type n) noexcept
     {
         index_ += n;
@@ -217,10 +232,14 @@ public:
     }
 
     reference operator[](difference_type n) const noexcept
-    { return *(*this + n); }
+    {
+        return *(*this + n);
+    }
 
     bool operator==(const span_iterator& rhs) const noexcept
-    { return span_ == rhs.span_ && index_ == rhs.index_; }
+    {
+        return span_ == rhs.span_ && index_ == rhs.index_;
+    }
 
     bool operator!=(const span_iterator& rhs) const noexcept { return !(*this == rhs); }
 
@@ -231,9 +250,9 @@ public:
     }
 
     bool operator<=(const span_iterator& rhs) const noexcept { return !(rhs < *this); }
-    
+
     bool operator>(const span_iterator& rhs) const noexcept { return rhs < *this; }
-    
+
     bool operator>=(const span_iterator& rhs) const noexcept { return !(rhs > *this); }
 
     void swap(span_iterator& rhs) noexcept
@@ -250,21 +269,20 @@ private:
 template <typename Span>
 span_iterator<Span> operator+(typename span_iterator<Span>::difference_type n,
     const span_iterator<Span>& rhs) noexcept
-{ return rhs + n; }
+{
+    return rhs + n;
+}
 
 template <typename Span>
 span_iterator<Span> operator-(typename span_iterator<Span>::difference_type n,
     const span_iterator<Span>& rhs) noexcept
-{ return rhs - n; }
-
+{
+    return rhs - n;
+}
 
 } // namespace details
 
 
-// [views.constants], constants  
-constexpr const std::ptrdiff_t dynamic_extent = -1;
-
-
 // [span], class template span 
 template <class ElementType, std::ptrdiff_t Extent>
 class span {
@@ -330,21 +348,23 @@ public:
     constexpr span(span&& other) noexcept = default;
 
     template <class OtherElementType, std::ptrdiff_t OtherExtent,
-        class = std::enable_if_t<!std::is_same<element_type, OtherElementType>::value &&
+        class = std::enable_if_t<
+            details::is_allowed_extent_conversion<OtherExtent, Extent>::value && 
             details::is_allowed_element_type_conversion<OtherElementType, element_type>::value
         >
     >
     constexpr span(const span<OtherElementType, OtherExtent>& other)
-        : storage_(reinterpret_cast<pointer>(other.data()), other.length())
+        : storage_(reinterpret_cast<pointer>(other.data()), extent_type<OtherExtent>(other.size()))
     {}
 
     template <class OtherElementType, std::ptrdiff_t OtherExtent,
-        class = std::enable_if_t<!std::is_same<element_type, OtherElementType>::value &&
+        class = std::enable_if_t<
+            details::is_allowed_extent_conversion<OtherExtent, Extent>::value &&
             details::is_allowed_element_type_conversion<OtherElementType, element_type>::value
         >
     >
     constexpr span(span<OtherElementType, OtherExtent>&& other)
-        : storage_(reinterpret_cast<pointer>(other.data()), other.length())
+        : storage_(reinterpret_cast<pointer>(other.data()), extent_type<OtherExtent>(other.size()))
     {}
 
     ~span() noexcept = default;
@@ -418,6 +438,8 @@ public:
     reverse_iterator rend() const noexcept { return {this, 0}; }
 
 private:
+    constexpr static const bool is_span_type = true;
+
     template <index_type Extent>
     class extent_type;
     
@@ -430,10 +452,11 @@ private:
         constexpr extent_type() noexcept {}
 
         template <index_type Other>
-        constexpr extent_type(extent_type<Other>) noexcept
+        constexpr extent_type(extent_type<Other> ext) noexcept
         {
-            static_assert(Other == Extent,
+            static_assert(Other == Extent || Other == dynamic_extent,
                 "Mismatch between fixed-size extent and size of initializing data.");
+            Expects(ext.size() == Extent);
         }
 
         constexpr extent_type(index_type size)
diff --git a/tests/span_tests.cpp b/tests/span_tests.cpp
index ddae404..81784c3 100644
--- a/tests/span_tests.cpp
+++ b/tests/span_tests.cpp
@@ -395,14 +395,14 @@ SUITE(span_tests)
 
         {
             auto get_an_array = []()->std::array<int, 4> { return{1, 2, 3, 4}; };
-            auto take_a_span = [](span<int> s) { (void)s; };
+            auto take_a_span = [](span<int> s) { static_cast<void>(s); };
             // try to take a temporary std::array
             take_a_span(get_an_array());
         }
 #endif
         {
             auto get_an_array = []() -> std::array<int, 4> { return { 1, 2, 3, 4 }; };
-            auto take_a_span = [](span<const int> s) { (void)s; };
+            auto take_a_span = [](span<const int> s) { static_cast<void>(s); };
             // try to take a temporary std::array
             take_a_span(get_an_array());
         }
@@ -438,7 +438,7 @@ SUITE(span_tests)
 
         {
             auto get_an_array = []() -> const std::array<int, 4> { return {1, 2, 3, 4}; };
-            auto take_a_span = [](span<const int> s) { (void) s; };
+            auto take_a_span = [](span<const int> s) { static_cast<void>(s); };
             // try to take a temporary std::array
             take_a_span(get_an_array());
         }
@@ -482,35 +482,35 @@ SUITE(span_tests)
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             auto get_temp_vector = []() -> std::vector<int> { return {}; };
-            auto use_span = [](span<int> s) { (void) s; };
+            auto use_span = [](span<int> s) { static_cast<void>(s); };
             use_span(get_temp_vector());
 #endif
         }
 
         {
             auto get_temp_vector = []() -> std::vector<int> { return{}; };
-            auto use_span = [](span<const int> s) { (void)s; };
+            auto use_span = [](span<const int> s) { static_cast<void>(s); };
             use_span(get_temp_vector());
         }
 
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             auto get_temp_string = []() -> std::string { return{}; };
-            auto use_span = [](span<char> s) { (void)s; };
+            auto use_span = [](span<char> s) { static_cast<void>(s); };
             use_span(get_temp_string());
 #endif
         }
 
         {
             auto get_temp_string = []() -> std::string { return {}; };
-            auto use_span = [](span<const char> s) { (void) s; };
+            auto use_span = [](span<const char> s) { static_cast<void>(s); };
             use_span(get_temp_string());
         }
 
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             auto get_temp_vector = []() -> const std::vector<int> { return {}; };
-            auto use_span = [](span<const char> s) { (void) s; };
+            auto use_span = [](span<const char> s) { static_cast<void>(s); };
             use_span(get_temp_vector());
 #endif
         }
@@ -518,7 +518,7 @@ SUITE(span_tests)
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             auto get_temp_string = []() -> const std::string { return {}; };
-            auto use_span = [](span<const char> s) { (void) s; };
+            auto use_span = [](span<const char> s) { static_cast<void>(s); };
             use_span(get_temp_string());
 #endif
         }
@@ -536,34 +536,34 @@ SUITE(span_tests)
         {
             span<DerivedClass> avd;
             span<const DerivedClass> avcd = avd;
-            (void)avcd;
+            static_cast<void>(avcd);
         }
 
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             span<DerivedClass> avd;
             span<BaseClass> avb = avd;
-            (void) avb;
+            static_cast<void>(avb);
 #endif
         }
 
         {
             span<int> s;
             span<unsigned int> s2 = s;
-            (void)s2;
+            static_cast<void>(s2);
         }
 
         {
             span<int> s;
             span<const unsigned int> s2 = s;
-            (void)s2;
+            static_cast<void>(s2);
         }
 
         {
 #ifdef CONFIRM_COMPILATION_ERRORS
             span<int> s;
             span<short> s2 = s;
-            (void)s2;
+            static_cast<void>(s2);
 #endif
         }
     }
@@ -984,72 +984,59 @@ SUITE(span_tests)
         }
     }
 
-#if 0
     TEST(fixed_size_conversions)
     {
         int arr[] = {1, 2, 3, 4};
 
         // converting to an span from an equal size array is ok
-        span<int, 4> av4 = arr;
-        CHECK(av4.length() == 4);
+        span<int, 4> s4 = arr;
+        CHECK(s4.length() == 4);
 
-        // converting to dynamic_range a_v is always ok
+        // converting to dynamic_range is always ok
         {
-            span<int, dynamic_range> av = av4;
-            (void) av;
-        }
-        {
-            span<int, dynamic_range> av = arr;
-            (void) av;
+            span<int> s = s4;
+            CHECK(s.length() == s4.length());
+            static_cast<void>(s);
         }
 
 // initialization or assignment to static span that REDUCES size is NOT ok
 #ifdef CONFIRM_COMPILATION_ERRORS
         {
-            span<int, 2> av2 = arr;
+            span<int, 2> s = arr;
         }
         {
-            span<int, 2> av2 = av4;
+            span<int, 2> s2 = s4;
+            static_cast<void>(s2);
         }
 #endif
 
+        // even when done dynamically
         {
-            span<int, dynamic_range> av = arr;
-            span<int, 2> av2 = av;
-            (void) av2;
-        }
-
-#ifdef CONFIRM_COMPILATION_ERRORS
-        {
-            span<int, dynamic_range> av = arr;
-            span<int, 2, 1> av2 = av.as_span(dim<2>(), dim<2>());
-        }
-#endif
-
-        {
-            span<int, dynamic_range> av = arr;
-            span<int, 2, 1> av2 = as_span(av, dim<>(2), dim<>(2));
-            auto workaround_macro = [&]() { return av2[{1, 0}] == 2; };
-            CHECK(workaround_macro());
+            span<int> s = arr;
+            auto f = [&]() {
+                span<int, 2> s2 = s;
+                static_cast<void>(s2);
+            };
+            CHECK_THROW(f(), fail_fast);
         }
 
         // but doing so explicitly is ok
 
         // you can convert statically
         {
-            span<int, 2> av2 = {arr, 2};
-            (void) av2;
+            span<int, 2> s2 = {arr, 2};
+            static_cast<void>(s2);
         }
         {
-            span<int, 1> av2 = av4.first<1>();
-            (void) av2;
+            span<int, 1> s1 = s4.first<1>();
+            static_cast<void>(s1);
         }
 
         // ...or dynamically
         {
-            // NB: implicit conversion to span<int,2> from span<int,dynamic_range>
-            span<int, 1> av2 = av4.first(1);
-            (void) av2;
+            // NB: implicit conversion to span<int,1> from span<int>
+            span<int, 1> s1 = s4.first(1);
+            static_cast<void>(s1);
         }
 
         // initialization or assignment to static span that requires size INCREASE is not ok.
@@ -1057,31 +1044,29 @@ SUITE(span_tests)
 
 #ifdef CONFIRM_COMPILATION_ERRORS
         {
-            span<int, 4> av4 = arr2;
+            span<int, 4> s3 = arr2;
         }
         {
-            span<int, 2> av2 = arr2;
-            span<int, 4> av4 = av2;
+            span<int, 2> s2 = arr2;
+            span<int, 4> s4a = s2;
         }
 #endif
         {
             auto f = [&]() {
-                span<int, 4> av9 = {arr2, 2};
-                (void) av9;
-            };
+                span<int, 4> s4 = {arr2, 2};
+                static_cast<void>(s4);
+            };                     
             CHECK_THROW(f(), fail_fast);
         }
 
-        // this should fail - we are trying to assign a small dynamic a_v to a fixed_size larger one
-        span<int, dynamic_range> av = arr2;
+        // this should fail - we are trying to assign a small dynamic span to a fixed_size larger one
+        span<int> av = arr2;
         auto f = [&]() {
-            span<int, 4> av2 = av;
-            (void) av2;
+            span<int, 4> s4 = av;
+            static_cast<void>(s4);            
         };
         CHECK_THROW(f(), fail_fast);
     }
-
-#endif    
 }
 
 int main(int, const char* []) { return UnitTest::RunAllTests(); }