[libc++] [compare] Named comparison functions, is_eq etc.

[• Quuxplusone]

Summary

Some of these were previously half-implemented in "ordering.h"; now they're all implemented, and tested.

[• Quuxplusone]

libcxx/include/__compare/is_eq.h

24–29

(1) Not marked _LIBCPP_HIDE_FROM_ABI, because I can't imagine their ABI will ever change, but I don't really understand the meaning of the marking, and maybe it's better to mark them; @ldionne opinion?

(2) TIL (again) that although constexpr on a global variable implies static, constexpr on a function implies inline. So it's correct that these header-defined functions aren't explicitly marked inline; they still have the right behavior.

[Mordante]

Would it be possible to handle the comment in test/std/ranges/range.adaptors/range.transform/iterator/compare.pass.cpp:39 in this commit/in a separate commit?

// TODO: when three_way_comparable is implemented and std::is_eq is implemented,

LGTM, but would like to see @ldionne's answer to _LIBCPP_HIDE_FROM_ABI since I think it should be added.

libcxx/include/__compare/ordering.h

326

Interesting only these 4 were implemented instead of all 6.

[rarutyun]

Overall, LGTM except the question with HIDE_FROM_ABI that is already raised and the small comment that I've left.

libcxx/include/__compare/is_eq.h

2

I would prefer to change this file to named_comparison_functions.h with the corresponding changes in guards and module.modulemap. From my perspective it gives better match to what section of the standard this private module implements.

[• Quuxplusone]

libcxx/include/__compare/is_eq.h

2

I'm not strongly opposed to that idea. In fact I certainly would have done that, if these functions had actually been described in a named section like [named.comparison.functions]. Unfortunately, they're submarined into [compare.syn] with no English description anywhere in the standard (and compare_syn.h felt like the wrong answer).
Therefore, the recent-historical default behavior of "name the header after the entity it declares" came into play. I don't think anyone will be confused by how is_eq.h defines both is_eq and is_gt. And vice versa, if I'm looking for the header that defines is_gt, I'll sooner look in is_*.h than in named_*.h.

libcxx/test/libcxx/diagnostics/detail.headers/compare/is_eq.module.verify.cpp

17

@Mordante wrote:

Would it be possible to handle the comment in test/std/ranges/range.adaptors/range.transform/iterator/compare.pass.cpp:39 in this commit/in a separate commit?

In a separate PR, yes. That was actually how I stumbled onto this. It turns out that that test can't be merely uncommented; it needs some actual "design decision" stuff, so I want to do it in a separate PR. (Namely, ContiguousView has iterator type int*, so its iterators are three-way-comparable; which makes ThreeWayComparableView basically redundant. Whereas contiguous_iterator<int*> right now is not three-way-comparable, but I don't know that we should hard-code too many tests that rely on that being forever true. Anyway, separate PR, etc. :))

[• Quuxplusone]

Rebase on main. Add _LIBCPP_HIDE_FROM_ABI, because I still don't know whether it's needed but at least it's consistent with everywhere else. @ldionne ping!

[Mordante]

libcxx/test/libcxx/diagnostics/detail.headers/compare/is_eq.module.verify.cpp

17

When it's indeed more work than uncommenting I also prefer a separate PR.

[ldionne]

LGTM but please consider my inline suggestion!

Thanks a lot for the patch, the operator<=> support has been in need of some attention for a while.

libcxx/include/__compare/is_eq.h

24–29

Yes, we want _LIBCPP_HIDE_FROM_ABI here, like we do everywhere else. I agree in this case we could probably skip it since they are probably never going to change, but we might as well be consistent.

I wasn't aware of the constexpr implies inline bit. I find that surprising and weird. If it were me, I wouldn't rely on that corner case and I'd mark them as inline explicitly, since I expect most readers of the code will go "wut?" if we don't.

[• Quuxplusone]

libcxx/include/__compare/is_eq.h

24–29

If it were me, I wouldn't rely on that corner case and I'd mark them as inline explicitly, since I expect most readers of the code will go "wut?" if we don't.

Works for me! Marked.

libcxx/include/CMakeLists.txt

