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

[avogelsgesang]

Summary

Implements part of P1614R2 "The Mothership has Landed"

[avogelsgesang]

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

[avogelsgesang]

rebase on D130859 and fix oversights

[avogelsgesang]

Fix comment

[Mordante]

SGTM modulo some minor issues.

libcxx/include/__filesystem/directory_entry.h

253

Please use _LIBCPP_HIDE_FROM_ABI in new code and only use one blank line.

254

Please update the synopsis.

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/comparisons.pass.cpp

30

Instead of updating this macro can you use the generic test macros?

[avogelsgesang]

address comments

[avogelsgesang]

fix CI

[avogelsgesang]

fix test cases

[Mordante]

LGTM modulo some comments.

libcxx/include/filesystem

218

XXX ?? That's not in the standard I assume?

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/comparisons.pass.cpp

34

I think in other test you tested only one of these asserts, but no real objection against testing both.

[avogelsgesang]

libcxx/include/filesystem

218

that's my marker for "I still need to change something here"... because if I forget to actually address it and don't remove the marker, reviewers will usually catch it - as you just did :)

in this case, I already did the intended changes: I added the removed in comments which were not there of the copy-pasted synopsis from the standard. But I forgot to remove the marker

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/comparisons.pass.cpp

34

good point! Those should be checking AssertComparisonsAreNoexcept<directory_entry> instead of AssertComparisonsAreNoexcept<path>... damn copy-pasting...

