From 7decda9ff8d96285ea8493a2907d0c5d1efb3821 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 10 Dec 2020 13:15:45 -0800 Subject: [PATCH 1/4] Convert contiguous_iterators to pointers correctly ... even in std algorithms that optimize general `contiguous_iterator`s. Fixes #1523. --- stl/inc/algorithm | 12 +++--- stl/inc/xutility | 24 ++++++++--- .../VSO_0180469_ptr_cat/test.compile.pass.cpp | 43 +++++++++++++++++++ 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 29d6bcbefe4..b2181254097 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -2161,9 +2161,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(_First1); - const auto _First2_ch = reinterpret_cast(_First2); - const auto _Count = static_cast(reinterpret_cast(_Last2) - _First2_ch); + const auto _First1_ch = _To_pointer(_First1); + const auto _First2_ch = _To_pointer(_First2); + const auto _Count = static_cast(_To_pointer(_Last2) - _First2_ch); return _CSTD memcmp(_First1_ch, _First2_ch, _Count) == 0; } } @@ -2192,9 +2192,9 @@ bool _Equal_rev_pred_unchecked(_InIt1 _First1, _InIt2 _First2, const _InIt2 _Las template , int> = 0> bool _Equal_rev_pred_unchecked(const _InIt1 _First1, const _InIt2 _First2, const _InIt2 _Last2, _Pr) { // compare [_First1, ...) to [_First2, _Last2), memcmp optimization - const auto _First1_ch = reinterpret_cast(_First1); - const auto _First2_ch = reinterpret_cast(_First2); - const auto _Count = static_cast(reinterpret_cast(_Last2) - _First2_ch); + const auto _First1_ch = _To_pointer(_First1); + const auto _First2_ch = _To_pointer(_First2); + const auto _Count = static_cast(_To_pointer(_Last2) - _First2_ch); return _CSTD memcmp(_First1_ch, _First2_ch, _Count) == 0; } #endif // _HAS_IF_CONSTEXPR diff --git a/stl/inc/xutility b/stl/inc/xutility index a613a5f8548..ae93a99f72a 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5044,10 +5044,22 @@ _INLINE_VAR constexpr bool _Can_memcmp_elements_with_pred = _Can_memcmp_elements template _INLINE_VAR constexpr bool _Iterators_are_contiguous = contiguous_iterator<_Iter1> // && contiguous_iterator<_Iter2>; + +template +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 _INLINE_VAR constexpr bool _Iterators_are_contiguous = conjunction_v, is_pointer<_Iter2>>; + +template +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 @@ -5075,9 +5087,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(_UFirst1); - const auto _First2_ch = reinterpret_cast(_UFirst2); - const auto _Count = static_cast(reinterpret_cast(_ULast1) - _First1_ch); + const auto _First1_ch = _To_pointer(_UFirst1); + const auto _First2_ch = _To_pointer(_UFirst2); + const auto _Count = static_cast(_To_pointer(_ULast1) - _First1_ch); return _CSTD memcmp(_First1_ch, _First2_ch, _Count) == 0; } } @@ -5106,9 +5118,9 @@ bool _Equal_unchecked(_InIt1 _First1, const _InIt1 _Last1, _InIt2 _First2, _Pr _ template , int> = 0> bool _Equal_unchecked(const _InIt1 _First1, const _InIt1 _Last1, const _InIt2 _First2, _Pr) { // compare [_First1, _Last1) to [_First2, ...), memcmp optimization - const auto _First1_ch = reinterpret_cast(_First1); - const auto _First2_ch = reinterpret_cast(_First2); - const auto _Count = static_cast(reinterpret_cast(_Last1) - _First1_ch); + const auto _First1_ch = _To_pointer(_First1); + const auto _First2_ch = _To_pointer(_First2); + const auto _Count = static_cast(_To_pointer(_Last1) - _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 index 8d7c7ea08e8..b5b96a809cf 100644 --- 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,46 @@ void test_Lex_compare_optimize() { test_case_Lex_compare_optimize_pr(); test_case_Lex_compare_optimize_pr(); } + +#ifdef __cpp_lib_concepts +// Also test GH-1523, in which std::equal does not 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& operator*() const; + bool operator==(const gh1523_iter&) const; + gh1523_iter& operator++(); + gh1523_iter operator++(int); + gh1523_iter& operator--(); + gh1523_iter operator--(int); + + friend ptrdiff_t operator-(const gh1523_iter&, const gh1523_iter&); + + strong_ordering operator<=>(const gh1523_iter&) const; + + gh1523_iter& operator-=(ptrdiff_t); + gh1523_iter operator-(ptrdiff_t) const; + friend gh1523_iter operator-(ptrdiff_t, const gh1523_iter&); + gh1523_iter& operator+=(ptrdiff_t); + gh1523_iter operator+(ptrdiff_t) const; + friend gh1523_iter operator+(ptrdiff_t, const gh1523_iter&); + int& operator[](ptrdiff_t) const; +}; + +template <> +struct std::pointer_traits { + using pointer = gh1523_iter; + using element_type = int; + using difference_type = ptrdiff_t; + + static int* to_address(const pointer&) noexcept; +}; +static_assert(contiguous_iterator); + +void test_gh1523() { + (void) std::equal(gh1523_iter{}, gh1523_iter{}, gh1523_iter{}, gh1523_iter{}); +} +#endif // __cpp_lib_concepts From 04cb5c5646196023ebac2a340dfbaf81729e7774 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 10 Dec 2020 13:24:05 -0800 Subject: [PATCH 2/4] Clang-format --- tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 index b5b96a809cf..e3df9ea59ea 100644 --- a/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp +++ b/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp @@ -507,9 +507,9 @@ void test_Lex_compare_optimize() { // Also test GH-1523, in which std::equal does not 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_concept = contiguous_iterator_tag; using iterator_category = random_access_iterator_tag; - using value_type = int; + using value_type = int; int& operator*() const; bool operator==(const gh1523_iter&) const; @@ -533,8 +533,8 @@ struct gh1523_iter { template <> struct std::pointer_traits { - using pointer = gh1523_iter; - using element_type = int; + using pointer = gh1523_iter; + using element_type = int; using difference_type = ptrdiff_t; static int* to_address(const pointer&) noexcept; From c0b5ec069a9b610c620246e5d126726fff5a0de2 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 5 Jan 2021 23:02:10 -0800 Subject: [PATCH 3/4] Code review feedback. --- stl/inc/xutility | 4 ++-- tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index 94ccb401711..df3867075b1 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -4552,7 +4552,7 @@ _INLINE_VAR constexpr bool _Iterators_are_contiguous = contiguous_iterator<_Iter && contiguous_iterator<_Iter2>; template -constexpr _Target* _To_pointer(const _Iter& _It) noexcept { +_NODISCARD constexpr _Target* _To_pointer(const _Iter& _It) noexcept { _STL_INTERNAL_STATIC_ASSERT(contiguous_iterator<_Iter>); return reinterpret_cast<_Target*>(_STD to_address(_It)); } @@ -4562,7 +4562,7 @@ template _INLINE_VAR constexpr bool _Iterators_are_contiguous = conjunction_v, is_pointer<_Iter2>>; template -constexpr _Target* _To_pointer(const _Iter& _It) noexcept { +_NODISCARD constexpr _Target* _To_pointer(const _Iter& _It) noexcept { _STL_INTERNAL_STATIC_ASSERT(is_pointer_v<_Iter>); return reinterpret_cast<_Target*>(_It); } 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 index e3df9ea59ea..3611f32a8cc 100644 --- a/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp +++ b/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp @@ -504,7 +504,7 @@ void test_Lex_compare_optimize() { } #ifdef __cpp_lib_concepts -// Also test GH-1523, in which std::equal does not properly convert non-pointer contiguous iterators to pointers. +// 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; @@ -524,7 +524,6 @@ struct gh1523_iter { gh1523_iter& operator-=(ptrdiff_t); gh1523_iter operator-(ptrdiff_t) const; - friend gh1523_iter operator-(ptrdiff_t, const gh1523_iter&); gh1523_iter& operator+=(ptrdiff_t); gh1523_iter operator+(ptrdiff_t) const; friend gh1523_iter operator+(ptrdiff_t, const gh1523_iter&); @@ -542,6 +541,6 @@ struct std::pointer_traits { static_assert(contiguous_iterator); void test_gh1523() { - (void) std::equal(gh1523_iter{}, gh1523_iter{}, gh1523_iter{}, gh1523_iter{}); + (void) equal(gh1523_iter{}, gh1523_iter{}, gh1523_iter{}, gh1523_iter{}); } #endif // __cpp_lib_concepts From d0846e3c17ea10683d1dce32c5b5a2dd9e498b7f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 7 Jan 2021 16:54:25 -0800 Subject: [PATCH 4/4] Add function definitions so the test will link. --- .../VSO_0180469_ptr_cat/test.compile.pass.cpp | 61 +++++++++++++------ 1 file changed, 43 insertions(+), 18 deletions(-) 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 index 3611f32a8cc..1862d65e7d8 100644 --- a/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp +++ b/tests/std/tests/VSO_0180469_ptr_cat/test.compile.pass.cpp @@ -511,23 +511,46 @@ struct gh1523_iter { using iterator_category = random_access_iterator_tag; using value_type = int; - int& operator*() const; - bool operator==(const gh1523_iter&) const; - gh1523_iter& operator++(); - gh1523_iter operator++(int); - gh1523_iter& operator--(); - gh1523_iter operator--(int); - - friend ptrdiff_t operator-(const gh1523_iter&, const gh1523_iter&); - - strong_ordering operator<=>(const gh1523_iter&) const; - - gh1523_iter& operator-=(ptrdiff_t); - gh1523_iter operator-(ptrdiff_t) const; - gh1523_iter& operator+=(ptrdiff_t); - gh1523_iter operator+(ptrdiff_t) const; - friend gh1523_iter operator+(ptrdiff_t, const gh1523_iter&); - int& operator[](ptrdiff_t) const; + 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 <> @@ -536,7 +559,9 @@ struct std::pointer_traits { using element_type = int; using difference_type = ptrdiff_t; - static int* to_address(const pointer&) noexcept; + static int* to_address(const pointer&) noexcept { + return nullptr; + } }; static_assert(contiguous_iterator);