set(files
   __bits
   __bsd_locale_defaults.h
   __bsd_locale_fallbacks.h
   __charconv/chars_format.h
   __charconv/from_chars_result.h
   __charconv/to_chars_result.h
   __compare/common_comparison_category.h
   __compare/compare_three_way_result.h
+  __compare/is_eq.h
   __compare/ordering.h
   __compare/synth_three_way.h
   __compare/three_way_comparable.h
   __concepts/arithmetic.h
   __concepts/assignable.h
   __concepts/boolean_testable.h
   __concepts/class_or_enum.h
   __concepts/common_reference_with.h

libcxx/include/__compare/is_eq.h

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___COMPARE_IS_EQ_H
+#define _LIBCPP___COMPARE_IS_EQ_H
+
+#include <__compare/ordering.h>
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17
+
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_eq(partial_ordering __c) noexcept { return __c == 0; }
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_neq(partial_ordering __c) noexcept { return __c != 0; }
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_lt(partial_ordering __c) noexcept { return __c < 0; }
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_lteq(partial_ordering __c) noexcept { return __c <= 0; }
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_gt(partial_ordering __c) noexcept { return __c > 0; }
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_gteq(partial_ordering __c) noexcept { return __c >= 0; }
+
+#endif // _LIBCPP_STD_VER > 17
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___COMPARE_IS_EQ_H

libcxx/include/__compare/ordering.h

private:
   _ValueT __value_;
 };
 
 inline constexpr strong_ordering strong_ordering::less(_OrdResult::__less);
 inline constexpr strong_ordering strong_ordering::equal(_OrdResult::__equiv);
 inline constexpr strong_ordering strong_ordering::equivalent(_OrdResult::__equiv);
 inline constexpr strong_ordering strong_ordering::greater(_OrdResult::__greater);
 
-// named comparison functions
-_LIBCPP_HIDE_FROM_ABI
-constexpr bool is_lt(partial_ordering __cmp) noexcept { return __cmp < 0; }
-
-_LIBCPP_HIDE_FROM_ABI
-constexpr bool is_lteq(partial_ordering __cmp) noexcept { return __cmp <= 0; }
-
-_LIBCPP_HIDE_FROM_ABI
-constexpr bool is_gt(partial_ordering __cmp) noexcept { return __cmp > 0; }
-
-_LIBCPP_HIDE_FROM_ABI
-constexpr bool is_gteq(partial_ordering __cmp) noexcept { return __cmp >= 0; }
-
 #endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_SPACESHIP_OPERATOR)
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___COMPARE_ORDERING_H

libcxx/include/compare

public:
     friend constexpr strong_ordering operator<=>(strong_ordering v, unspecified) noexcept;
     friend constexpr strong_ordering operator<=>(unspecified, strong_ordering v) noexcept;
   };
 }
 */
 
 #include <__compare/common_comparison_category.h>
 #include <__compare/compare_three_way_result.h>
+#include <__compare/is_eq.h>
 #include <__compare/ordering.h>
 #include <__compare/three_way_comparable.h>
 #include <__config>
 
 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #pragma GCC system_header
 #endif
 

libcxx/include/module.modulemap

