Skip to content

Conversation

@celonymire
Copy link
Contributor

@celonymire celonymire commented Nov 14, 2020

Very likely partially addresses #206.

Note: _NODISCARD is already present for ranges function objects.

@celonymire celonymire requested a review from a team as a code owner November 14, 2020 03:29
@ghost
Copy link

ghost commented Nov 14, 2020

CLA assistant check
All CLA requirements met.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Nov 14, 2020
@celonymire
Copy link
Contributor Author

There seems to be a missing test case for compare_three_way (noticed while I was fixing the test), should it be added?

@StephanTLavavej StephanTLavavej self-assigned this Nov 18, 2020
@StephanTLavavej
Copy link
Member

That file is indeed missing compare_three_way coverage, but we have extensive tests in a different file:

// Validate static properties of compare_three_way, compare_three_way_result, and compare_three_way_result_t
template <class T>
concept is_trait = requires {
typename T::type;
};
template <class T, class U>
concept can_three_way = requires(T const& t, U const& u) {
t <=> u;
};
template <class T, class U, class Cat>
constexpr bool test_compare_three_way() {
STATIC_ASSERT(same_as<T, std::remove_cvref_t<T>>);
STATIC_ASSERT(same_as<U, std::remove_cvref_t<U>>);
STATIC_ASSERT(can_three_way<T, U> == !std::is_void_v<Cat>);
STATIC_ASSERT(can_three_way<U, T> == !std::is_void_v<Cat>);
if constexpr (can_three_way<T, U>) {
STATIC_ASSERT(same_as<decltype(std::declval<T const&>() <=> std::declval<U const&>()), Cat>);
STATIC_ASSERT(same_as<decltype(std::declval<U const&>() <=> std::declval<T const&>()), Cat>);
STATIC_ASSERT(same_as<compare_three_way_result_t<T, U>, Cat>);
STATIC_ASSERT(same_as<compare_three_way_result_t<U, T>, Cat>);
STATIC_ASSERT(same_as<typename compare_three_way_result<T, U>::type, Cat>);
STATIC_ASSERT(same_as<typename compare_three_way_result<U, T>::type, Cat>);
STATIC_ASSERT(same_as<decltype(compare_three_way{}(std::declval<T const&>(), std::declval<U const&>())), Cat>);
STATIC_ASSERT(same_as<decltype(compare_three_way{}(std::declval<U const&>(), std::declval<T const&>())), Cat>);
} else {
STATIC_ASSERT(!is_trait<compare_three_way_result<T, U>>);
STATIC_ASSERT(!is_trait<compare_three_way_result<U, T>>);
}
return true;
}
enum class some_enum { value };
STATIC_ASSERT(test_compare_three_way<int, int, strong_ordering>());
STATIC_ASSERT(test_compare_three_way<int, long, strong_ordering>());
STATIC_ASSERT(test_compare_three_way<float, float, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<float, double, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<long, double, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<bool, int, void>());
STATIC_ASSERT(test_compare_three_way<some_enum, some_enum, strong_ordering>());
STATIC_ASSERT(test_compare_three_way<some_enum, int, void>());
STATIC_ASSERT(test_compare_three_way<int*, int*, strong_ordering>());
STATIC_ASSERT(test_compare_three_way<int*, void*, strong_ordering>());
STATIC_ASSERT(test_compare_three_way<int (*)(), int (*)(), void>());
STATIC_ASSERT(test_compare_three_way<int (*)(), void (*)(), void>());
template <class Cat>
struct compares_as {};
template <class Cat1, class Cat2>
bool operator==(compares_as<Cat1> const&, compares_as<Cat2> const&);
template <class Cat1, class Cat2>
common_comparison_category_t<Cat1, Cat2> operator<=>(compares_as<Cat1> const&, compares_as<Cat2> const&);
template <class Cat1, class Cat2>
struct std::common_type<compares_as<Cat1>, compares_as<Cat2>> {
using type = common_comparable;
};
STATIC_ASSERT(test_compare_three_way<compares_as<partial_ordering>, compares_as<partial_ordering>, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<compares_as<partial_ordering>, compares_as<weak_ordering>, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<compares_as<partial_ordering>, compares_as<strong_ordering>, partial_ordering>());
STATIC_ASSERT(test_compare_three_way<compares_as<weak_ordering>, compares_as<weak_ordering>, weak_ordering>());
STATIC_ASSERT(test_compare_three_way<compares_as<weak_ordering>, compares_as<strong_ordering>, weak_ordering>());
STATIC_ASSERT(test_compare_three_way<compares_as<strong_ordering>, compares_as<strong_ordering>, strong_ordering>());
// Validate dynamic properties of compare_three_way, ranges::equal_to, ranges::not_equal_to, ranges::less,
// ranges::less_equal, ranges::greater, ranges::greater_equal
#define assert_three_way(t, u, result) assert(compare_three_way{}((t), (u)) == (result))

So you don't need to add anything. (You can if you really want to.)

@celonymire
Copy link
Contributor Author

Ah okay. I'll just leave it as it is.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great here. 😸

@StephanTLavavej
Copy link
Member

Status update: We simultaneously merge changes on GitHub and our MS-internal repo, so after Thanksgiving we should be able to batch up your PR with everything else that’s ready to merge and get this checked in for VS 2019 16.9 Preview 3.

@StephanTLavavej StephanTLavavej self-assigned this Dec 1, 2020
@StephanTLavavej StephanTLavavej merged commit b4af6e9 into microsoft:master Dec 1, 2020
@StephanTLavavej
Copy link
Member

Thanks for improving the user experience with function objects, and congratulations on your first microsoft/STL commit! 🎉 😺 🚀

This will ship in VS 2019 16.9 Preview 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants