[libc++] Extract format_string from filesystem_common.h

[ldionne]

Summary

As a fly-by, rename format_string_impl to vformat_string and avoid
triggering ADL in a free function call.

[Mordante]

LGTM. I noticed this reporting code is quite C-ish. I think once C++ has transcoding we could change this code to use transcoding and std::format instead.

libcxx/src/filesystem/format_string.h

12

[ldionne]

Dropping since this has been merged into D152377.

libcxx/src/filesystem/filesystem_common.h

 #include <climits>
 #include <cstdarg>
 #include <ctime>
 #include <filesystem>
 #include <ratio>
 #include <utility>
 #include <system_error>
 
+#include "format_string.h"
 #include "posix_compat.h"
 
 #if defined(_LIBCPP_WIN32API)
 # define WIN32_LEAN_AND_MEAN
 # define NOMINMAX
 # include <windows.h>
 #else
 # 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 <unistd.h>
 #endif // defined(_LIBCPP_WIN32API)
 
 // We can use the presence of UTIME_OMIT to detect platforms that provide utimensat.
 #if defined(UTIME_OMIT)
 # define _LIBCPP_USE_UTIMENSAT
 #endif
 
-#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
-
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
 
 #if defined(_LIBCPP_WIN32API)
 // Non anonymous, to allow access from two translation units.
 _LIBCPP_HIDE_FROM_ABI errc __win_err_to_errc(int err);
 #endif
 
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 0) string
-format_string_impl(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;
-}
-
-inline _LIBCPP_HIDE_FROM_ABI _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 = format_string_impl(msg, ap);
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-  } catch (...) {
-    va_end(ap);
-    throw;
-  }
-#endif // _LIBCPP_HAS_NO_EXCEPTIONS
-  va_end(ap);
-  return ret;
-}
-
 inline _LIBCPP_HIDE_FROM_ABI error_code capture_errno() {
   _LIBCPP_ASSERT(errno != 0, "Expected errno to be non-zero");
   return error_code(errno, generic_category());
 }
 
 #if defined(_LIBCPP_WIN32API)
 inline _LIBCPP_HIDE_FROM_ABI error_code make_windows_error(int err) {
   return make_error_code(__win_err_to_errc(err));
struct ErrorHandler {
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 3, 0)
   void report_impl(const error_code& ec, const char* msg, va_list ap) const {
     if (ec_) {
       *ec_ = ec;
       return;
     }
     string what =
-        string("in ") + func_name_ + ": " + format_string_impl(msg, ap);
+        string("in ") + func_name_ + ": " + detail::vformat_string(msg, ap);
     switch (bool(p1_) + bool(p2_)) {
     case 0:
       __throw_filesystem_error(what, ec);
     case 1:
       __throw_filesystem_error(what, *p1_, ec);
     case 2:
       __throw_filesystem_error(what, *p1_, *p2_, ec);
     }

libcxx/src/filesystem/filesystem_error.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 <__utility/unreachable.h>
 #include <filesystem>
 #include <system_error>
 
-#include "filesystem_common.h"
+#include "format_string.h"
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 filesystem_error::~filesystem_error() {}
 
 void filesystem_error::__create_what(int __num_paths) {
   const char* derived_what = system_error::what();
   __storage_->__what_ = [&]() -> string {

libcxx/src/filesystem/format_string.h

+//===----------------------------------------------------------------------===////
+//
+// 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 FILESYSTEM_FORMAT_STRING_H
+#define FILESYSTEM_FORMAT_STRING_H
+
+#include <__assert>
+#include <array>
+#include <cstdarg>
+#include <cstddef>
+#include <cstdio>
+#include <string>
+
+#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
+
+_LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
+
+namespace detail {
+
+inline _LIBCPP_HIDE_FROM_ABI _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;
+}
+
+inline _LIBCPP_HIDE_FROM_ABI _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 namespace detail
+
+_LIBCPP_END_NAMESPACE_FILESYSTEM
+
+#endif // FILESYSTEM_FORMAT_STRING_H

libcxx/src/filesystem/path_parser.h

 //===----------------------------------------------------------------------===//
 //
 // 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 PATH_PARSER_H
 #define PATH_PARSER_H
 
 #include <__utility/unreachable.h>
-#include <climits>
-#include <cstdlib>
+#include <cstddef>
 #include <filesystem>
-#include <iterator>
-#include <string_view>
-#include <type_traits>
+#include <utility>
 
-#include "filesystem_common.h"
+#include "format_string.h"
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 inline _LIBCPP_HIDE_FROM_ABI
 bool isSeparator(path::value_type C) {
   if (C == '/')
     return true;
 #if defined(_LIBCPP_WIN32API)