[libc++] Implement `operator<=>` for `filesystem::path`

[avogelsgesang]

Summary

Implements part of P1614R2 "The Mothership has Landed"

[avogelsgesang]

libcxx/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp

28

clang-format proposed this change when posting the commit for review. Please let me know if you prefer me to undo it

[avogelsgesang]

need to update based on comments in https://reviews.llvm.org/D130852

[avogelsgesang]

Update based on feedback from D130852

[avogelsgesang]

fix clang-format CI failure (I don't understand why it wants this indentation - but I don't care to fit it...)

[Mordante]

SGTM modulo some nits.

libcxx/include/filesystem

28

Can you update the synopsis to make them friends?

libcxx/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp

134

Maye use the test macros.

134

Can you test for the proper return type too?

[avogelsgesang]

libcxx/include/filesystem

28

not sure where I should put them, if I change them to be friends.

The path class is only forward-declared in the synopsis.
As friends, I should probably move them into the path class.
Does that mean, that I should simply delete the comparisons from the synopsis here, thereby being consistent with the other members of path which are also not shown in the synopsis?

[Mordante]

libcxx/include/filesystem

28

Path has its own synopsis hidden in http://eel.is/c++draft/fs.class.path.general#6

We could add that to the main synopsis. We've done that in other places too, but not in a consistent fashion.
(I know updating the synopsis is a bit of a hassle.)

[avogelsgesang]

address review comment

[avogelsgesang]

libcxx/include/filesystem

28

I have added a synopsis for class path now. Please let me know if this is what you had in mind

[avogelsgesang]

simplify test cases

[avogelsgesang]

add missing include

[avogelsgesang]

fix CI

[avogelsgesang]

fix CI for real

[Mordante]

LGTM, thanks!

libcxx/include/filesystem

17

We normally don't do this, but it's interesting to do this for those "hidden" synopsis.
(I get the impression their number grows.)

[avogelsgesang]

libcxx/include/filesystem

17

is your comment an ask to remove this and follow the established way of not including this?
Or does your " it's interesting to do this for those "hidden" synopsis" imply that it's interesting/useful enough so that we should keep this comment?

[Mordante]

libcxx/include/filesystem

17

Sorry if I was unclear.

I think it's useful and we might even consider doing that for other "hidden" synopsis too. That is sometimes useful during review. I noticed with ranges it's sometimes not easy to find where to look in the Standard.

libcxx/docs/Status/SpaceshipProjects.csv

 | chrono::year_month_day
 | chrono::year_month_day_last",None,Mark de Wever,|Complete|
 "| `[time.zone.nonmembers] <https://wg21.link/time.zone.nonmembers>`_
 | `[time.zone.leap.nonmembers] <https://wg21.link/time.zone.leap.nonmembers>`_
 | `[time.zone.link.nonmembers] <https://wg21.link/time.zone.link.nonmembers>`_","| chrono::time_zone
 | chrono::leap_second
 | chrono::time_zone_link",A ``<chrono>`` implementation,Unassigned,|Not Started|
 | `[fs.filesystem.syn] <https://wg21.link/fs.filesystem.syn>`_,| `filesystem::space_info <https://reviews.llvm.org/D130861>`_,None,Adrian Vogelsgesang,|In Progress|
-| `[fs.path.nonmember] <https://wg21.link/fs.path.nonmember>`_,| `filesystem::path <https://reviews.llvm.org/D130859>`_,None,Adrian Vogelsgesang,|In Progress|
+| `[fs.path.nonmember] <https://wg21.link/fs.path.nonmember>`_,| `filesystem::path <https://reviews.llvm.org/D130859>`_,None,Adrian Vogelsgesang,|Complete|
 | `[fs.dir.entry.obs] <https://wg21.link/fs.dir.entry.obs>`_,| `filesystem::directory_entry <https://reviews.llvm.org/D130860>`_,None,Adrian Vogelsgesang,|In Progress|
 | `[re.submatch.op] <https://wg21.link/re.submatch.op>`_,| sub_match,None,Mark de Wever,|In Progress|
 | `[thread.thread.id] <https://wg21.link/thread.thread.id>`_,| thread::id,None,Unassigned,|Not Started|

libcxx/include/__filesystem/path.h

operator>>(basic_istream<_CharT, _Traits>& __is, path& __p) {
     __p = __tmp;
     return __is;
   }
 #endif // !_LIBCPP_HAS_NO_LOCALIZATION
 
   friend _LIBCPP_HIDE_FROM_ABI bool operator==(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) == 0;
   }
