From 9a7897915edbd52468b61310635dff38dbf14513 Mon Sep 17 00:00:00 2001
From: Anna Gringauze <annagrin@microsoft.com>
Date: Fri, 3 Nov 2017 16:13:39 -0700
Subject: [PATCH] Minimize checking in subspan (#569)

* rewrite span subspan checks to help optimizations
* Removed checking pointer for null for subspans. We would never
  check pointer for null in between ptr and ptr+size in the
  original span, so there seems to be  no reason to do so for
  subspans, provided that the subspan's boundaries are ensured
  to be within the range of the original span's boundaries.

This change allows to simplify generated code, for example, to
remove 5 out of 9 branches in code generated from the following
by MSVC, and 4 out 8 branches in clang and gcc-generated code:

span<int> mysubspan(int* p, std::ptrdiff_t size, std::ptrdiff_t i)
{
  if (p != nullptr)
  {
    span<int> s = { p, size };
    return s.subspan(i);
  }

  return { nullptr };
}

Similar effects are achieved for dynamic subspans of static spans,
where in the following code we remove 2 out of 4 branchs in MSVC
and GCC-generated code:

int test_dynamic_subspan_of_static_span(std::ptrdiff_t i)
{
  int x[] = { 0,1,2,3,4,5 };
  span<int, 6> s = { x };
  auto subspan = s.subspan(i);
  return subspan.size();
}
---
 include/gsl/span | 50 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/include/gsl/span b/include/gsl/span
index b828770..7ea1a6c 100644
--- a/include/gsl/span
+++ b/include/gsl/span
@@ -443,15 +443,16 @@ public:
     template <std::ptrdiff_t Count>
     constexpr span<element_type, Count> last() const
     {
-        Expects(Count >= 0 && Count <= size());
+        Expects(Count >= 0 && size() - Count >= 0);
         return {data() + (size() - Count), Count};
     }
 
     template <std::ptrdiff_t Offset, std::ptrdiff_t Count = dynamic_extent>
     constexpr span<element_type, Count> subspan() const
     {
-        Expects((Offset == 0 || (Offset > 0 && Offset <= size())) &&
+        Expects((Offset >= 0 && size() - Offset >= 0) &&
                 (Count == dynamic_extent || (Count >= 0 && Offset + Count <= size())));
+
         return {data() + Offset, Count == dynamic_extent ? size() - Offset : Count};
     }
 
@@ -463,18 +464,16 @@ public:
 
     constexpr span<element_type, dynamic_extent> last(index_type count) const
     {
-        Expects(count >= 0 && count <= size());
-        return {data() + (size() - count), count};
+        return make_subspan(size() - count, dynamic_extent, subspan_selector<Extent>{});
     }
 
     constexpr span<element_type, dynamic_extent> subspan(index_type offset,
                                                          index_type count = dynamic_extent) const
     {
-        Expects((offset == 0 || (offset > 0 && offset <= size())) &&
-                (count == dynamic_extent || (count >= 0 && offset + count <= size())));
-        return {data() + offset, count == dynamic_extent ? size() - offset : count};
+        return make_subspan(offset, count, subspan_selector<Extent>{});
     }
 
+
     // [span.obs], span observers
     constexpr index_type length() const GSL_NOEXCEPT { return size(); }
     constexpr index_type size() const GSL_NOEXCEPT { return storage_.size(); }
@@ -517,10 +516,13 @@ private:
     class storage_type : public ExtentType
     {
     public:
+
+        // checked parameter is needed to remove unnecessary null check in subspans
         template <class OtherExtentType>
-        constexpr storage_type(pointer data, OtherExtentType ext) : ExtentType(ext), data_(data)
+        constexpr storage_type(pointer data, OtherExtentType ext, bool checked = false) : ExtentType(ext), data_(data)
         {
-            Expects((!data && ExtentType::size() == 0) || (data && ExtentType::size() >= 0));
+            Expects(((checked || !data) && ExtentType::size() == 0) ||
+                    ((checked || data) && ExtentType::size() >= 0));
         }
 
         constexpr pointer data() const GSL_NOEXCEPT { return data_; }
@@ -530,8 +532,38 @@ private:
     };
 
     storage_type<details::extent_type<Extent>> storage_;
+
+    // The rest is needed to remove unnecessary null check in subspans
+    constexpr span(pointer ptr, index_type count, bool checked) : storage_(ptr, count, checked) {}
+
+    template <std::ptrdiff_t CallerExtent>
+    class subspan_selector {};
+
+    template <std::ptrdiff_t CallerExtent>
+    span<element_type, dynamic_extent> make_subspan(index_type offset,
+                                                    index_type count,
+                                                    subspan_selector<CallerExtent>) const GSL_NOEXCEPT
+    {
+        span<element_type, dynamic_extent> tmp(*this);
+        return tmp.subspan(offset, count);
+    }
+
+    span<element_type, dynamic_extent> make_subspan(index_type offset,
+                                                    index_type count,
+                                                    subspan_selector<dynamic_extent>) const GSL_NOEXCEPT
+    {
+        Expects(offset >= 0 && size() - offset >= 0);
+        if (count == dynamic_extent)
+        {
+            return { data() + offset, size() - offset, true };
+        }
+
+        Expects(count >= 0 && size() - offset >= count);
+        return { data() + offset,  count, true };
+    }
 };
 
+
 // [span.comparison], span comparison operators
 template <class ElementType, std::ptrdiff_t FirstExtent, std::ptrdiff_t SecondExtent>
 inline constexpr bool operator==(const span<ElementType, FirstExtent>& l,