<compare>: Fix 0 <=> partial_ordering::unordered (microsoft#1049)

* Break <compare> ABI again, and fix microsoft#1050. * Remove obsolete conditional compilation from <compare>. Co-authored-by: statementreply <[email protected]> Co-authored-by: Stephan T. Lavavej <[email protected]>
CaseyCarter · Jul 28, 2020 · add0228 · add0228
1 parent 6d39edb
commit add0228
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 172 deletions.
diff --git a/stl/inc/compare b/stl/inc/compare
@@ -32,7 +32,7 @@ using _Compare_t    = signed char;
 // These "pretty" enumerator names are safe since they reuse names of user-facing entities.
 enum class _Compare_eq : _Compare_t { equal = 0, equivalent = equal };
 enum class _Compare_ord : _Compare_t { less = -1, greater = 1 };
-enum class _Compare_ncmp : _Compare_t { unordered = -127 };
+enum class _Compare_ncmp : _Compare_t { unordered = -128 };
 
 // CLASS partial_ordering
 class partial_ordering {
@@ -53,21 +53,7 @@ public:
         return _Val._Value == 0;
     }
 
-#if defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
-    _NODISCARD friend constexpr bool operator==(const partial_ordering&, const partial_ordering&) noexcept = default;
-#else // ^^^ supports <=> and P1185 / supports neither vvv
-    _NODISCARD friend constexpr bool operator!=(const partial_ordering _Val, _Literal_zero) noexcept {
-        return _Val._Value != 0;
-    }
-
-    _NODISCARD friend constexpr bool operator==(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 == _Val._Value;
-    }
-
-    _NODISCARD friend constexpr bool operator!=(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 != _Val._Value;
-    }
-#endif // defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
+    _NODISCARD friend constexpr bool operator==(partial_ordering, partial_ordering) noexcept = default;
 
     _NODISCARD friend constexpr bool operator<(const partial_ordering _Val, _Literal_zero) noexcept {
         return _Val._Value == static_cast<_Compare_t>(_Compare_ord::less);
@@ -78,44 +64,44 @@ public:
     }
 
     _NODISCARD friend constexpr bool operator<=(const partial_ordering _Val, _Literal_zero) noexcept {
-        return _Val._Value <= 0 && _Val._Is_ordered();
+        // The stored value is either less (0xff), equivalent (0x00), greater (0x01), or unordered (0x80).
+        // Subtracting from 0 produces either 0x01, 0x00, 0xff, or 0x80. The result is greater than or equal to 0
+        // if and only if the initial value was less or equivalent, for which we want to return true.
+        return static_cast<signed char>(0 - static_cast<unsigned int>(_Val._Value)) >= 0;
     }
 
     _NODISCARD friend constexpr bool operator>=(const partial_ordering _Val, _Literal_zero) noexcept {
         return _Val._Value >= 0;
     }
 
     _NODISCARD friend constexpr bool operator<(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 < _Val._Value;
+        return _Val > 0;
     }
 
     _NODISCARD friend constexpr bool operator>(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 > _Val._Value && _Val._Is_ordered();
+        return _Val < 0;
     }
 
     _NODISCARD friend constexpr bool operator<=(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 <= _Val._Value;
+        return _Val >= 0;
     }
 
     _NODISCARD friend constexpr bool operator>=(_Literal_zero, const partial_ordering _Val) noexcept {
-        return 0 >= _Val._Value && _Val._Is_ordered();
+        return _Val <= 0;
     }
 
-#ifdef __cpp_impl_three_way_comparison
     _NODISCARD friend constexpr partial_ordering operator<=>(const partial_ordering _Val, _Literal_zero) noexcept {
         return _Val;
     }
 
     _NODISCARD friend constexpr partial_ordering operator<=>(_Literal_zero, const partial_ordering _Val) noexcept {
-        return partial_ordering{static_cast<_Compare_ord>(-_Val._Value)};
+        // The stored value is either less (0xff), equivalent (0x00), greater (0x01), or unordered (0x80).
+        // Subtracting from 0 produces either 0x01, 0x00, 0xff, or 0x80. Note that the effect is to
+        // exchange less for greater (and vice versa), while leaving equivalent and unordered unchanged.
+        return partial_ordering{static_cast<_Compare_ord>(0 - static_cast<unsigned int>(_Val._Value))};
     }
-#endif // __cpp_impl_three_way_comparison
 
 private:
-    _NODISCARD constexpr bool _Is_ordered() const noexcept {
-        return _Value != static_cast<_Compare_t>(_Compare_ncmp::unordered);
-    }
-
     _Compare_t _Value;
 };
 
