[libc++][filesystem] Avoid using anonymous namespaces in support headers

[ldionne]

Summary

This avoids using anonymous namespaces in headers and ensures that
the various helper functions get deduplicated across the TUs
implementing <filesystem>. Otherwise, we'd get a definition of
these helper functions in each TU where they are used, which is
entirely unnecessary.

[philnik]

I don't think there is any need to mark the functions as _LIBCPP_HIDE_FROM_ABI. These are all implementation details of the dylib, which is built with -fvisibility=hidden, so these symbols can never escape the dylib, making this pure noise.

[ldionne]

In D152378#4403616, @philnik wrote:

I don't think there is any need to mark the functions as _LIBCPP_HIDE_FROM_ABI. These are all implementation details of the dylib, which is built with -fvisibility=hidden, so these symbols can never escape the dylib, making this pure noise.

These functions live in headers and they are included in multiple .cpp files that are linked together to create the dylib. Without HIDE_FROM_ABI and with the anonymous namespace, we get duplicate definitions in each .o file created using these headers. Also, the compiler complains about internal-linkage functions being unused in some of the .cpp files.

[philnik]

In D152378#4403640, @ldionne wrote:

In D152378#4403616, @philnik wrote:

I don't think there is any need to mark the functions as _LIBCPP_HIDE_FROM_ABI. These are all implementation details of the dylib, which is built with -fvisibility=hidden, so these symbols can never escape the dylib, making this pure noise.

These functions live in headers and they are included in multiple .cpp files that are linked together to create the dylib. Without HIDE_FROM_ABI and with the anonymous namespace, we get duplicate definitions in each .o file created using these headers. Also, the compiler complains about internal-linkage functions being unused in some of the .cpp files.

I understand that, but why not simply remove the anonymous namespace? What's the point of additionally adding _LIBCPP_HIDE_FROM_ABI?

[ldionne]

In D152378#4403648, @philnik wrote:

I understand that, but why not simply remove the anonymous namespace? What's the point of additionally adding _LIBCPP_HIDE_FROM_ABI?

I guess we build the dylib with -fvisibility=hidden, so we don't need it technically. That was the original purpose. We could also argue that it's more consistent with the rest of the code, but I agree it's technically not needed.

So I can either keep it for consistency, or just remove the namespace { } bit. Thoughts?

[philnik]

In D152378#4403678, @ldionne wrote:

In D152378#4403648, @philnik wrote:

I understand that, but why not simply remove the anonymous namespace? What's the point of additionally adding _LIBCPP_HIDE_FROM_ABI?

I guess we build the dylib with -fvisibility=hidden, so we don't need it technically. That was the original purpose. We could also argue that it's more consistent with the rest of the code, but I agree it's technically not needed.

So I can either keep it for consistency, or just remove the namespace { } bit. Thoughts?

Most of the stuff in src/ is written very similar to the headers, but IMO we should strive to make that stuff more maintainable. i.e. avoid anything that isn't required, like _Uglification, adding _LIBCPP_HIDE_FROM_ABI everywhere etc. I'm pretty sure everybody would love to remove that shit in the headers if it weren't necessary.

[ldionne]

Actually I just thought about this and if users are linking against static libc++.a, then they could potentially end up with symbols with the same name from different versions of libc++, which would be an ODR violation. In other words, we do need the same precautions in our own built library as we have in our headers due to static archives. So I believe _LIBCPP_HIDE_FROM_ABI is necessary if we drop the anonymous namespace. Do you agree?

[ldionne]

Rebase to poke CI

[philnik]

In D152378#4404050, @ldionne wrote:

Actually I just thought about this and if users are linking against static libc++.a, then they could potentially end up with symbols with the same name from different versions of libc++, which would be an ODR violation. In other words, we do need the same precautions in our own built library as we have in our headers due to static archives. So I believe _LIBCPP_HIDE_FROM_ABI is necessary if we drop the anonymous namespace. Do you agree?

How would users end up with multiple versions of a function? All the functions are exclusively in libc++.a, and linking against multiple versions or that will definitely blow up.

[Mordante]

In D152378#4403710, @philnik wrote:

In D152378#4403678, @ldionne wrote:

In D152378#4403648, @philnik wrote:

I understand that, but why not simply remove the anonymous namespace? What's the point of additionally adding _LIBCPP_HIDE_FROM_ABI?

I guess we build the dylib with -fvisibility=hidden, so we don't need it technically. That was the original purpose. We could also argue that it's more consistent with the rest of the code, but I agree it's technically not needed.

So I can either keep it for consistency, or just remove the namespace { } bit. Thoughts?

