Skip to content

Commit

Permalink
Resolve @StephanTLavavej's PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@cbezault
cbezault committed Aug 18, 2020
1 parent 11f7fb6 commit 8033dc1
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 38 deletions.
86 changes: 59 additions & 27 deletions stl/inc/regex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,8 @@ _NODISCARD auto operator<=>(const sub_match<_BidIt>& _Left, const sub_match<_Bid
return static_cast<typename sub_match<_BidIt>::_Comparison_category>(_Left.compare(_Right) <=> 0);
}

#if 1 // TRANSITION, VSO-900973
// Overload resolution prefers implicit conversions over re-writes.
#ifdef _MSC_VER // TRANSITION, VSO-900973
// The compiler incorrectly performs overload resolution by preferring implicit conversions over rewrites.
// In C++20 mode the only required comparison operators should be == and <=>.
template <class _BidIt>
_NODISCARD bool operator!=(const sub_match<_BidIt>& _Left, const sub_match<_BidIt>& _Right) {
Expand Down Expand Up @@ -797,8 +797,8 @@ _NODISCARD auto operator<=>(const sub_match<_BidIt>& _Left, const _Iter_value_t<
return static_cast<typename sub_match<_BidIt>::_Comparison_category>(_Left.compare(_Right) <=> 0);
}

#if 1 // TRANSITION, VSO-900973
// Overload resolution prefers implicit conversions over re-writes.
#ifdef _MSC_VER // TRANSITION, VSO-900973
// The compiler incorrectly performs overload resolution by preferring implicit conversions over rewrites.
// In C++20 mode the only required comparison operators should be == and <=>.
template <class _BidIt>
_NODISCARD auto operator<=>(const _Iter_value_t<_BidIt>* _Left, const sub_match<_BidIt>& _Right) {
Expand Down Expand Up @@ -930,8 +930,8 @@ _NODISCARD auto operator<=>(const sub_match<_BidIt>& _Left, const _Iter_value_t<
_Left._Compare(_STD addressof(_Right), 1) <=> 0);
}

#if 1 // TRANSITION, VSO-900973
// Overload resolution prefers implicit conversions over re-writes.
#ifdef _MSC_VER // TRANSITION, VSO-900973
// The compiler incorrectly performs overload resolution by preferring implicit conversions over rewrites.
// In C++20 mode the only required comparison operators should be == and <=>.
template <class _BidIt>
_NODISCARD auto operator<=>(const _Iter_value_t<_BidIt>& _Left, const sub_match<_BidIt>& _Right) {
Expand Down Expand Up @@ -1064,15 +1064,51 @@ _NODISCARD auto operator<=>(
return static_cast<typename sub_match<_BidIt>::_Comparison_category>(_Left.compare(_Right) <=> 0);
}

#if 1 // TRANSITION, VSO-900973
// Overload resolution prefers implicit conversions over re-writes.
#ifdef _MSC_VER // TRANSITION, VSO-900973
// The compiler incorrectly performs overload resolution by preferring implicit conversions over rewrites.
// In C++20 mode the only required comparison operators should be == and <=>.
template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD auto operator<=>(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return static_cast<typename sub_match<_BidIt>::_Comparison_category>(0 <=> (_Right <=> _Left));
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator==(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return _Right._Match_equal(_Left.data(), _Left.size());
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator!=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return !(_Left == _Right);
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) < 0;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) > 0;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) <= 0;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) >= 0;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator!=(
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
Expand Down Expand Up @@ -1102,7 +1138,8 @@ _NODISCARD bool operator>=(
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return (_Left <=> _Right) >= 0;
}

#endif // TRANSITION, VSO-900973
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator==(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
Expand All @@ -1118,28 +1155,27 @@ _NODISCARD bool operator!=(
template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) < 0;
return _Right._Greater(_Left.data(), _Left.size());
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) > 0;
return _Right < _Left;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) <= 0;
return !(_Right < _Left);
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return (_Left <=> _Right) >= 0;
return !(_Left < _Right);
}
#endif // TRANSITION, VSO-900973
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator!=(
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
Expand Down Expand Up @@ -1170,41 +1206,37 @@ _NODISCARD bool operator>=(
return !(_Left < _Right);
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator==(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return _Right._Match_equal(_Left.data(), _Left.size());
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator!=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return !(_Left == _Right);
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
return _Right._Greater(_Left.data(), _Left.size());
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return _Left._Less(_Right.data(), _Right.size());
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return _Right < _Left;
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator<=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return !(_Right < _Left);
}

template <class _BidIt, class _Traits, class _Alloc>
_NODISCARD bool operator>=(
const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Left, const sub_match<_BidIt>& _Right) {
const sub_match<_BidIt>& _Left, const basic_string<_Iter_value_t<_BidIt>, _Traits, _Alloc>& _Right) {
return !(_Left < _Right);
}

#endif // !_HAS_CXX20

// INSERT sub_match IN STREAM
Expand Down Expand Up @@ -1418,7 +1450,7 @@ template <class _BidIt, class _Alloc>
_NODISCARD bool operator!=(const match_results<_BidIt, _Alloc>& _Left, const match_results<_BidIt, _Alloc>& _Right) {
return !(_Left == _Right);
}
#endif
#endif // !_HAS_CXX20

// NFA PROPERTIES
const unsigned int _BRE_MAX_GRP = 9U;
Expand Down
32 changes: 21 additions & 11 deletions tests/std/tests/P1614R2_spaceship/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ struct SynthOrdered {

struct OrderedChar {
OrderedChar() = default;
OrderedChar(const char c) : c(c) {}
OrderedChar(const char other) : c(other) {}

OrderedChar& operator=(const char& other) {
c = other;
Expand All @@ -77,12 +77,12 @@ struct OrderedChar {
};

struct WeaklyOrderedChar : OrderedChar {};
struct WeaklyOrderdByOmissionChar : OrderedChar {};
struct WeaklyOrderedByOmissionChar : OrderedChar {};
struct PartiallyOrderedChar : OrderedChar {};

namespace std {
template <>
struct char_traits<OrderedChar> : public char_traits<char> {
struct char_traits<OrderedChar> : char_traits<char> {
using char_type = OrderedChar;

static int compare(const char_type* first1, const char_type* first2, size_t count) {
Expand All @@ -101,18 +101,21 @@ namespace std {
};

template <>
struct char_traits<WeaklyOrderedChar> : public char_traits<OrderedChar> {
struct char_traits<WeaklyOrderedChar> : char_traits<OrderedChar> {
using char_type = WeaklyOrderedChar;
using comparison_category = weak_ordering;
};

template <>
struct char_traits<WeaklyOrderdByOmissionChar> : public char_traits<OrderedChar> {
using char_type = WeaklyOrderdByOmissionChar;
struct char_traits<WeaklyOrderedByOmissionChar> : char_traits<OrderedChar> {
using char_type = WeaklyOrderedByOmissionChar;

private:
using comparison_category = strong_ordering;
};

template <>
struct char_traits<PartiallyOrderedChar> : public char_traits<OrderedChar> {
struct char_traits<PartiallyOrderedChar> : char_traits<OrderedChar> {
using char_type = PartiallyOrderedChar;
using comparison_category = partial_ordering;
};
Expand All @@ -125,9 +128,13 @@ void spaceship_test(const SmallType& smaller, const EqualType& smaller_equal, co
assert(smaller != larger);
assert(larger != smaller);
assert(smaller < larger);
assert(!(larger < smaller));
assert(larger > smaller);
assert(!(smaller > larger));
assert(smaller <= larger);
assert(!(larger <= smaller));
assert(larger >= smaller);
assert(!(smaller >= larger));
assert((smaller <=> larger) < 0);
assert((larger <=> smaller) > 0);
assert((smaller <=> smaller_equal) == 0);
Expand Down Expand Up @@ -368,7 +375,10 @@ void ordering_test_cases() {
const std::string s2{"meow"};
const std::regex all(".*");
const std::regex each(".");
std::smatch m1, m2, m3, m4;
std::smatch m1;
std::smatch m2;
std::smatch m3;
std::smatch m4;

std::regex_match(s1, m1, all);
std::regex_match(s2, m2, all);
Expand All @@ -381,7 +391,7 @@ void ordering_test_cases() {
std::ssub_match sm3 = m3[0];
std::ssub_match sm4 = m4[0];

// TRANSITION, std::char_traits<char> doesn't define comparison_type
// TRANSITION, std::char_traits<char> doesn't define comparison_category
spaceship_test<std::weak_ordering>(sm1, sm1_equal, sm2);
spaceship_test<std::weak_ordering>(sm1, s1, s2);
spaceship_test<std::weak_ordering>(sm1, s1.c_str(), s2.c_str());
Expand All @@ -393,10 +403,10 @@ void ordering_test_cases() {
using StronglyOrderedMatch = std::ssub_match;
using WeaklyOrderedMatch = std::sub_match<std::basic_string<WeaklyOrderedChar>::const_iterator>;
using WeaklyOrderdByOmissionMatch =
std::sub_match<std::basic_string<WeaklyOrderdByOmissionChar>::const_iterator>;
std::sub_match<std::basic_string<WeaklyOrderedByOmissionChar>::const_iterator>;
using PartiallyOrderedMatch = std::sub_match<std::basic_string<PartiallyOrderedChar>::const_iterator>;

// TRANSITION, std::char_traits<char> doesn't define comparison_type
// TRANSITION, std::char_traits<char> doesn't define comparison_category
static_assert(std::is_same_v<SpaceshipType<StronglyOrderedMatch>, std::weak_ordering>);
static_assert(std::is_same_v<SpaceshipType<WeaklyOrderedMatch>, std::weak_ordering>);
static_assert(std::is_same_v<SpaceshipType<WeaklyOrderdByOmissionMatch>, std::weak_ordering>);
Expand Down

0 comments on commit 8033dc1

Please sign in to comment.