Convert contiguous_iterators to pointers correctly (microsoft#1527)

Co-authored-by: Stephan T. Lavavej <[email protected]>
miscco · Jan 8, 2021 · ac4fde7 · ac4fde7
1 parent 2561974
commit ac4fde7
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 6 deletions.
diff --git a/stl/inc/algorithm b/stl/inc/algorithm
@@ -2123,9 +2123,9 @@ _NODISCARD _CONSTEXPR20 bool _Equal_rev_pred_unchecked(_InIt1 _First1, _InIt2 _F
         if (!_STD is_constant_evaluated())
 #endif // __cpp_lib_is_constant_evaluated
         {
-            const auto _First1_ch = reinterpret_cast<const char*>(_First1);
-            const auto _First2_ch = reinterpret_cast<const char*>(_First2);
-            const auto _Count     = static_cast<size_t>(reinterpret_cast<const char*>(_Last2) - _First2_ch);
+            const auto _First1_ch = _To_pointer<const char>(_First1);
+            const auto _First2_ch = _To_pointer<const char>(_First2);
+            const auto _Count     = static_cast<size_t>(_To_pointer<const char>(_Last2) - _First2_ch);
             return _CSTD memcmp(_First1_ch, _First2_ch, _Count) == 0;
         }
     }

diff --git a/stl/inc/xutility b/stl/inc/xutility
@@ -4560,10 +4560,22 @@ _INLINE_VAR constexpr bool _Can_memcmp_elements_with_pred = _Can_memcmp_elements
 template <class _Iter1, class _Iter2>
 _INLINE_VAR constexpr bool _Iterators_are_contiguous = contiguous_iterator<_Iter1> //
     && contiguous_iterator<_Iter2>;
+
+template <class _Target, class _Iter>
+_NODISCARD constexpr _Target* _To_pointer(const _Iter& _It) noexcept {
+    _STL_INTERNAL_STATIC_ASSERT(contiguous_iterator<_Iter>);
+    return reinterpret_cast<_Target*>(_STD to_address(_It));
+}
 #else // ^^^ defined(__cpp_lib_concepts) ^^^ / vvv !defined(__cpp_lib_concepts) vvv
 // When concepts aren't available, we can detect pointers. (Iterators should be unwrapped before using this.)
 template <class _Iter1, class _Iter2>
 _INLINE_VAR constexpr bool _Iterators_are_contiguous = conjunction_v<is_pointer<_Iter1>, is_pointer<_Iter2>>;
+
+template <class _Target, class _Iter>
+_NODISCARD constexpr _Target* _To_pointer(const _Iter& _It) noexcept {
+    _STL_INTERNAL_STATIC_ASSERT(is_pointer_v<_Iter>);
+    return reinterpret_cast<_Target*>(_It);
+}
 #endif // ^^^ !defined(__cpp_lib_concepts) ^^^
 
 // _Equal_memcmp_is_safe<_Iter1, _Iter2, _Pr> reports whether we can activate the memcmp optimization
@@ -4590,9 +4602,9 @@ _NODISCARD _CONSTEXPR20 bool equal(const _InIt1 _First1, const _InIt1 _Last1, co
         if (!_STD is_constant_evaluated())
 #endif // __cpp_lib_is_constant_evaluated
         {
-            const auto _First1_ch = reinterpret_cast<const char*>(_UFirst1);
-            const auto _First2_ch = reinterpret_cast<const char*>(_UFirst2);
-            const auto _Count     = static_cast<size_t>(reinterpret_cast<const char*>(_ULast1) - _First1_ch);
+            const auto _First1_ch = _To_pointer<const char>(_UFirst1);
+            const auto _First2_ch = _To_pointer<const char>(_UFirst2);
+            const auto _Count     = static_cast<size_t>(_To_pointer<const char>(_ULast1) - _First1_ch);
             return _CSTD memcmp(_First1_ch, _First2_ch, _Count) == 0;
         }
     }

diff --git a/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp b/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp
@@ -502,3 +502,70 @@ void test_Lex_compare_optimize() {
     test_case_Lex_compare_optimize_pr<less>();
     test_case_Lex_compare_optimize_pr<greater>();
 }
+
+#ifdef __cpp_lib_concepts
+// Also test GH-1523, in which std::equal didn't properly convert non-pointer contiguous iterators to pointers.
+struct gh1523_iter {
+    // a contiguous_iterator that doesn't unwrap into a pointer
+    using iterator_concept  = contiguous_iterator_tag;
+    using iterator_category = random_access_iterator_tag;
+    using value_type        = int;
+
+    int* ptr = nullptr;
+
+    // This test is compile-only; the following function definitions allow it to link.
+    int& operator*() const {
+        return *ptr;
+    }
+    gh1523_iter& operator++() {
+        return *this;
+    }
+    gh1523_iter operator++(int) {
+        return {};
+    }
+    gh1523_iter& operator--() {
+        return *this;
+    }
+    gh1523_iter operator--(int) {
+        return {};
+    }
+    ptrdiff_t operator-(const gh1523_iter&) const {
+        return 0;
+    }
+    auto operator<=>(const gh1523_iter&) const = default;
+    gh1523_iter& operator-=(ptrdiff_t) {
+        return *this;
+    }
+    gh1523_iter operator-(ptrdiff_t) const {
+        return {};
+    }
+    gh1523_iter& operator+=(ptrdiff_t) {
+        return *this;
+    }
+    gh1523_iter operator+(ptrdiff_t) const {
+        return {};
+    }
+    friend gh1523_iter operator+(ptrdiff_t, const gh1523_iter&) {
+        return {};
+    }
+    int& operator[](ptrdiff_t) const {
+        return *ptr;
+    }
+};
+
+template <>
+struct std::pointer_traits<gh1523_iter> {
+    using pointer         = gh1523_iter;
+    using element_type    = int;
+    using difference_type = ptrdiff_t;
+
+    static int* to_address(const pointer&) noexcept {
+        return nullptr;
+    }
+};
+static_assert(contiguous_iterator<gh1523_iter>);
+
+void test_gh1523() {
+    (void) equal(gh1523_iter{}, gh1523_iter{}, gh1523_iter{}, gh1523_iter{});
+}
+#endif // __cpp_lib_concepts