@@ -144,21 +130,7 @@ public:
         return _Val._Value == 0;
     }
 
-#if defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
-    _NODISCARD friend constexpr bool operator==(const weak_ordering&, const weak_ordering&) noexcept = default;
-#else // ^^^ supports <=> and P1185 / supports neither vvv
-    _NODISCARD friend constexpr bool operator!=(const weak_ordering _Val, _Literal_zero) noexcept {
-        return _Val._Value != 0;
-    }
-
-    _NODISCARD friend constexpr bool operator==(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 == _Val._Value;
-    }
-
-    _NODISCARD friend constexpr bool operator!=(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 != _Val._Value;
-    }
-#endif // defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
+    _NODISCARD friend constexpr bool operator==(weak_ordering, weak_ordering) noexcept = default;
 
     _NODISCARD friend constexpr bool operator<(const weak_ordering _Val, _Literal_zero) noexcept {
         return _Val._Value < 0;
@@ -177,30 +149,28 @@ public:
     }
 
     _NODISCARD friend constexpr bool operator<(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 < _Val._Value;
+        return _Val > 0;
     }
 
     _NODISCARD friend constexpr bool operator>(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 > _Val._Value;
+        return _Val < 0;
     }
 
     _NODISCARD friend constexpr bool operator<=(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 <= _Val._Value;
+        return _Val >= 0;
     }
 
     _NODISCARD friend constexpr bool operator>=(_Literal_zero, const weak_ordering _Val) noexcept {
-        return 0 >= _Val._Value;
+        return _Val <= 0;
     }
 
-#ifdef __cpp_impl_three_way_comparison
     _NODISCARD friend constexpr weak_ordering operator<=>(const weak_ordering _Val, _Literal_zero) noexcept {
         return _Val;
     }
 
     _NODISCARD friend constexpr weak_ordering operator<=>(_Literal_zero, const weak_ordering _Val) noexcept {
         return weak_ordering{static_cast<_Compare_ord>(-_Val._Value)};
     }
-#endif // __cpp_impl_three_way_comparison
 
 private:
     _Compare_t _Value;
@@ -235,21 +205,7 @@ public:
         return _Val._Value == 0;
     }
 
-#if defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
-    _NODISCARD friend constexpr bool operator==(const strong_ordering&, const strong_ordering&) noexcept = default;
-#else // ^^^ supports <=> and P1185 / supports neither vvv
-    _NODISCARD friend constexpr bool operator!=(const strong_ordering _Val, _Literal_zero) noexcept {
-        return _Val._Value != 0;
-    }
-
-    _NODISCARD friend constexpr bool operator==(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 == _Val._Value;
-    }
-
-    _NODISCARD friend constexpr bool operator!=(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 != _Val._Value;
-    }
-#endif // defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
+    _NODISCARD friend constexpr bool operator==(strong_ordering, strong_ordering) noexcept = default;
 
     _NODISCARD friend constexpr bool operator<(const strong_ordering _Val, _Literal_zero) noexcept {
         return _Val._Value < 0;
@@ -268,30 +224,28 @@ public:
     }
 
     _NODISCARD friend constexpr bool operator<(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 < _Val._Value;
+        return _Val > 0;
     }
 
     _NODISCARD friend constexpr bool operator>(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 > _Val._Value;
+        return _Val < 0;
     }
 
     _NODISCARD friend constexpr bool operator<=(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 <= _Val._Value;
+        return _Val >= 0;
     }
 
     _NODISCARD friend constexpr bool operator>=(_Literal_zero, const strong_ordering _Val) noexcept {
-        return 0 >= _Val._Value;
+        return _Val <= 0;
     }
 
-#ifdef __cpp_impl_three_way_comparison
     _NODISCARD friend constexpr strong_ordering operator<=>(const strong_ordering _Val, _Literal_zero) noexcept {
         return _Val;
     }
 
     _NODISCARD friend constexpr strong_ordering operator<=>(_Literal_zero, const strong_ordering _Val) noexcept {
         return strong_ordering{static_cast<_Compare_ord>(-_Val._Value)};
     }