module std [system] {
   }
   module compare {
     header "compare"
     export *
 
     module __compare {
       module common_comparison_category { private header "__compare/common_comparison_category.h" }
       module compare_three_way_result   { private header "__compare/compare_three_way_result.h"   }
+      module is_eq                      { private header "__compare/is_eq.h"                      }
       module ordering                   { private header "__compare/ordering.h"                   }
       module synth_three_way            { private header "__compare/synth_three_way.h"            }
       module three_way_comparable       { private header "__compare/three_way_comparable.h"       }
     }
   }
   module complex {
     header "complex"
     export *

libcxx/test/libcxx/diagnostics/detail.headers/compare/is_eq.module.verify.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__compare/is_eq.h'}}
+#include <__compare/is_eq.h>

libcxx/test/std/language.support/cmp/compare.syn/named_functions.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <compare>
+
+// constexpr bool is_eq  (partial_ordering cmp) noexcept { return cmp == 0; }
+// constexpr bool is_neq (partial_ordering cmp) noexcept { return cmp != 0; }
+// constexpr bool is_lt  (partial_ordering cmp) noexcept { return cmp < 0; }
+// constexpr bool is_lteq(partial_ordering cmp) noexcept { return cmp <= 0; }
+// constexpr bool is_gt  (partial_ordering cmp) noexcept { return cmp > 0; }
+// constexpr bool is_gteq(partial_ordering cmp) noexcept { return cmp >= 0; }
+
+#include <compare>
+#include <cassert>
+
+#include "test_macros.h"
+
+constexpr bool test()
+{
+    assert(!std::is_eq(std::strong_ordering::less));
+    assert( std::is_eq(std::strong_ordering::equal));
+    assert(!std::is_eq(std::strong_ordering::greater));
+    assert(!std::is_eq(std::weak_ordering::less));
+    assert( std::is_eq(std::weak_ordering::equivalent));
+    assert(!std::is_eq(std::weak_ordering::greater));
+    assert(!std::is_eq(std::partial_ordering::less));
+    assert( std::is_eq(std::partial_ordering::equivalent));
+    assert(!std::is_eq(std::partial_ordering::greater));
+    assert(!std::is_eq(std::partial_ordering::unordered));
+
+    assert( std::is_neq(std::strong_ordering::less));
+    assert(!std::is_neq(std::strong_ordering::equal));
+    assert( std::is_neq(std::strong_ordering::greater));
+    assert( std::is_neq(std::weak_ordering::less));
+    assert(!std::is_neq(std::weak_ordering::equivalent));
+    assert( std::is_neq(std::weak_ordering::greater));
+    assert( std::is_neq(std::partial_ordering::less));
+    assert(!std::is_neq(std::partial_ordering::equivalent));
+    assert( std::is_neq(std::partial_ordering::greater));
+    assert( std::is_neq(std::partial_ordering::unordered));
+
+    assert( std::is_lt(std::strong_ordering::less));
+    assert(!std::is_lt(std::strong_ordering::equal));
+    assert(!std::is_lt(std::strong_ordering::greater));
+    assert( std::is_lt(std::weak_ordering::less));
+    assert(!std::is_lt(std::weak_ordering::equivalent));
+    assert(!std::is_lt(std::weak_ordering::greater));
+    assert( std::is_lt(std::partial_ordering::less));
+    assert(!std::is_lt(std::partial_ordering::equivalent));
+    assert(!std::is_lt(std::partial_ordering::greater));
+    assert(!std::is_lt(std::partial_ordering::unordered));
+
+    assert( std::is_lteq(std::strong_ordering::less));
+    assert( std::is_lteq(std::strong_ordering::equal));
+    assert(!std::is_lteq(std::strong_ordering::greater));
+    assert( std::is_lteq(std::weak_ordering::less));
+    assert( std::is_lteq(std::weak_ordering::equivalent));
+    assert(!std::is_lteq(std::weak_ordering::greater));
+    assert( std::is_lteq(std::partial_ordering::less));
+    assert( std::is_lteq(std::partial_ordering::equivalent));
+    assert(!std::is_lteq(std::partial_ordering::greater));
+    assert(!std::is_lteq(std::partial_ordering::unordered));
+
+    assert(!std::is_gt(std::strong_ordering::less));
+    assert(!std::is_gt(std::strong_ordering::equal));
+    assert( std::is_gt(std::strong_ordering::greater));
+    assert(!std::is_gt(std::weak_ordering::less));
+    assert(!std::is_gt(std::weak_ordering::equivalent));
+    assert( std::is_gt(std::weak_ordering::greater));
+    assert(!std::is_gt(std::partial_ordering::less));
+    assert(!std::is_gt(std::partial_ordering::equivalent));
+    assert( std::is_gt(std::partial_ordering::greater));
+    assert(!std::is_gt(std::partial_ordering::unordered));
+
+    assert(!std::is_gteq(std::strong_ordering::less));
+    assert( std::is_gteq(std::strong_ordering::equal));
+    assert( std::is_gteq(std::strong_ordering::greater));
+    assert(!std::is_gteq(std::weak_ordering::less));
+    assert( std::is_gteq(std::weak_ordering::equivalent));
+    assert( std::is_gteq(std::weak_ordering::greater));
+    assert(!std::is_gteq(std::partial_ordering::less));
+    assert( std::is_gteq(std::partial_ordering::equivalent));
+    assert( std::is_gteq(std::partial_ordering::greater));
+    assert(!std::is_gteq(std::partial_ordering::unordered));
+
+    // Test noexceptness.
+    ASSERT_NOEXCEPT(std::is_eq(std::partial_ordering::less));
+    ASSERT_NOEXCEPT(std::is_neq(std::partial_ordering::less));
+    ASSERT_NOEXCEPT(std::is_lt(std::partial_ordering::less));
+    ASSERT_NOEXCEPT(std::is_lteq(std::partial_ordering::less));
+    ASSERT_NOEXCEPT(std::is_gt(std::partial_ordering::less));
+    ASSERT_NOEXCEPT(std::is_gteq(std::partial_ordering::less));
+
+    return true;
+}
+
+int main(int, char**) {
+    test();
+    static_assert(test());
+
+    return 0;
+}