Merge d988e0e19b into f8ec309118

* improve performance of span_iterator w/ clang

Issue: #1165

Before this PR, the range-for loop was ~3300x slower. After this PR, it
is ~1.005x slower

The clang optimizer is very good at optimizing `current != end`, so
we changed to this idiom. This moves the Expects assertion into the
constructor instead of on the hot-path which is called whenever either
operator++ or operator* is called.

Note: The codegen for the assertion is still a missed optimization,
but less worrisome as it only happens once per iterator.

Note: benchmarks on M1 Macbook Pro w/ Apple Clang 16.0.0

docs/headers.md

@@ -294,6 +294,12 @@ void operator[](std::ptrdiff_t) const = delete;
 
 Array index operator is explicitly deleted. Pointers point to single objects ([I.13: Do not pass an array as a single pointer](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ri-array)), so don't allow treating them as an array.
 
+```cpp
+void swap(not_null<T>& other) { std::swap(ptr_, other.ptr_); }
+```
+
+Swaps contents with another `gsl::not_null` object.
+
 #### Non-member functions
 
 ```cpp
@@ -303,6 +309,13 @@ auto make_not_null(T&& t) noexcept;
 
 Creates a `gsl::not_null` object, deducing the target type from the type of the argument.
 
+```cpp
+template <typename T, typename = std::enable_if_t<std::is_move_assignable<T>::value && std::is_move_constructible<T>::value>>
+void swap(not_null<T>& a, not_null<T>& b);
+```
+
+Swaps the contents of two `gsl::not_null` objects.
+
 ```cpp
 template <class T, class U>
 auto operator==(const not_null<T>& lhs,
@@ -858,3 +871,10 @@ constexpr auto at(std::span<T, extent> sp, const index i) -> decltype(sp[sp.size
 This overload returns a reference to the `i`s element of the `std::span` `sp`. It [`Expects`](#user-content-H-assert-expects) that the provided index is within the bounds of the array.
 
 For [`gsl::at`](#user-content-H-span_ext-at) for [`gsl::span`](#user-content-H-span-span) see header [`span_ext`](#user-content-H-span_ext).
+
+```cpp
+template <class T, std::enable_if_t<std::is_move_assignable<T>::value && std::is_move_constructible<T>::value>>
+void swap(T& a, T& b);
+```
+
+Swaps the contents of two objects. Exists only to specialize `gsl::swap<T>(gsl::not_null<T>&, gsl::not_null<T>&)`.

include/gsl/pointers

@@ -139,10 +139,18 @@ public:
     not_null& operator-=(std::ptrdiff_t) = delete;
     void operator[](std::ptrdiff_t) const = delete;
 
+    void swap(not_null<T>& other) { std::swap(ptr_, other.ptr_); }
+
 private:
     T ptr_;
 };
 
+template <typename T, typename = std::enable_if_t<std::is_move_assignable<T>::value && std::is_move_constructible<T>::value>>
+void swap(not_null<T>& a, not_null<T>& b)
+{
+    a.swap(b);
+}
+
 template <class T>
 auto make_not_null(T&& t) noexcept
 {

include/gsl/span

@@ -140,7 +140,9 @@ namespace details
 
         constexpr span_iterator(pointer begin, pointer end, pointer current)
             : begin_(begin), end_(end), current_(current)
-        {}
+        {
+            Expects(begin_ <= current_ && current <= end_);
+        }
 
         constexpr operator span_iterator<const Type>() const noexcept
         {
@@ -149,21 +151,18 @@ namespace details
 
         constexpr reference operator*() const noexcept
         {
-            Expects(begin_ && end_);
+            Expects(current_ != end_);
-            Expects(begin_ <= current_ && current_ < end_);
             return *current_;
         }
 
         constexpr pointer operator->() const noexcept
         {
-            Expects(begin_ && end_);
+            Expects(current_ != end_);
-            Expects(begin_ <= current_ && current_ < end_);
             return current_;
         }
         constexpr span_iterator& operator++() noexcept
         {
-            Expects(begin_ && current_ && end_);
+            Expects(current_ != end_);
-            Expects(current_ < end_);
             // clang-format off
             GSL_SUPPRESS(bounds.1) // NO-FORMAT: attribute
             // clang-format on
@@ -180,8 +179,7 @@ namespace details
 
         constexpr span_iterator& operator--() noexcept
         {
-            Expects(begin_ && end_);
+            Expects(begin_ != current_);
-            Expects(begin_ < current_);
             --current_;
             return *this;
         }

include/gsl/util

@@ -146,6 +146,9 @@ GSL_SUPPRESS(bounds.1) // NO-FORMAT: attribute
     return *(cont.begin() + i);
 }
 
+template <class T, std::enable_if_t<std::is_move_assignable<T>::value && std::is_move_constructible<T>::value>>
+void swap(T& a, T& b) { std::swap(a, b); }
+
 #if defined(__cpp_lib_span) && __cpp_lib_span >= 202002L
 template <class T, std::size_t extent = std::dynamic_extent>
 constexpr auto at(std::span<T, extent> sp, const index i) -> decltype(sp[sp.size()])

tests/CMakeLists.txt

@@ -205,6 +205,7 @@ add_executable(gsl_tests
     byte_tests.cpp
     notnull_tests.cpp
     owner_tests.cpp
+    pointers_tests.cpp
     span_compatibility_tests.cpp
     span_ext_tests.cpp
     span_tests.cpp

tests/pointers_tests.cpp

@@ -0,0 +1,21 @@
+#include <gtest/gtest.h>
+
+#include <gsl/pointers>
+
+#include <memory>
+
+TEST(pointers_test, swap)
+{
+    // taken from gh-1129:
+    gsl::not_null<std::unique_ptr<int>> a(std::make_unique<int>(0));
+    gsl::not_null<std::unique_ptr<int>> b(std::make_unique<int>(1));
+
+    EXPECT_TRUE(*a == 0);
+    EXPECT_TRUE(*b == 1);
+
+    gsl::swap(a, b);
+
+    EXPECT_TRUE(*a == 1);
+    EXPECT_TRUE(*b == 0);
+}
+