Skip to content

Commit

Permalink
[libc++][spaceship] Fixed __debug_three_way_comp's operator() for…
Browse files Browse the repository at this point in the history
… `vector<bool>'s `operator<=>`

An issue with `operator()` was found during the implementation of https://reviews.llvm.org/D132268.

This patch aims to resolve the issues by updating the operator to use perfect forwarding.

The original motivation for `three_way_comp_ref_type` is given in: https://reviews.llvm.org/D131395

`three_way_comp_ref_type`'s implementation is inspired by `comp_ref_type`, which has two overloads:

```
    template <class _Tp, class _Up>
    bool operator()(const _Tp& __x,  const _Up& __y);

    template <class _Tp, class _Up>
    bool operator()(_Tp& __x,  _Up& __y);
```

`__debug_three_way_comp` is missing the first overload and also declares the typealias`_three_way_comp_ref_type ` incorrectly.

Reviewed By: #libc, philnik

Differential Revision: https://reviews.llvm.org/D150188
  • Loading branch information
Zingam committed Jun 10, 2023
1 parent 5914ae3 commit 8fe609c
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
7 changes: 4 additions & 3 deletions libcxx/include/__algorithm/three_way_comp_ref_type.h
Expand Up @@ -13,6 +13,7 @@
#include <__config>
#include <__debug>
#include <__utility/declval.h>
#include <__utility/forward.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
Expand All @@ -28,8 +29,8 @@ struct __debug_three_way_comp {
_LIBCPP_HIDE_FROM_ABI constexpr __debug_three_way_comp(_Comp& __c) : __comp_(__c) {}

template <class _Tp, class _Up>
_LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp& __x, _Up& __y) {
auto __r = __comp_(__x, __y);
_LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp&& __x, _Up&& __y) {
auto __r = __comp_(std::forward<_Tp>(__x), std::forward<_Up>(__y));
__do_compare_assert(0, __y, __x, __r);
return __r;
}
Expand All @@ -55,7 +56,7 @@ struct __debug_three_way_comp {

// Pass the comparator by lvalue reference. Or in debug mode, using a
// debugging wrapper that stores a reference.
# ifndef _LIBCPP_ENABLE_DEBUG_MODE
# ifdef _LIBCPP_ENABLE_DEBUG_MODE
template <class _Comp>
using __three_way_comp_ref_type = __debug_three_way_comp<_Comp>;
# else
Expand Down
Expand Up @@ -16,15 +16,15 @@
// Cmp comp)
// -> decltype(comp(*b1, *b2));

#include <array>
#include <algorithm>
#include <array>
#include <cassert>
#include <compare>
#include <concepts>
#include <limits>

#include "test_macros.h"
#include "test_iterators.h"
#include "test_macros.h"

using std::array;

Expand Down Expand Up @@ -155,7 +155,11 @@ constexpr void test_comparator_invocation_count() {
// The comparator is invoked only `min(left.size(), right.size())` times
test_lexicographical_compare<const int*, const int*>(
std::array{0, 1, 2}, std::array{0, 1, 2, 3}, compare_last_digit_counting, std::strong_ordering::less);
assert(compare_invocation_count == 3);
#ifdef _LIBCPP_ENABLE_DEBUG_MODE
assert(compare_invocation_count <= 6);
#else
assert(compare_invocation_count <= 3);
#endif
}

constexpr bool test() {
Expand Down

0 comments on commit 8fe609c

Please sign in to comment.