Prevent external construction of checked iterators (#1263)

* Prevent external construction of checked iterators

Keep iterator state constructors private to span and dyn_array so callers cannot fabricate bounds metadata.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* clang-format

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: carsonRadtke

include/gsl/dyn_array

@@ -34,6 +34,9 @@
 
 namespace gsl
 {
+template <typename T, typename Allocator = std::allocator<T>>
+class dyn_array;
+
 namespace details
 {
     template <typename T, typename Allocator = std::allocator<T>>
@@ -178,13 +181,6 @@ namespace details
         constexpr dyn_array_iterator() = default;
 #endif /* __cpp_lib_ranges >= 201911L */
 
-        constexpr dyn_array_iterator(pointer ptr, size_type pos, size_type end_pos)
-            : _ptr{ptr}, _pos{pos}, _end_pos{end_pos}
-        {
-            Ensures((_ptr != nullptr && _end_pos > 0) || (_ptr == nullptr && _end_pos == 0));
-            Ensures(_pos <= _end_pos);
-        }
-
         constexpr operator dyn_array_iterator<const T>() const { return {_ptr, _pos, _end_pos}; }
 
 #if defined(_MSC_VER) && defined(__cpp_lib_ranges) && (__cpp_lib_ranges >= 201911L)
@@ -281,13 +277,23 @@ namespace details
         }
 
     private:
+        constexpr dyn_array_iterator(pointer ptr, size_type pos, size_type end_pos)
+            : _ptr{ptr}, _pos{pos}, _end_pos{end_pos}
+        {
+            Ensures((_ptr != nullptr && _end_pos > 0) || (_ptr == nullptr && _end_pos == 0));
+            Ensures(_pos <= _end_pos);
+        }
+
         pointer _ptr{};
         size_type _pos{};
         size_type _end_pos{};
+
+        template <typename, typename>
+        friend class ::gsl::dyn_array;
     };
 } // namespace details
 
-template <typename T, typename Allocator = std::allocator<T>>
+template <typename T, typename Allocator>
 class dyn_array : private details::dyn_array_base<T, Allocator>
 {
     using base = details::dyn_array_base<T, Allocator>;

include/gsl/span

@@ -139,12 +139,6 @@ namespace details
 #endif // _MSC_VER
         constexpr span_iterator() = default;
 
-        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
         {
             return {begin_, end_, current_};
@@ -335,10 +329,21 @@ namespace details
         }
 #endif
 
+    private:
+        constexpr span_iterator(pointer begin, pointer end, pointer current)
+            : begin_(begin), end_(end), current_(current)
+        {
+            Expects(begin_ <= current_ && current <= end_);
+        }
+
         pointer begin_ = nullptr;
         pointer end_ = nullptr;
         pointer current_ = nullptr;
 
+        template <class>
+        friend class span_iterator;
+        template <class, std::size_t>
+        friend class ::gsl::span;
         template <typename Ptr>
         friend struct std::pointer_traits;
     };

tests/dyn_array_tests.cpp

@@ -19,6 +19,20 @@ static_assert(sizeof(gsl::dyn_array<int>) == 2 * sizeof(void*),
 static_assert(
     std::is_convertible<gsl::dyn_array<int>::iterator, gsl::dyn_array<int>::const_iterator>::value,
     "gsl::dyn_array iterator should be implicitly convertible to const_iterator");
+static_assert(!std::is_constructible<gsl::dyn_array<int>::iterator, gsl::dyn_array<int>&>::value,
+              "dyn_array<int>::iterator should not be constructible from dyn_array<int>");
+static_assert(
+    !std::is_constructible<gsl::dyn_array<int>::iterator, int*, std::size_t, std::size_t>::value,
+    "dyn_array<int>::iterator should not be constructible from an arbitrary state triple");
+static_assert(
+    !std::is_constructible<gsl::dyn_array<int>::const_iterator, const gsl::dyn_array<int>&>::value,
+    "dyn_array<int>::const_iterator should not be constructible from dyn_array<int>");
+static_assert(!std::is_constructible<gsl::dyn_array<int>::const_iterator, const int*, std::size_t,
+                                     std::size_t>::value,
+              "dyn_array<int>::const_iterator should not be constructible from an arbitrary state "
+              "triple");
+static_assert(std::is_copy_constructible<gsl::dyn_array<int>::iterator>::value,
+              "dyn_array<int>::iterator should remain copy constructible");
 
 #if defined(__cpp_lib_concepts) && (__cpp_lib_concepts >= 202002L)
 static_assert(std::input_iterator<gsl::dyn_array<int>::iterator>,

tests/span_compatibility_tests.cpp

@@ -654,6 +654,19 @@ static_assert(std::is_trivially_copyable<gsl::span<const int, 3>>::value,
 static_assert(std::is_trivially_copyable<gsl::span<const int, 3>::iterator>::value,
               "span<const int, 3>::iterator should be trivially copyable");
 
+static_assert(!std::is_constructible<gsl::span<int>::iterator, gsl::span<int>>::value,
+              "span<int>::iterator should not be constructible from span<int>");
+static_assert(!std::is_constructible<gsl::span<int>::iterator, int*, int*, int*>::value,
+              "span<int>::iterator should not be constructible from an arbitrary pointer triple");
+static_assert(!std::is_constructible<gsl::span<const int>::iterator, gsl::span<const int>>::value,
+              "span<const int>::iterator should not be constructible from span<const int>");
+static_assert(
+    !std::is_constructible<gsl::span<const int>::iterator, const int*, const int*,
+                           const int*>::value,
+    "span<const int>::iterator should not be constructible from an arbitrary pointer triple");
+static_assert(std::is_copy_constructible<gsl::span<int>::iterator>::value,
+              "span<int>::iterator should remain copy constructible");
+
 // nothrow constructible assertions
 static_assert(std::is_nothrow_constructible<gsl::span<int>, int*, std::size_t>::value,
               "std::is_nothrow_constructible<gsl::span<int>, int*, std::size_t>");