-#endif // __cpp_impl_three_way_comparison
 
 private:
     _Compare_t _Value;
@@ -364,7 +318,7 @@ struct common_comparison_category {
     using type = common_comparison_category_t<_Types...>;
 };
 
-#if defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#ifdef __cpp_lib_concepts
 // clang-format off
 template <class _Ty, class _Cat>
 concept _Compares_as = same_as<common_comparison_category_t<_Ty, _Cat>, _Cat>;
@@ -409,7 +363,7 @@ struct compare_three_way {
     using is_transparent = int;
 };
 // clang-format on
-#endif // defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#endif // __cpp_lib_concepts
 
 // Other components not yet implemented
 _STD_END

diff --git a/stl/inc/span b/stl/inc/span
@@ -12,13 +12,11 @@
 #if !_HAS_CXX20
 #pragma message("The contents of <span> are available only with C++20 or later.")
 #else // ^^^ !_HAS_CXX20 / _HAS_CXX20 vvv
+#include <compare>
 #include <cstddef>
 #include <type_traits>
 #include <xutility>
 
-#if defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
-#include <compare>
-#endif // defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
 
 #pragma pack(push, _CRT_PACKING)
 #pragma warning(push, _STL_WARNING_LEVEL)
@@ -153,39 +151,13 @@ struct _Span_iterator {
         return _Myptr == _Right._Myptr;
     }
 
-#if defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
     _NODISCARD constexpr strong_ordering operator<=>(const _Span_iterator& _Right) const noexcept {
 #if _ITERATOR_DEBUG_LEVEL >= 1
         _STL_VERIFY(
             _Mybegin == _Right._Mybegin && _Myend == _Right._Myend, "cannot compare incompatible span iterators");
 #endif // _ITERATOR_DEBUG_LEVEL >= 1
         return _Myptr <=> _Right._Myptr;
     }
