[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

165

Can you update the synopsis to make them friends?

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

118

Maye use the test macros.

118

Can you test for the proper return type too?

[avogelsgesang]

libcxx/include/filesystem

165

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

165

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

165

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–156

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–156

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–156

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 <https://reviews.llvm.org/D129887>`_
 | `chrono::year_month_day_last <https://reviews.llvm.org/D129887>`_",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,|Complete|
-| `[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 <https://reviews.llvm.org/D131362>`_,None,Adrian Vogelsgesang,|Complete|

libcxx/include/__filesystem/path.h

#endif
   _LIBCPP_HIDE_FROM_ABI
   path& replace_filename(const path& __replacement) {
     remove_filename();
     return (*this /= __replacement);
   }
 
   path& replace_extension(const path& __replacement = path());
 
+  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;
+  }
+
   _LIBCPP_HIDE_FROM_ABI
   void swap(path& __rhs) noexcept { __pn_.swap(__rhs.__pn_); }
 
   // private helper to allow reserving memory in the path
   _LIBCPP_HIDE_FROM_ABI
   void __reserve(size_t __s) { __pn_.reserve(__s); }
 
   // native format observers
#if !defined(_LIBCPP_HAS_NO_LOCALIZATION)
   operator>>(basic_istream<_CharT, _Traits>& __is, path& __p) {
     basic_string<_CharT, _Traits> __tmp;
     __is >> _VSTD::__quoted(__tmp);
     __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;
-  }
-  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;
-  }
-
-  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&
   __assign_view(__string_view const& __s) noexcept {
     __pn_ = string_type(__s);
     return *this;
   }
   string_type __pn_;
 };

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;
+    // `class path` from http://eel.is/c++draft/fs.class.path.general#6
+    class path {
+      public:
+        using value_type  = see below;
+        using string_type = basic_string<value_type>;
+        static constexpr value_type preferred_separator = see below;
+
+        enum format;
+
+        path() noexcept;
+        path(const path& p);
+        path(path&& p) noexcept;
+        path(string_type&& source, format fmt = auto_format);
+        template<class Source>
+          path(const Source& source, format fmt = auto_format);
+        template<class InputIterator>
+          path(InputIterator first, InputIterator last, format fmt = auto_format);
+        template<class Source>
+          path(const Source& source, const locale& loc, format fmt = auto_format);
+        template<class InputIterator>
+          path(InputIterator first, InputIterator last, const locale& loc, format fmt = auto_format);
+        ~path();
 
-    void swap(path& lhs, path& rhs) noexcept;
-    size_t hash_value(const path& p) noexcept;
+        path& operator=(const path& p);
+        path& operator=(path&& p) noexcept;
+        path& operator=(string_type&& source);
+        path& assign(string_type&& source);
+        template<class Source>
+          path& operator=(const Source& source);
+        template<class Source>
+          path& assign(const Source& source);
+        template<class InputIterator>
+          path& assign(InputIterator first, InputIterator last);
 
-    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;
+        path& operator/=(const path& p);
+        template<class Source>
+          path& operator/=(const Source& source);
+        template<class Source>
+          path& append(const Source& source);
+        template<class InputIterator>
+          path& append(InputIterator first, InputIterator last);
 
-    path operator/ (const path& lhs, const path& rhs);
+        path& operator+=(const path& x);
+        path& operator+=(const string_type& x);
+        path& operator+=(basic_string_view<value_type> x);
+        path& operator+=(const value_type* x);
+        path& operator+=(value_type x);
+        template<class Source>
+          path& operator+=(const Source& x);
+        template<class EcharT>
+          path& operator+=(EcharT x);
+        template<class Source>
+          path& concat(const Source& x);
+        template<class InputIterator>
+          path& concat(InputIterator first, InputIterator last);
+
+        void  clear() noexcept;
+        path& make_preferred();
+        path& remove_filename();
+        path& replace_filename(const path& replacement);
+        path& replace_extension(const path& replacement = path());
+        void  swap(path& rhs) noexcept;
+
+        friend bool operator==(const path& lhs, const path& rhs) noexcept;
+        friend bool operator!=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+        friend bool operator< (const path& lhs, const path& rhs) noexcept;             // removed in C++20
+        friend bool operator<=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+        friend bool operator> (const path& lhs, const path& rhs) noexcept;             // removed in C++20
+        friend bool operator>=(const path& lhs, const path& rhs) noexcept;             // removed in C++20
+        friend strong_ordering operator<=>(const path& lhs, const path& rhs) noexcept; // C++20
+
+        friend path operator/(const path& lhs, const path& rhs);
+
+        const string_type& native() const noexcept;
+        const value_type*  c_str() const noexcept;
+        operator string_type() const;
+
+        template<class EcharT, class traits = char_traits<EcharT>,
+                 class Allocator = allocator<EcharT>>
+          basic_string<EcharT, traits, Allocator>
+            string(const Allocator& a = Allocator()) const;
+        std::string    string() const;
+        std::wstring   wstring() const;
+        std::u8string  u8string() const;
+        std::u16string u16string() const;
+        std::u32string u32string() const;
+
+        template<class EcharT, class traits = char_traits<EcharT>,
+                 class Allocator = allocator<EcharT>>
+          basic_string<EcharT, traits, Allocator>
+            generic_string(const Allocator& a = Allocator()) const;
+        std::string    generic_string() const;
+        std::wstring   generic_wstring() const;
+        std::u8string  generic_u8string() const;
+        std::u16string generic_u16string() const;
+        std::u32string generic_u32string() const;
+
+        int compare(const path& p) const noexcept;
+        int compare(const string_type& s) const;
+        int compare(basic_string_view<value_type> s) const;
+        int compare(const value_type* s) const;
+
+        path root_name() const;
+        path root_directory() const;
+        path root_path() const;
+        path relative_path() const;
+        path parent_path() const;
+        path filename() const;
+        path stem() const;
+        path extension() const;
+
+        [[nodiscard]] bool empty() const noexcept;
+        bool has_root_name() const;
+        bool has_root_directory() const;
+        bool has_root_path() const;
+        bool has_relative_path() const;
+        bool has_parent_path() const;
+        bool has_filename() const;
+        bool has_stem() const;
+        bool has_extension() const;
+        bool is_absolute() const;
+        bool is_relative() const;
+
+        path lexically_normal() const;
+        path lexically_relative(const path& base) const;
+        path lexically_proximate(const path& base) const;
+
+        class iterator;
+        using const_iterator = iterator;
+
+        iterator begin() const;
+        iterator end() const;
 
-    // fs.path.io operators are friends of path.
-    template <class charT, class traits>
+        template<class charT, class traits>
           friend basic_ostream<charT, traits>&
             operator<<(basic_ostream<charT, traits>& os, const path& p);
-
-    template <class charT, class traits>
+        template<class charT, class traits>
           friend basic_istream<charT, traits>&
             operator>>(basic_istream<charT, traits>& is, path& p);
+    };
+
+    void swap(path& lhs, path& rhs) noexcept;
+    size_t hash_value(const path& p) noexcept;
 
     template <class Source>
       path u8path(const Source& source);
     template <class InputIterator>
       path u8path(InputIterator first, InputIterator last);
 
     class filesystem_error;
     class directory_entry;
 
     class directory_iterator;
 
     // enable directory_iterator range-based for statements
     directory_iterator begin(directory_iterator iter) noexcept;
     directory_iterator end(directory_iterator) noexcept;

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_comparisons.h"
 #include "test_iterators.h"
 #include "count_new.h"
 #include "filesystem_test_helper.h"
 
 struct PathCompareTest {
   const char* LHS;
   const char* RHS;
   int expect;
const int E = TC.expect;
       assert(ret1 == E);
 
       // check signatures
       ASSERT_NOEXCEPT(p1.compare(p2));
     }
     { // comparison operators
       DisableAllocationGuard g; // none of these operations should allocate
 
-      // 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));
-
-      // 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);
+      // check signatures
+      AssertComparisonsAreNoexcept<path>();
+      AssertComparisonsReturnBool<path>();
+#if TEST_STD_VER > 17
+      AssertOrderAreNoexcept<path>();
+      AssertOrderReturn<std::strong_ordering, path>();
+#endif
+
+      // check comarison results
+      assert(testComparisons(p1, p2, /*isEqual*/ E == 0, /*isLess*/ E < 0));
+#if TEST_STD_VER > 17
+      assert(testOrder(p1, p2, E <=> 0));
+#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));