[libc++][NFC] Move some functions from directory_iterator.cpp to filesystem_common.h

[ldionne]

Summary

This move is going to be needed in order to reuse posix_readdir in
another translation unit. This doesn't change any of the code except
for removing an unused function parameter that otherwise triggers a
warning inside our tests.

[• Quuxplusone]

Did you try moving remove_all into directory_iterator.cpp, and did that not work for some reason?
(If you're just worried about naming, you could s/iterator/iteration/. ;))

[ldionne]

In D118254#3272672, @Quuxplusone wrote:

Did you try moving remove_all into directory_iterator.cpp, and did that not work for some reason?
(If you're just worried about naming, you could s/iterator/iteration/. ;))

No, I did not, because I like the fact that directory_iterator is implemented in directory_iterator.cpp, and that remove_all is defined in operations.cpp, which all makes sense to me. TBH I think the only reason why these functions were not defined in filesystem_common.h were to avoid including <windows.h> in a header, but since that's a private header, I think that's fine. IMO it could have been there from the start, along with all the other helpers.

libcxx/src/filesystem/directory_iterator.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
 //
 //===----------------------------------------------------------------------===//
 
 #include "__config"
 #include "filesystem"
 #include "stack"
-#if defined(_LIBCPP_WIN32API)
-#define WIN32_LEAN_AND_MEAN
-#define NOMINMAX
-#include <windows.h>
-#else
-#include <dirent.h>
-#endif
 #include <errno.h>
 
 #include "filesystem_common.h"
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
-namespace detail {
-namespace {
-
-#if !defined(_LIBCPP_WIN32API)
-
-#if defined(DT_BLK)
-template <class DirEntT, class = decltype(DirEntT::d_type)>
-static file_type get_file_type(DirEntT* ent, int) {
-  switch (ent->d_type) {
-  case DT_BLK:
-    return file_type::block;
-  case DT_CHR:
-    return file_type::character;
-  case DT_DIR:
-    return file_type::directory;
-  case DT_FIFO:
-    return file_type::fifo;
-  case DT_LNK:
-    return file_type::symlink;
-  case DT_REG:
-    return file_type::regular;
-  case DT_SOCK:
-    return file_type::socket;
-  // Unlike in lstat, hitting "unknown" here simply means that the underlying
-  // filesystem doesn't support d_type. Report is as 'none' so we correctly
-  // set the cache to empty.
-  case DT_UNKNOWN:
-    break;
-  }
-  return file_type::none;
-}
-#endif // defined(DT_BLK)
-
-template <class DirEntT>
-static file_type get_file_type(DirEntT* ent, long) {
-  return file_type::none;
-}
-
-static pair<string_view, file_type> posix_readdir(DIR* dir_stream,
-                                                  error_code& ec) {
-  struct dirent* dir_entry_ptr = nullptr;
-  errno = 0; // zero errno in order to detect errors
-  ec.clear();
-  if ((dir_entry_ptr = ::readdir(dir_stream)) == nullptr) {
-    if (errno)
-      ec = capture_errno();
-    return {};
-  } else {
-    return {dir_entry_ptr->d_name, get_file_type(dir_entry_ptr, 0)};
-  }
-}
-#else
-// defined(_LIBCPP_WIN32API)
-
-static file_type get_file_type(const WIN32_FIND_DATAW& data) {
-  if (data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT &&
-      data.dwReserved0 == IO_REPARSE_TAG_SYMLINK)
-    return file_type::symlink;
-  if (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
-    return file_type::directory;
-  return file_type::regular;
-}
-static uintmax_t get_file_size(const WIN32_FIND_DATAW& data) {
-  return (static_cast<uint64_t>(data.nFileSizeHigh) << 32) + data.nFileSizeLow;
-}
-static file_time_type get_write_time(const WIN32_FIND_DATAW& data) {
-  ULARGE_INTEGER tmp;
-  const FILETIME& time = data.ftLastWriteTime;
-  tmp.u.LowPart = time.dwLowDateTime;
-  tmp.u.HighPart = time.dwHighDateTime;
-  return file_time_type(file_time_type::duration(tmp.QuadPart));
-}
-
-#endif
-
-} // namespace
-} // namespace detail
-
 using detail::ErrorHandler;
 
 #if defined(_LIBCPP_WIN32API)
 class __dir_stream {
 public:
   __dir_stream() = delete;
   __dir_stream& operator=(const __dir_stream&) = delete;
 

libcxx/src/filesystem/filesystem_common.h

 #include "climits"
 #include "cstdarg"
 #include "cstdlib"
 #include "ctime"
 #include "filesystem"
 #include "ratio"
 #include "system_error"
 
+#if defined(_LIBCPP_WIN32API)
+# define WIN32_LEAN_AND_MEAN
+# define NOMINMAX
+# include <windows.h>
+#endif
+
 #if !defined(_LIBCPP_WIN32API)
-# include <unistd.h>
+# include <dirent.h>   // for DIR & friends
+# include <fcntl.h>    /* values for fchmodat */
 # include <sys/stat.h>
 # include <sys/statvfs.h>
 # include <sys/time.h> // for ::utimes as used in __last_write_time
-# include <fcntl.h>    /* values for fchmodat */
+# include <unistd.h>
 #endif
 
 #include "../include/apple_availability.h"
 
 #if !defined(__APPLE__)
 // We can use the presence of UTIME_OMIT to detect platforms that provide
 // utimensat.
 #if defined(UTIME_OMIT)

 bool set_file_times(const path& p, std::array<TimeSpec, 2> const& TS,
                     error_code& ec) {
 #if !defined(_LIBCPP_USE_UTIMENSAT)
   return posix_utimes(p, TS, ec);
 #else
   return posix_utimensat(p, TS, ec);
 #endif
 }
-#endif /* !_LIBCPP_WIN32API */
+
+#if defined(DT_BLK)
+template <class DirEntT, class = decltype(DirEntT::d_type)>
+static file_type get_file_type(DirEntT* ent, int) {
+  switch (ent->d_type) {
+  case DT_BLK:
+    return file_type::block;
+  case DT_CHR:
+    return file_type::character;
+  case DT_DIR:
+    return file_type::directory;
+  case DT_FIFO:
+    return file_type::fifo;
+  case DT_LNK:
+    return file_type::symlink;
+  case DT_REG:
+    return file_type::regular;
+  case DT_SOCK:
+    return file_type::socket;
+  // Unlike in lstat, hitting "unknown" here simply means that the underlying
+  // filesystem doesn't support d_type. Report is as 'none' so we correctly
+  // set the cache to empty.
+  case DT_UNKNOWN:
+    break;
+  }
+  return file_type::none;
+}
+#endif // defined(DT_BLK)
+
+template <class DirEntT>
+static file_type get_file_type(DirEntT*, long) {
+  return file_type::none;
+}
+
+static pair<string_view, file_type> posix_readdir(DIR* dir_stream,
+                                                  error_code& ec) {
+  struct dirent* dir_entry_ptr = nullptr;
+  errno = 0; // zero errno in order to detect errors
+  ec.clear();
+  if ((dir_entry_ptr = ::readdir(dir_stream)) == nullptr) {
+    if (errno)
+      ec = capture_errno();
+    return {};
+  } else {
+    return {dir_entry_ptr->d_name, get_file_type(dir_entry_ptr, 0)};
+  }
+}
+
+#else // _LIBCPP_WIN32API
+
+static file_type get_file_type(const WIN32_FIND_DATAW& data) {
+  if (data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT &&
+      data.dwReserved0 == IO_REPARSE_TAG_SYMLINK)
+    return file_type::symlink;
+  if (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+    return file_type::directory;
+  return file_type::regular;
+}
+static uintmax_t get_file_size(const WIN32_FIND_DATAW& data) {
+  return (static_cast<uint64_t>(data.nFileSizeHigh) << 32) + data.nFileSizeLow;
+}
+static file_time_type get_write_time(const WIN32_FIND_DATAW& data) {
+  ULARGE_INTEGER tmp;
+  const FILETIME& time = data.ftLastWriteTime;
+  tmp.u.LowPart = time.dwLowDateTime;
+  tmp.u.HighPart = time.dwHighDateTime;
+  return file_time_type(file_time_type::duration(tmp.QuadPart));
+}
+
+#endif // !_LIBCPP_WIN32API
 
 } // namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
 #endif // FILESYSTEM_COMMON_H