-#else // ^^^ use spaceship / no spaceship vvv
-    _NODISCARD constexpr bool operator!=(const _Span_iterator& _Right) const noexcept {
-        return !(*this == _Right);
-    }
-
-    _NODISCARD constexpr bool operator<(const _Span_iterator& _Right) const noexcept {
-#if _ITERATOR_DEBUG_LEVEL >= 1
-        _STL_VERIFY(
-            _Mybegin == _Right._Mybegin && _Myend == _Right._Myend, "cannot compare incompatible span iterators");
-#endif // _ITERATOR_DEBUG_LEVEL >= 1
-        return _Myptr < _Right._Myptr;
-    }
-
-    _NODISCARD constexpr bool operator>(const _Span_iterator& _Right) const noexcept {
-        return _Right < *this;
-    }
-
-    _NODISCARD constexpr bool operator<=(const _Span_iterator& _Right) const noexcept {
-        return !(_Right < *this);
-    }
-
-    _NODISCARD constexpr bool operator>=(const _Span_iterator& _Right) const noexcept {
-        return !(*this < _Right);
-    }
-#endif // defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201902L
 
 #if _ITERATOR_DEBUG_LEVEL >= 1
     friend constexpr void _Verify_range(const _Span_iterator& _First, const _Span_iterator& _Last) noexcept {

diff --git a/stl/inc/xutility b/stl/inc/xutility
@@ -2105,13 +2105,13 @@ _NODISCARD _CONSTEXPR17 bool operator>=(const reverse_iterator<_BidIt1>& _Left,
 #endif // __cpp_lib_concepts
 { return _Left._Get_current() <= _Right._Get_current(); }
 
-#if defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#ifdef __cpp_lib_concepts
 template <class _BidIt1, three_way_comparable_with<_BidIt1> _BidIt2>
 _NODISCARD constexpr compare_three_way_result_t<_BidIt1, _BidIt2> operator<=>(
     const reverse_iterator<_BidIt1>& _Left, const reverse_iterator<_BidIt2>& _Right) {
     return _Right._Get_current() <=> _Left._Get_current();
 }
-#endif // defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#endif // __cpp_lib_concepts
 
 template <class _BidIt1, class _BidIt2>
 _NODISCARD _CONSTEXPR17 auto operator-(const reverse_iterator<_BidIt1>& _Left, const reverse_iterator<_BidIt2>& _Right)
@@ -4103,12 +4103,12 @@ _NODISCARD _CONSTEXPR17 bool operator==(const move_iterator<_Iter1>& _Left, cons
 #endif // __cpp_lib_concepts
 { return _Left.base() == _Right.base(); }
 
-#if !_HAS_CXX20 || __cpp_impl_three_way_comparison < 201902L
+#if !_HAS_CXX20
 template <class _Iter1, class _Iter2>
 _NODISCARD _CONSTEXPR17 bool operator!=(const move_iterator<_Iter1>& _Left, const move_iterator<_Iter2>& _Right) {
     return !(_Left == _Right);
 }
-#endif // !_HAS_CXX20 || __cpp_impl_three_way_comparison < 201902L
+#endif // !_HAS_CXX20
 
 template <class _Iter1, class _Iter2>
 _NODISCARD _CONSTEXPR17 bool operator<(const move_iterator<_Iter1>& _Left, const move_iterator<_Iter2>& _Right)
@@ -4148,13 +4148,13 @@ _NODISCARD _CONSTEXPR17 bool operator>=(const move_iterator<_Iter1>& _Left, cons
 #endif // __cpp_lib_concepts
 { return !(_Left < _Right); }
 
-#if defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#ifdef __cpp_lib_concepts
 template <class _Iter1, three_way_comparable_with<_Iter1> _Iter2>
 _NODISCARD constexpr compare_three_way_result_t<_Iter1, _Iter2> operator<=>(
     const move_iterator<_Iter1>& _Left, const move_iterator<_Iter2>& _Right) {
     return _Left.base() <=> _Right.base();
 }
-#endif // defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#endif // __cpp_lib_concepts
 
 template <class _Iter1, class _Iter2>
 _NODISCARD _CONSTEXPR17 auto operator-(const move_iterator<_Iter1>& _Left, const move_iterator<_Iter2>& _Right)
@@ -4195,21 +4195,6 @@ namespace _Unreachable_sentinel_detail {
         _NODISCARD friend constexpr bool operator==(const unreachable_sentinel_t&, const _Winc&) noexcept {
             return false;
         }
-#if !defined(__cpp_impl_three_way_comparison) || __cpp_impl_three_way_comparison < 201902L
-        template <weakly_incrementable _Winc>
-        _NODISCARD friend constexpr bool operator==(const _Winc&, const unreachable_sentinel_t&) noexcept {
-            return false;
-        }
-
-        template <weakly_incrementable _Winc>
-        _NODISCARD friend constexpr bool operator!=(const unreachable_sentinel_t&, const _Winc&) noexcept {
-            return true;
-        }
-        template <weakly_incrementable _Winc>
-        _NODISCARD friend constexpr bool operator!=(const _Winc&, const unreachable_sentinel_t&) noexcept {
-            return true;
-        }
-#endif // !defined(__cpp_impl_three_way_comparison) || __cpp_impl_three_way_comparison < 201902L
     };
 } // namespace _Unreachable_sentinel_detail
 struct unreachable_sentinel_t : _Unreachable_sentinel_detail::_Base {}; // TRANSITION, /permissive-
@@ -5236,7 +5221,7 @@ _NODISCARD bool lexicographical_compare(
 }
 #endif // _HAS_CXX17
 
-#if defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#ifdef __cpp_lib_concepts
 // FUNCTION TEMPLATE lexicographical_compare_three_way
 template <class _InIt1, class _InIt2, class _Cmp>
 _NODISCARD constexpr auto lexicographical_compare_three_way(
@@ -5294,7 +5279,7 @@ _NODISCARD constexpr auto lexicographical_compare_three_way(
     _InIt1 _First1, _InIt1 _Last1, _InIt2 _First2, _InIt2 _Last2) {
     return _STD lexicographical_compare_three_way(_First1, _Last1, _First2, _Last2, compare_three_way{});
 }
-#endif // defined(__cpp_impl_three_way_comparison) && defined(__cpp_lib_concepts)
+#endif // __cpp_lib_concepts
 
 // FUNCTION TEMPLATE find
 template <class _Ty>