+#  if _LIBCPP_STD_VER <= 17
   friend _LIBCPP_HIDE_FROM_ABI bool operator!=(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) != 0;
   }
   friend _LIBCPP_HIDE_FROM_ABI bool operator<(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) < 0;
   }
   friend _LIBCPP_HIDE_FROM_ABI bool operator<=(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) <= 0;
   }
   friend _LIBCPP_HIDE_FROM_ABI bool operator>(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) > 0;
   }
   friend _LIBCPP_HIDE_FROM_ABI bool operator>=(const path& __lhs, const path& __rhs) noexcept {
     return __lhs.__compare(__rhs.__pn_) >= 0;
   }
+#  else // _LIBCPP_STD_VER <= 17
+  friend _LIBCPP_HIDE_FROM_ABI strong_ordering operator<=>(const path& __lhs, const path& __rhs) noexcept {
+    return __lhs.__compare(__rhs.__pn_) <=> 0;
+  }
+#  endif // _LIBCPP_STD_VER <= 17
 
   friend _LIBCPP_HIDE_FROM_ABI path operator/(const path& __lhs, const path& __rhs) {
     path __result(__lhs);
     __result /= __rhs;
     return __result;
   }
 private:
   inline _LIBCPP_HIDE_FROM_ABI path&

libcxx/include/filesystem

 // -*- 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
 //
 //===----------------------------------------------------------------------===//
 #ifndef _LIBCPP_FILESYSTEM
 #define _LIBCPP_FILESYSTEM
 
 /*
     filesystem synopsis
 
     namespace std::filesystem {
 
     class path;
 
     void swap(path& lhs, path& rhs) noexcept;
     size_t hash_value(const path& p) noexcept;
 
     bool operator==(const path& lhs, const path& rhs) noexcept;
-    bool operator!=(const path& lhs, const path& rhs) noexcept;
-    bool operator< (const path& lhs, const path& rhs) noexcept;
-    bool operator<=(const path& lhs, const path& rhs) noexcept;
-    bool operator> (const path& lhs, const path& rhs) noexcept;
-    bool operator>=(const path& lhs, const path& rhs) noexcept;
+    bool operator!=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+    bool operator< (const path& lhs, const path& rhs) noexcept;             // removed in C++20
+    bool operator<=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+    bool operator> (const path& lhs, const path& rhs) noexcept;             // removed in C++20
+    bool operator>=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+    strong_ordering operator<=>(const path& lhs, const path& rhs) noexcept; // C++20
 
     path operator/ (const path& lhs, const path& rhs);
 
     // fs.path.io operators are friends of path.
     template <class charT, class traits>
     friend basic_ostream<charT, traits>&
     operator<<(basic_ostream<charT, traits>& os, const path& p);
 

libcxx/test/std/input.output/filesystems/class.path/path.member/path.compare.pass.cpp

 // int compare(value_type const*) const;
 //
 // bool operator==(path const&, path const&) noexcept;
 // bool operator!=(path const&, path const&) noexcept;
 // bool operator< (path const&, path const&) noexcept;
 // bool operator<=(path const&, path const&) noexcept;
 // bool operator> (path const&, path const&) noexcept;
 // bool operator>=(path const&, path const&) noexcept;
+// strong_ordering operator<=>(path const&, path const&) noexcept;
 //
 // size_t hash_value(path const&) noexcept;
 
-
 #include "filesystem_include.h"
 #include <type_traits>
 #include <vector>
 #include <cassert>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "count_new.h"
const int E = TC.expect;
 
       // Check runtime result
       assert((p1 == p2) == (E == 0));
       assert((p1 != p2) == (E != 0));
       assert((p1 <  p2) == (E <  0));
       assert((p1 <= p2) == (E <= 0));
       assert((p1 >  p2) == (E >  0));
       assert((p1 >= p2) == (E >= 0));
+#if TEST_STD_VER > 17
+      assert((p1 <=> p2) == (E <=> 0));
+#endif
 
       // Check signatures
       ASSERT_NOEXCEPT(p1 == p2);
       ASSERT_NOEXCEPT(p1 != p2);
       ASSERT_NOEXCEPT(p1 <  p2);
       ASSERT_NOEXCEPT(p1 <= p2);
       ASSERT_NOEXCEPT(p1 >  p2);
       ASSERT_NOEXCEPT(p1 >= p2);
+#if TEST_STD_VER > 17
+      ASSERT_NOEXCEPT(p1 <=> p2);
+#endif
     }
     { // check hash values
       auto h1 = hash_value(p1);
       auto h2 = hash_value(p2);
       assert((h1 == h2) == (p1 == p2));
       // check signature
       ASSERT_SAME_TYPE(size_t, decltype(hash_value(p1)));
       ASSERT_NOEXCEPT(hash_value(p1));