Most of the stuff in src/ is written very similar to the headers, but IMO we should strive to make that stuff more maintainable. i.e. avoid anything that isn't required, like _Uglification, adding _LIBCPP_HIDE_FROM_ABI everywhere etc. I'm pretty sure everybody would love to remove that shit in the headers if it weren't necessary.

I actually prefer consistency in our code base so using _Uglified everywhere. It also makes it easier to move code from the dylib to the header. (Obviously that has its own issues.)

LGTM!

libcxx/src/filesystem/filesystem_common.h

119 ↗

(On Diff #529411)

It would be nice to do this in a followup. The dylib requires C++20.

121 ↗

(On Diff #529411)

IIRC inline is not needed for templates, or am I mistaken?

libcxx/src/filesystem/posix_compat.h

249

This wchar_t is not guarded by _LIBCPP_HAS_NO_WIDE_CHARACTERS, maybe something for a followup patch.

[ldionne]

In D152378#4404281, @philnik wrote:

How would users end up with multiple versions of a function? All the functions are exclusively in libc++.a, and linking against multiple versions or that will definitely blow up.

Why would linking against multiple versions of libc++.a blow up? I know it's not a good idea but I'm curious if you know of specific concerns, except e.g. global stream initialization.

In D152378#4406337, @Mordante wrote:

I actually prefer consistency in our code base so using _Uglified everywhere. It also makes it easier to move code from the dylib to the header. (Obviously that has its own issues.)

That's my preference as well.

libcxx/src/filesystem/filesystem_common.h

119 ↗

(On Diff #529411)

Made a TODO locally.

121 ↗

(On Diff #529411)

This is an explicit specialization, I think those are not linkonce-odr by default. Godbolt confirms this: https://godbolt.org/z/Wzb4Esb35

libcxx/src/filesystem/posix_compat.h

249

Added a todo locally.

[philnik]

In D152378#4414017, @ldionne wrote:

In D152378#4404281, @philnik wrote:

How would users end up with multiple versions of a function? All the functions are exclusively in libc++.a, and linking against multiple versions or that will definitely blow up.

Why would linking against multiple versions of libc++.a blow up? I know it's not a good idea but I'm curious if you know of specific concerns, except e.g. global stream initialization.

You have multiple strong definitions of lots of symbols. How would that not blow up?

[Mordante]

libcxx/src/filesystem/filesystem_common.h

121 ↗

(On Diff #529411)

Thanks for the info!

[ldionne]

In D152378#4414051, @philnik wrote:

In D152378#4414017, @ldionne wrote:

In D152378#4404281, @philnik wrote:

How would users end up with multiple versions of a function? All the functions are exclusively in libc++.a, and linking against multiple versions or that will definitely blow up.

Why would linking against multiple versions of libc++.a blow up? I know it's not a good idea but I'm curious if you know of specific concerns, except e.g. global stream initialization.

You have multiple strong definitions of lots of symbols. How would that not blow up?

Right, obviously, not sure what I was thinking. This means we could just drop the anonymous namespace instead and it would work as well. I'd prefer using HIDE_FROM_ABI because it makes this clearer and more consistent with the rest of the code base, but let's start with just dropping anonymous namespaces.

[ldionne]

Switch from HIDE_FROM_ABI to just inline.

[ldionne]

Rebase to fix format issues.

[ldionne]

Format mis-formatted header.

[ldionne]

Add missing inline on function.

libcxx/src/filesystem/error.h

 #include "format_string.h"
 
 #if defined(_LIBCPP_WIN32API)
 # define WIN32_LEAN_AND_MEAN
 # define NOMINMAX
 # include <windows.h> // ERROR_* macros
 #endif
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
-namespace {
 
 #if defined(_LIBCPP_WIN32API)
 
-errc __win_err_to_errc(int err) {
+inline errc __win_err_to_errc(int err) {
   constexpr struct {
     DWORD win;
     errc errc;
   } win_error_mapping[] = {
       {ERROR_ACCESS_DENIED, errc::permission_denied},
       {ERROR_ALREADY_EXISTS, errc::file_exists},
       {ERROR_BAD_NETPATH, errc::no_such_file_or_directory},
       {ERROR_BAD_PATHNAME, errc::no_such_file_or_directory},
inline errc __win_err_to_errc(int err) {
   for (const auto &pair : win_error_mapping)
     if (pair.win == static_cast<DWORD>(err))
       return pair.errc;
   return errc::invalid_argument;
 }
 
 #endif // _LIBCPP_WIN32API
 
-error_code capture_errno() {
+inline error_code capture_errno() {
   _LIBCPP_ASSERT(errno != 0, "Expected errno to be non-zero");
   return error_code(errno, generic_category());
 }
 
 #if defined(_LIBCPP_WIN32API)
-error_code make_windows_error(int err) {
+inline error_code make_windows_error(int err) {
   return make_error_code(__win_err_to_errc(err));
 }
 #endif
 
 template <class T>
 T error_value();
 template <>
-_LIBCPP_CONSTEXPR_SINCE_CXX14 void error_value<void>() {}
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 void error_value<void>() {}
 template <>
-bool error_value<bool>() {
+inline bool error_value<bool>() {
   return false;
 }
 #if __SIZEOF_SIZE_T__ != __SIZEOF_LONG_LONG__
 template <>
-size_t error_value<size_t>() {
+inline size_t error_value<size_t>() {
   return size_t(-1);
 }
 #endif
 template <>
-uintmax_t error_value<uintmax_t>() {
+inline uintmax_t error_value<uintmax_t>() {
   return uintmax_t(-1);
 }
 template <>
-_LIBCPP_CONSTEXPR_SINCE_CXX14 file_time_type error_value<file_time_type>() {
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 file_time_type error_value<file_time_type>() {
   return file_time_type::min();
 }
 template <>
-path error_value<path>() {
+inline path error_value<path>() {
   return {};
 }
 
 template <class T>
 struct ErrorHandler {
   const char* func_name_;
   error_code* ec_ = nullptr;
   const path* p1_ = nullptr;
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
     return error_value<T>();
   }
 
 private:
   ErrorHandler(ErrorHandler const&) = delete;
   ErrorHandler& operator=(ErrorHandler const&) = delete;
 };
 
-} // end anonymous namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
 #endif // FILESYSTEM_ERROR_H

libcxx/src/filesystem/file_descriptor.h

 #else
 # include <dirent.h>   // for DIR & friends
 # include <fcntl.h>    // values for fchmodat
 # include <sys/stat.h>
 # include <sys/statvfs.h>
 # include <unistd.h>
 #endif // defined(_LIBCPP_WIN32API)
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _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) {
+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:
file_type get_file_type(DirEntT* ent, int) {
   case DT_UNKNOWN:
     break;
   }
   return file_type::none;
 }
 #endif // defined(DT_BLK)
 
 template <class DirEntT>
-static file_type get_file_type(DirEntT*, long) {
+file_type get_file_type(DirEntT*, long) {
   return file_type::none;
 }
 
-static pair<string_view, file_type> posix_readdir(DIR* dir_stream,
+inline 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) {
+inline 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) {
+inline 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) {
+inline 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
struct FileDescriptor {
 
   FileDescriptor(FileDescriptor const&) = delete;
   FileDescriptor& operator=(FileDescriptor const&) = delete;
 
 private:
   explicit FileDescriptor(const path* p, int descriptor = -1) : name(*p), fd(descriptor) {}
 };
 
-perms posix_get_perms(const StatT& st) noexcept {
+inline perms posix_get_perms(const StatT& st) noexcept {
   return static_cast<perms>(st.st_mode) & perms::mask;
 }
 
-file_status create_file_status(error_code& m_ec, path const& p,
+inline file_status create_file_status(error_code& m_ec, path const& p,
                                       const StatT& path_stat, error_code* ec) {
   if (ec)
     *ec = m_ec;
   if (m_ec && (m_ec.value() == ENOENT || m_ec.value() == ENOTDIR)) {
     return file_status(file_type::not_found);
   } else if (m_ec) {
     ErrorHandler<void> err("posix_stat", ec, &p);
     err.report(m_ec, "failed to determine attributes for the specified path");
     return file_status(file_type::none);
else if (S_ISSOCK(mode))
     fs_tmp.type(file_type::socket);
   else
     fs_tmp.type(file_type::unknown);
 
   fs_tmp.permissions(detail::posix_get_perms(path_stat));
   return fs_tmp;
 }
 
-file_status posix_stat(path const& p, StatT& path_stat, error_code* ec) {
+inline file_status posix_stat(path const& p, StatT& path_stat, error_code* ec) {
   error_code m_ec;
   if (detail::stat(p.c_str(), &path_stat) == -1)
     m_ec = detail::capture_errno();
   return create_file_status(m_ec, p, path_stat, ec);
 }
 
-file_status posix_stat(path const& p, error_code* ec) {
+inline file_status posix_stat(path const& p, error_code* ec) {
   StatT path_stat;
   return posix_stat(p, path_stat, ec);
 }
 
-file_status posix_lstat(path const& p, StatT& path_stat, error_code* ec) {
+inline file_status posix_lstat(path const& p, StatT& path_stat, error_code* ec) {
   error_code m_ec;
   if (detail::lstat(p.c_str(), &path_stat) == -1)
     m_ec = detail::capture_errno();
   return create_file_status(m_ec, p, path_stat, ec);
 }
 
-file_status posix_lstat(path const& p, error_code* ec) {
+inline file_status posix_lstat(path const& p, error_code* ec) {
   StatT path_stat;
   return posix_lstat(p, path_stat, ec);
 }
 
 // http://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html
-bool posix_ftruncate(const FileDescriptor& fd, off_t to_size, error_code& ec) {
+inline bool posix_ftruncate(const FileDescriptor& fd, off_t to_size, error_code& ec) {
   if (detail::ftruncate(fd.fd, to_size) == -1) {
     ec = capture_errno();
     return true;
   }
   ec.clear();
   return false;
 }
 
-bool posix_fchmod(const FileDescriptor& fd, const StatT& st, error_code& ec) {
+inline bool posix_fchmod(const FileDescriptor& fd, const StatT& st, error_code& ec) {
   if (detail::fchmod(fd.fd, st.st_mode) == -1) {
     ec = capture_errno();
     return true;
   }
   ec.clear();
   return false;
 }
 
-bool stat_equivalent(const StatT& st1, const StatT& st2) {
+inline bool stat_equivalent(const StatT& st1, const StatT& st2) {
   return (st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
 }
 
-file_status FileDescriptor::refresh_status(error_code& ec) {
+inline file_status FileDescriptor::refresh_status(error_code& ec) {
   // FD must be open and good.
   m_status = file_status{};
   m_stat = {};
   error_code m_ec;
   if (detail::fstat(fd, &m_stat) == -1)
     m_ec = capture_errno();
   m_status = create_file_status(m_ec, name, m_stat, &ec);
   return m_status;
 }
 
-} // namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_DIAGNOSTIC_POP
-
 #endif // FILESYSTEM_FILE_DESCRIPTOR_H

libcxx/src/filesystem/format_string.h

 #if defined(_LIBCPP_WIN32API)
 #  define PATHSTR(x) (L##x)
 #  define PATH_CSTR_FMT "\"%ls\""
 #else
 #  define PATHSTR(x) (x)
 #  define PATH_CSTR_FMT "\"%s\""
 #endif
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
-namespace {
 
-_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 0)
-string vformat_string(const char* msg, va_list ap) {
+inline _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 0) string vformat_string(const char* msg, va_list ap) {
   array<char, 256> buf;
 
   va_list apcopy;
   va_copy(apcopy, ap);
   int ret = ::vsnprintf(buf.data(), buf.size(), msg, apcopy);
   va_end(apcopy);
 
   string result;
   if (static_cast<size_t>(ret) < buf.size()) {
     result.assign(buf.data(), static_cast<size_t>(ret));
   } else {
     // we did not provide a long enough buffer on our first attempt. The
     // return value is the number of bytes (excluding the null byte) that are
     // needed for formatting.
     size_t size_with_null = static_cast<size_t>(ret) + 1;
     result.__resize_default_init(size_with_null - 1);
     ret = ::vsnprintf(&result[0], size_with_null, msg, ap);
     _LIBCPP_ASSERT(static_cast<size_t>(ret) == (size_with_null - 1), "TODO");
   }
   return result;
 }
 
-_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2)
-string format_string(const char* msg, ...) {
+inline _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2) string format_string(const char* msg, ...) {
   string ret;
   va_list ap;
   va_start(ap, msg);
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   try {
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
     ret = detail::vformat_string(msg, ap);
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
   } catch (...) {
     va_end(ap);
     throw;
   }
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
   va_end(ap);
   return ret;
 }
 
-} // end anonymous namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_DIAGNOSTIC_POP
-
 #endif // FILESYSTEM_FORMAT_STRING_H

libcxx/src/filesystem/operations.cpp

if (to_exists) {
 
     // Set the permissions and truncate the file we opened.
     if (detail::posix_fchmod(to_fd, from_stat, m_ec))
       return err.report(m_ec);
     if (detail::posix_ftruncate(to_fd, 0, m_ec))
       return err.report(m_ec);
   }
 
-  if (!copy_file_impl(from_fd, to_fd, m_ec)) {
+  if (!detail::copy_file_impl(from_fd, to_fd, m_ec)) {
     // FIXME: Remove the dest file if we failed, and it didn't exist previously.
     return err.report(m_ec);
   }
 
   return true;
 }
 
 void __copy_symlink(const path& existing_symlink, const path& new_symlink,

libcxx/src/filesystem/path_parser.h

 #include <__config>
 #include <__utility/unreachable.h>
 #include <cstddef>
 #include <filesystem>
 #include <utility>
 
 #include "format_string.h"
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
-namespace {
-
-bool isSeparator(path::value_type C) {
+inline bool isSeparator(path::value_type C) {
   if (C == '/')
     return true;
 #if defined(_LIBCPP_WIN32API)
   if (C == '\\')
     return true;
 #endif
   return false;
 }
 
-bool isDriveLetter(path::value_type C) {
+inline bool isDriveLetter(path::value_type C) {
   return (C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z');
 }
 
 namespace parser {
 
 using string_view_t = path::__string_view;
 using string_view_pair = pair<string_view_t, string_view_t>;
 using PosPtr = path::value_type const*;
if (PosPtr Ret = consumeDriveLetter(P, End))
       return Ret;
     if (PosPtr Ret = consumeNetworkRoot(P, End))
       return Ret;
 #endif
     return nullptr;
   }
 };
 
-string_view_pair separate_filename(string_view_t const& s) {
+inline string_view_pair separate_filename(string_view_t const& s) {
   if (s == PATHSTR(".") || s == PATHSTR("..") || s.empty())
     return string_view_pair{s, PATHSTR("")};
   auto pos = s.find_last_of('.');
   if (pos == string_view_t::npos || pos == 0)
     return string_view_pair{s, string_view_t{}};
   return string_view_pair{s.substr(0, pos), s.substr(pos)};
 }
 
-string_view_t createView(PosPtr S, PosPtr E) noexcept {
+inline string_view_t createView(PosPtr S, PosPtr E) noexcept {
   return {S, static_cast<size_t>(E - S) + 1};
 }
 
 } // namespace parser
-} // namespace
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_DIAGNOSTIC_POP
-
 #endif // PATH_PARSER_H

libcxx/src/filesystem/posix_compat.h

union {
     } MountPointReparseBuffer;
     struct {
       unsigned char DataBuffer[1];
     } GenericReparseBuffer;
   };
 };
 #endif
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
-namespace {
 
 #if defined(_LIBCPP_WIN32API)
 // Various C runtime versions (UCRT, or the legacy msvcrt.dll used by
 // some mingw toolchains) provide different stat function implementations,
 // with a number of limitations with respect to what we want from the
 // stat function. Instead provide our own which does exactly what we want,
 // along with our own stat structure and flag macros.
 

 
 #define O_NONBLOCK 0
 
 
 // There were 369 years and 89 leap days from the Windows epoch
 // (1601) to the Unix epoch (1970).
 #define FILE_TIME_OFFSET_SECS (uint64_t(369 * 365 + 89) * (24 * 60 * 60))
 
-TimeSpec filetime_to_timespec(LARGE_INTEGER li) {
+inline TimeSpec filetime_to_timespec(LARGE_INTEGER li) {
   TimeSpec ret;
   ret.tv_sec = li.QuadPart / 10000000 - FILE_TIME_OFFSET_SECS;
   ret.tv_nsec = (li.QuadPart % 10000000) * 100;
   return ret;
 }
 
-TimeSpec filetime_to_timespec(FILETIME ft) {
+inline TimeSpec filetime_to_timespec(FILETIME ft) {
   LARGE_INTEGER li;
   li.LowPart = ft.dwLowDateTime;
   li.HighPart = ft.dwHighDateTime;
   return filetime_to_timespec(li);
 }
 
-FILETIME timespec_to_filetime(TimeSpec ts) {
+inline FILETIME timespec_to_filetime(TimeSpec ts) {
   LARGE_INTEGER li;
   li.QuadPart =
       ts.tv_nsec / 100 + (ts.tv_sec + FILE_TIME_OFFSET_SECS) * 10000000;
   FILETIME ft;
   ft.dwLowDateTime = li.LowPart;
   ft.dwHighDateTime = li.HighPart;
   return ft;
 }
 
-int set_errno(int e = GetLastError()) {
+inline int set_errno(int e = GetLastError()) {
   errno = static_cast<int>(__win_err_to_errc(e));
   return -1;
 }
 
 class WinHandle {
 public:
   WinHandle(const wchar_t *p, DWORD access, DWORD flags) {
     h = CreateFileW(
         p, access, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
         nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | flags, nullptr);
   }
   ~WinHandle() {
     if (h != INVALID_HANDLE_VALUE)
       CloseHandle(h);
   }
   operator HANDLE() const { return h; }
   operator bool() const { return h != INVALID_HANDLE_VALUE; }
 
 private:
   HANDLE h;
 };
 
-int stat_handle(HANDLE h, StatT *buf) {
+inline int stat_handle(HANDLE h, StatT *buf) {
   FILE_BASIC_INFO basic;
   if (!GetFileInformationByHandleEx(h, FileBasicInfo, &basic, sizeof(basic)))
     return set_errno();
   memset(buf, 0, sizeof(*buf));
   buf->st_mtim = filetime_to_timespec(basic.LastWriteTime);
   buf->st_atim = filetime_to_timespec(basic.LastAccessTime);
   buf->st_mode = 0555; // Read-only
   if (!(basic.FileAttributes & FILE_ATTRIBUTE_READONLY))
inline int stat_handle(HANDLE h, StatT *buf) {
   if (!GetFileInformationByHandle(h, &info))
     return set_errno();
   buf->st_dev = info.dwVolumeSerialNumber;
   memcpy(&buf->st_ino.id[0], &info.nFileIndexHigh, 4);
   memcpy(&buf->st_ino.id[4], &info.nFileIndexLow, 4);
   return 0;
 }
 
-int stat_file(const wchar_t *path, StatT *buf, DWORD flags) {
+inline int stat_file(const wchar_t *path, StatT *buf, DWORD flags) {
   WinHandle h(path, FILE_READ_ATTRIBUTES, flags);
   if (!h)
     return set_errno();
   int ret = stat_handle(h, buf);
   return ret;
 }
 
-int stat(const wchar_t *path, StatT *buf) { return stat_file(path, buf, 0); }
+inline int stat(const wchar_t *path, StatT *buf) { return stat_file(path, buf, 0); }
 
-int lstat(const wchar_t *path, StatT *buf) {
+inline int lstat(const wchar_t *path, StatT *buf) {
   return stat_file(path, buf, FILE_FLAG_OPEN_REPARSE_POINT);
 }
 
-int fstat(int fd, StatT *buf) {
+inline int fstat(int fd, StatT *buf) {
   HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   return stat_handle(h, buf);
 }
 
-int mkdir(const wchar_t *path, int permissions) {
+inline int mkdir(const wchar_t *path, int permissions) {
   (void)permissions;
   return _wmkdir(path);
 }
 
-int symlink_file_dir(const wchar_t *oldname, const wchar_t *newname,
+inline int symlink_file_dir(const wchar_t *oldname, const wchar_t *newname,
                             bool is_dir) {
   path dest(oldname);
   dest.make_preferred();
   oldname = dest.c_str();
   DWORD flags = is_dir ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0;
   if (CreateSymbolicLinkW(newname, oldname,
                           flags | SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE))
     return 0;
   int e = GetLastError();
   if (e != ERROR_INVALID_PARAMETER)
     return set_errno(e);
   if (CreateSymbolicLinkW(newname, oldname, flags))
     return 0;
   return set_errno();
 }
 
-int symlink_file(const wchar_t *oldname, const wchar_t *newname) {
+inline int symlink_file(const wchar_t *oldname, const wchar_t *newname) {
   return symlink_file_dir(oldname, newname, false);
 }
 
-int symlink_dir(const wchar_t *oldname, const wchar_t *newname) {
+inline int symlink_dir(const wchar_t *oldname, const wchar_t *newname) {
   return symlink_file_dir(oldname, newname, true);
 }
 
-int link(const wchar_t *oldname, const wchar_t *newname) {
+inline int link(const wchar_t *oldname, const wchar_t *newname) {
   if (CreateHardLinkW(newname, oldname, nullptr))
     return 0;
   return set_errno();
 }
 
-int remove(const wchar_t *path) {
+inline int remove(const wchar_t *path) {
   detail::WinHandle h(path, DELETE, FILE_FLAG_OPEN_REPARSE_POINT);
   if (!h)
     return set_errno();
   FILE_DISPOSITION_INFO info;
   info.DeleteFile = TRUE;
   if (!SetFileInformationByHandle(h, FileDispositionInfo, &info, sizeof(info)))
     return set_errno();
   return 0;
 }
 
-int truncate_handle(HANDLE h, off_t length) {
+inline int truncate_handle(HANDLE h, off_t length) {
   LARGE_INTEGER size_param;
   size_param.QuadPart = length;
   if (!SetFilePointerEx(h, size_param, 0, FILE_BEGIN))
     return set_errno();
   if (!SetEndOfFile(h))
     return set_errno();
   return 0;
 }
 
-int ftruncate(int fd, off_t length) {
+inline int ftruncate(int fd, off_t length) {
   HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   return truncate_handle(h, length);
 }
 
-int truncate(const wchar_t *path, off_t length) {
+inline int truncate(const wchar_t *path, off_t length) {
   detail::WinHandle h(path, GENERIC_WRITE, 0);
   if (!h)
     return set_errno();
   return truncate_handle(h, length);
 }
 
-int rename(const wchar_t *from, const wchar_t *to) {
+inline int rename(const wchar_t *from, const wchar_t *to) {
   if (!(MoveFileExW(from, to,
                     MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING |
                         MOVEFILE_WRITE_THROUGH)))
     return set_errno();
   return 0;
 }
 
 template <class... Args> int open(const wchar_t *filename, Args... args) {
   return _wopen(filename, args...);
 }
-int close(int fd) { return _close(fd); }
-int chdir(const wchar_t *path) { return _wchdir(path); }
+inline int close(int fd) { return _close(fd); }
+inline int chdir(const wchar_t *path) { return _wchdir(path); }
 
 struct StatVFS {
   uint64_t f_frsize;
   uint64_t f_blocks;
   uint64_t f_bfree;
   uint64_t f_bavail;
 };
 
-int statvfs(const wchar_t *p, StatVFS *buf) {
+inline int statvfs(const wchar_t *p, StatVFS *buf) {
   path dir = p;
   while (true) {
     error_code local_ec;
     const file_status st = status(dir, local_ec);
     if (!exists(st) || is_directory(st))
       break;
     path parent = dir.parent_path();
     if (parent == dir) {
if (!GetDiskFreeSpaceExW(dir.c_str(), &free_bytes_available_to_caller,
     return set_errno();
   buf->f_frsize = 1;
   buf->f_blocks = total_number_of_bytes.QuadPart;
   buf->f_bfree = total_number_of_free_bytes.QuadPart;
   buf->f_bavail = free_bytes_available_to_caller.QuadPart;
   return 0;
 }
 
-wchar_t *getcwd(wchar_t *buff, size_t size) { return _wgetcwd(buff, size); }
+inline wchar_t *getcwd(wchar_t *buff, size_t size) { return _wgetcwd(buff, size); }
 
-wchar_t *realpath(const wchar_t *path, wchar_t *resolved_name) {
+inline wchar_t *realpath(const wchar_t *path, wchar_t *resolved_name) {
   // Only expected to be used with us allocating the buffer.
   _LIBCPP_ASSERT(resolved_name == nullptr,
                  "Windows realpath() assumes a null resolved_name");
 
   WinHandle h(path, FILE_READ_ATTRIBUTES, 0);
   if (!h) {
     set_errno();
     return nullptr;
inline wchar_t *realpath(const wchar_t *path, wchar_t *resolved_name) {
   }
   return buff.release();
 }
 
 #define AT_FDCWD -1
 #define AT_SYMLINK_NOFOLLOW 1
 using ModeT = int;
 
-int fchmod_handle(HANDLE h, int perms) {
+inline int fchmod_handle(HANDLE h, int perms) {
   FILE_BASIC_INFO basic;
   if (!GetFileInformationByHandleEx(h, FileBasicInfo, &basic, sizeof(basic)))
     return set_errno();
   DWORD orig_attributes = basic.FileAttributes;
   basic.FileAttributes &= ~FILE_ATTRIBUTE_READONLY;
   if ((perms & 0222) == 0)
     basic.FileAttributes |= FILE_ATTRIBUTE_READONLY;
   if (basic.FileAttributes != orig_attributes &&
       !SetFileInformationByHandle(h, FileBasicInfo, &basic, sizeof(basic)))
     return set_errno();
   return 0;
 }
 
-int fchmodat(int /*fd*/, const wchar_t *path, int perms, int flag) {
+inline int fchmodat(int /*fd*/, const wchar_t *path, int perms, int flag) {
   DWORD attributes = GetFileAttributesW(path);
   if (attributes == INVALID_FILE_ATTRIBUTES)
     return set_errno();
   if (attributes & FILE_ATTRIBUTE_REPARSE_POINT &&
       !(flag & AT_SYMLINK_NOFOLLOW)) {
     // If the file is a symlink, and we are supposed to operate on the target
     // of the symlink, we need to open a handle to it, without the
     // FILE_FLAG_OPEN_REPARSE_POINT flag, to open the destination of the
if (attributes & FILE_ATTRIBUTE_REPARSE_POINT &&
     if ((perms & 0222) == 0)
       attributes |= FILE_ATTRIBUTE_READONLY;
     if (attributes != orig_attributes && !SetFileAttributesW(path, attributes))
       return set_errno();
   }
   return 0;
 }
 
-int fchmod(int fd, int perms) {
+inline int fchmod(int fd, int perms) {
   HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   return fchmod_handle(h, perms);
 }
 
 #define MAX_SYMLINK_SIZE MAXIMUM_REPARSE_DATA_BUFFER_SIZE
 using SSizeT = ::int64_t;
 
-SSizeT readlink(const wchar_t *path, wchar_t *ret_buf, size_t bufsize) {
+inline SSizeT readlink(const wchar_t *path, wchar_t *ret_buf, size_t bufsize) {
   uint8_t buf[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
   detail::WinHandle h(path, FILE_READ_ATTRIBUTES, FILE_FLAG_OPEN_REPARSE_POINT);
   if (!h)
     return set_errno();
   DWORD out;
   if (!DeviceIoControl(h, FSCTL_GET_REPARSE_POINT, nullptr, 0, buf, sizeof(buf),
                        &out, 0))
     return set_errno();
if (name_length / sizeof(wchar_t) > bufsize) {
     return -1;
   }
   memcpy(ret_buf, &symlink.PathBuffer[name_offset / sizeof(wchar_t)],
          name_length);
   return name_length / sizeof(wchar_t);
 }
 
 #else
-int symlink_file(const char *oldname, const char *newname) {
+inline int symlink_file(const char *oldname, const char *newname) {
   return ::symlink(oldname, newname);
 }
-int symlink_dir(const char *oldname, const char *newname) {
+inline int symlink_dir(const char *oldname, const char *newname) {
   return ::symlink(oldname, newname);
 }
 using ::chdir;
 using ::close;
 using ::fchmod;
 #if defined(AT_SYMLINK_NOFOLLOW) && defined(AT_FDCWD)
 using ::fchmodat;
 #endif

 #define O_BINARY 0
 
 using StatVFS = struct statvfs;
 using ModeT = ::mode_t;
 using SSizeT = ::ssize_t;
 
 #endif
 
-} // namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_DIAGNOSTIC_POP
-
 #endif // POSIX_COMPAT_H

libcxx/src/filesystem/time_utils.h

 # include <sys/time.h> // for ::utimes as used in __last_write_time
 #endif
 
 // We can use the presence of UTIME_OMIT to detect platforms that provide utimensat.
 #if defined(UTIME_OMIT)
 # define _LIBCPP_USE_UTIMENSAT
 #endif
 
-// TODO: Check whether these functions actually need internal linkage, or if they can be made normal header functions
-_LIBCPP_DIAGNOSTIC_PUSH
-_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
-_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-template")
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
-namespace {
 
 using chrono::duration;
 using chrono::duration_cast;
 
 template <class FileTimeT, class TimeT,
           bool IsFloat = is_floating_point<typename FileTimeT::rep>::value>
 struct time_util_base {
   using rep = typename FileTimeT::rep;
inline bool posix_utimes(const path& p, std::array<TimeSpec, 2> const& TS,
   if (::utimes(p.c_str(), ConvertedTS) == -1) {
     ec = capture_errno();
     return true;
   }
   return false;
 }
 
 #if defined(_LIBCPP_USE_UTIMENSAT)
-bool posix_utimensat(const path& p, std::array<TimeSpec, 2> const& TS,
+inline bool posix_utimensat(const path& p, std::array<TimeSpec, 2> const& TS,
                             error_code& ec) {
   if (::utimensat(AT_FDCWD, p.c_str(), TS.data(), 0) == -1) {
     ec = capture_errno();
     return true;
   }
   return false;
 }
 #endif
 
-bool set_file_times(const path& p, std::array<TimeSpec, 2> const& TS,
+inline 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
 
-file_time_type __extract_last_write_time(const path& p, const StatT& st,
+inline file_time_type __extract_last_write_time(const path& p, const StatT& st,
                                                 error_code* ec) {
   using detail::fs_time;
   ErrorHandler<file_time_type> err("last_write_time", ec, &p);
 
   auto ts = detail::extract_mtime(st);
   if (!fs_time::is_representable(ts))
     return err.report(errc::value_too_large);
 
   return fs_time::convert_from_timespec(ts);
 }
 
-} // namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
-_LIBCPP_DIAGNOSTIC_POP
-
 #endif // FILESYSTEM_TIME_UTILS_H