libcxx/docs/Status/SpaceshipProjects.csv

 | 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,|Complete|
 | `[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|
+| `[fs.dir.entry.obs] <https://wg21.link/fs.dir.entry.obs>`_,| `filesystem::directory_entry <https://reviews.llvm.org/D130860>`_,None,Adrian Vogelsgesang,|Complete|
 | `[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/directory_entry.h

public:
   _LIBCPP_INLINE_VISIBILITY
   file_status symlink_status() const { return __get_symlink_status(); }
 
   _LIBCPP_INLINE_VISIBILITY
   file_status symlink_status(error_code& __ec) const noexcept {
     return __get_symlink_status(&__ec);
   }
 
-  _LIBCPP_INLINE_VISIBILITY
-  bool operator<(directory_entry const& __rhs) const noexcept {
-    return __p_ < __rhs.__p_;
-  }
 
   _LIBCPP_INLINE_VISIBILITY
   bool operator==(directory_entry const& __rhs) const noexcept {
     return __p_ == __rhs.__p_;
   }
 
+#if _LIBCPP_STD_VER <= 17
   _LIBCPP_INLINE_VISIBILITY
   bool operator!=(directory_entry const& __rhs) const noexcept {
     return __p_ != __rhs.__p_;
   }
 
   _LIBCPP_INLINE_VISIBILITY
+  bool operator<(directory_entry const& __rhs) const noexcept {
+    return __p_ < __rhs.__p_;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
   bool operator<=(directory_entry const& __rhs) const noexcept {
     return __p_ <= __rhs.__p_;
   }
 
   _LIBCPP_INLINE_VISIBILITY
   bool operator>(directory_entry const& __rhs) const noexcept {
     return __p_ > __rhs.__p_;
   }
 
   _LIBCPP_INLINE_VISIBILITY
   bool operator>=(directory_entry const& __rhs) const noexcept {
     return __p_ >= __rhs.__p_;
   }
 
+#else // _LIBCPP_STD_VER <= 17
+
+  _LIBCPP_HIDE_FROM_ABI
+  strong_ordering operator<=>(const directory_entry& __rhs) const noexcept {
+    return __p_ <=> __rhs.__p_;
+  }
+
+#endif // _LIBCPP_STD_VER <= 17
+
   template <class _CharT, class _Traits>
   _LIBCPP_INLINE_VISIBILITY
   friend basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& __os, const directory_entry& __d) {
     return __os << __d.path();
   }
 
 private:
   friend class directory_iterator;

libcxx/include/filesystem

/*
     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_entry {
+    public:
+      directory_entry() noexcept = default;
+      directory_entry(const directory_entry&) = default;
+      directory_entry(directory_entry&&) noexcept = default;
+      explicit directory_entry(const filesystem::path& p);
+      directory_entry(const filesystem::path& p, error_code& ec);
+      ~directory_entry();
+
+      directory_entry& operator=(const directory_entry&) = default;
+      directory_entry& operator=(directory_entry&&) noexcept = default;
+
+      void assign(const filesystem::path& p);
+      void assign(const filesystem::path& p, error_code& ec);
+      void replace_filename(const filesystem::path& p);
+      void replace_filename(const filesystem::path& p, error_code& ec);
+      void refresh();
+      void refresh(error_code& ec) noexcept;
+
+      const filesystem::path& path() const noexcept;
+      operator const filesystem::path&() const noexcept;
+      bool exists() const;
+      bool exists(error_code& ec) const noexcept;
+      bool is_block_file() const;
+      bool is_block_file(error_code& ec) const noexcept;
+      bool is_character_file() const;
+      bool is_character_file(error_code& ec) const noexcept;
+      bool is_directory() const;
+      bool is_directory(error_code& ec) const noexcept;
+      bool is_fifo() const;
+      bool is_fifo(error_code& ec) const noexcept;
+      bool is_other() const;
+      bool is_other(error_code& ec) const noexcept;
+      bool is_regular_file() const;
+      bool is_regular_file(error_code& ec) const noexcept;
+      bool is_socket() const;
+      bool is_socket(error_code& ec) const noexcept;
+      bool is_symlink() const;
+      bool is_symlink(error_code& ec) const noexcept;
+      uintmax_t file_size() const;
+      uintmax_t file_size(error_code& ec) const noexcept;
+      uintmax_t hard_link_count() const;
+      uintmax_t hard_link_count(error_code& ec) const noexcept;
+      file_time_type last_write_time() const;
+      file_time_type last_write_time(error_code& ec) const noexcept;
+      file_status status() const;
+      file_status status(error_code& ec) const noexcept;
+      file_status symlink_status() const;
+      file_status symlink_status(error_code& ec) const noexcept;
+
+      // XXX
+      bool operator==(const directory_entry& rhs) const noexcept;
+      bool operator!=(const directory_entry& rhs) const noexcept;             // removed  in C++20
+      bool operator< (const directory_entry& rhs) const noexcept;             // removed  in C++20
+      bool operator<=(const directory_entry& rhs) const noexcept;             // removed  in C++20
+      bool operator> (const directory_entry& rhs) const noexcept;             // removed  in C++20
+      bool operator>=(const directory_entry& rhs) const noexcept;             // removed  in C++20
+      strong_ordering operator<=>(const directory_entry& rhs) const noexcept; // since C++20
+
+      template<class charT, class traits>
+        friend basic_ostream<charT, traits>&
+          operator<<(basic_ostream<charT, traits>& os, const directory_entry& d);
+
+    private:
+      filesystem::path pathobject;        // exposition only
+      friend class directory_iterator;    // exposition only
+    };
 
     class directory_iterator;
 
     // enable directory_iterator range-based for statements
     directory_iterator begin(directory_iterator iter) noexcept;
     directory_iterator end(directory_iterator) noexcept;
 
     class recursive_directory_iterator;

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/comparisons.pass.cpp

 // class directory_entry
 
 // bool operator==(directory_entry const&) const noexcept;
 // bool operator!=(directory_entry const&) const noexcept;
 // bool operator< (directory_entry const&) const noexcept;
 // bool operator<=(directory_entry const&) const noexcept;
 // bool operator> (directory_entry const&) const noexcept;
 // bool operator>=(directory_entry const&) const noexcept;
+// strong_ordering operator<=>(directory_entry const&) const noexcept;
 
 #include "filesystem_include.h"
 #include <cassert>
 #include <type_traits>
 #include <utility>
 
 #include "test_macros.h"
+#include "test_comparisons.h"
 
-
-#define CHECK_OP(Op) \
-  static_assert(std::is_same<decltype(ce Op ce), bool>::value, ""); \
-  static_assert(noexcept(ce Op ce), "Operation must be noexcept" )
-
-void test_comparison_signatures() {
+int main(int, char**) {
   using namespace fs;
-  path const p("foo/bar/baz");
-  // Check that the operators are valid, yield bool, and are not
-  // potentially-throwing.
-  {
-    directory_entry const ce(p);
-    CHECK_OP(==);
-    CHECK_OP(!=);
-    CHECK_OP(< );
-    CHECK_OP(<=);
-    CHECK_OP(> );
-    CHECK_OP(>=);
-  }
-}
-#undef CHECK_OP
 
-// The actual semantics of the comparisons are testing via paths operators.
-void test_comparisons_simple() {
-  using namespace fs;
+  AssertComparisonsAreNoexcept<path>();
+  AssertComparisonsReturnBool<path>();
+#if TEST_STD_VER > 17
+  AssertOrderAreNoexcept<path>();
+  AssertOrderReturn<path>();
+#endif
+
   typedef std::pair<path, path> TestType;
-  TestType TestCases[] =
-  {
-      {"", ""},
-      {"", "a"},
-      {"a", "a"},
-      {"a", "b"},
-      {"foo/bar/baz", "foo/bar/baz/"}
-  };
-  auto TestFn = [](path const& LHS, const directory_entry& LHSE,
-                   path const& RHS, const directory_entry& RHSE) {
-    assert((LHS == RHS) == (LHSE == RHSE));
-    assert((LHS != RHS) == (LHSE != RHSE));
-    assert((LHS < RHS) ==  (LHSE < RHSE));
-    assert((LHS <= RHS) == (LHSE <= RHSE));
-    assert((LHS > RHS) ==  (LHSE > RHSE));
-    assert((LHS >= RHS) == (LHSE >= RHSE));
-  };
+  TestType TestCases[] = {{"", ""}, {"", "a"}, {"a", "a"}, {"a", "b"}, {"foo/bar/baz", "foo/bar/baz/"}};
   for (auto const& TC : TestCases) {
-    const directory_entry L(TC.first);
-    const directory_entry R(TC.second);
-    TestFn(TC.first,  L, TC.second, R);
-    TestFn(TC.second, R, TC.first, L);
-  }
+    assert(testComparisonsValues<std::directory_entry>(TC.first, TC.second));
+    assert(testComparisonsValues<std::directory_entry>(TC.second, TC.first));
+#if TEST_STD_VER > 17
+    assert(testOrderValues<std::directory_entry>(TC.first, TC.second));
+    assert(testOrderValues<std::directory_entry>(TC.second, TC.first));
+#endif
   }
 
-int main(int, char**) {
-  test_comparison_signatures();
-  test_comparisons_simple();
-
   return 0;
 }