On Thu, Feb 12, 2026 at 11:07 AM Tomasz Kaminski <[email protected]> wrote:
> > > On Wed, Feb 11, 2026 at 2:59 PM Ivan Lazaric <[email protected]> > wrote: > >> This patch implements formatting for std::filesystem::path from P2845R8, >> and defines the feature test macro __cpp_lib_format_path to 202403L, >> provided only in <filesystem>. >> >> libstdc++-v3/ChangeLog: >> * include/bits/fs_path.h: Include bits/formatfwd.h. >> (__format::__formatter_fs_path<_CharT>) >> (formatter<filesystem::path, _CharT>): Define. >> * include/bits/version.def: Add format_path. >> * include/bits/version.h: Regenerate. >> * include/std/filesystem: Expose __cpp_lib_format_path. >> * testsuite/std/format/fs_path.cc: New test. >> >> Signed-off-by: Ivan Lazaric <[email protected]> >> Co-authored-by: Jonathan Wakely <[email protected]> >> > Thank you very much for the patch, the implementation looks good. > For the implementation, my only suggestion is to remove __formatter_fs_path > entirely, and put code directly in formatter specialization. > > I have made more suggestion on making the test generic. > > Please let me know if you preffer us to do further patch updates. > >> --- >> libstdc++-v3/include/bits/fs_path.h | 136 +++++++++++++++++++ >> libstdc++-v3/include/bits/version.def | 10 ++ >> libstdc++-v3/include/bits/version.h | 10 ++ >> libstdc++-v3/include/std/filesystem | 1 + >> libstdc++-v3/testsuite/std/format/fs_path.cc | 88 ++++++++++++ >> 5 files changed, 245 insertions(+) >> create mode 100644 libstdc++-v3/testsuite/std/format/fs_path.cc >> >> diff --git a/libstdc++-v3/include/bits/fs_path.h >> b/libstdc++-v3/include/bits/fs_path.h >> index 07b74de6cbe..1ca4942235e 100644 >> --- a/libstdc++-v3/include/bits/fs_path.h >> +++ b/libstdc++-v3/include/bits/fs_path.h >> @@ -50,6 +50,10 @@ >> # include <compare> >> #endif >> >> +#ifdef __cpp_lib_format_path // C++ >= 26 && HOSTED >> +# include <bits/formatfwd.h> >> +#endif >> + >> #if defined(_WIN32) && !defined(__CYGWIN__) >> # define _GLIBCXX_FILESYSTEM_IS_WINDOWS 1 >> #endif >> @@ -1451,6 +1455,138 @@ template<> >> { return filesystem::hash_value(__p); } >> }; >> >> +#ifdef __cpp_lib_format_path // C++ >= 26 && HOSTED >> +namespace __format >> +{ >> + template<__char _CharT> >> + struct __formatter_fs_path >> > I do not think we need to have a separate formatter class, > and you can define parse/format directly in the formatte specialization. > We define this __formatter_fs_path if we want to reuse it in implementing > other formatters. > >> + { >> + __formatter_fs_path() = default; >> + >> + constexpr >> + __formatter_fs_path(_Spec<_CharT> __spec) noexcept >> + : _M_spec(__spec) >> + { } >> + >> + constexpr typename basic_format_parse_context<_CharT>::iterator >> + parse(basic_format_parse_context<_CharT>& __pc) >> + { >> + auto __first = __pc.begin(); >> + const auto __last = __pc.end(); >> + _Spec<_CharT> __spec{}; >> + >> + auto __finalize = [this, &__spec] { >> + _M_spec = __spec; >> + }; >> + >> + auto __finished = [&] { >> + if (__first == __last || *__first == '}') >> + { >> + __finalize(); >> + return true; >> + } >> + return false; >> + }; >> + >> + if (__finished()) >> + return __first; >> + >> + __first = __spec._M_parse_fill_and_align(__first, __last); >> + if (__finished()) >> + return __first; >> + >> + __first = __spec._M_parse_width(__first, __last, __pc); >> + if (__finished()) >> + return __first; >> + >> + if (*__first == '?') >> + { >> + __spec._M_debug = true; >> + ++__first; >> + } >> + if (__finished()) >> + return __first; >> + >> + if (*__first == 'g') >> + { >> + __spec._M_type = _Pres_g; >> + ++__first; >> + } >> + if (__finished()) >> + return __first; >> + >> + __format::__failed_to_parse_format_spec(); >> + } >> + >> + template<typename _Out> >> + _Out >> + format(const filesystem::path& __p, >> + basic_format_context<_Out, _CharT>& __fc) const >> + { >> + using _ValueT = filesystem::path::value_type; >> + using _FmtStrT = __formatter_str<_CharT>; >> + >> + filesystem::path::string_type __s; > > + if (_M_spec._M_type == _Pres_g) >> + __s = __p.generic_string<_ValueT>(); >> + else >> + __s = __p.native(); >> > Native returns a const string_type& so we make unnecessary copy here, I would suggest replacing that with: filesystem::path::string_type __s; basic_string_view<_ValueT> __sv; if (_M_spec._M_type == _Pres_g) __sv = __s = __p.generic_string<_ValueT>(); else __sv = __p.native(); And then using __sv. > > + >> + auto __spec = _M_spec; >> + // 'g' should not be passed along. >> + __spec._M_type = _Pres_none; >> + >> + if constexpr (is_same_v<_CharT, _ValueT>) >> + return _FmtStrT(__spec).format(__s, __fc); >> > + else >> > The standard separates two cases here, i.e. literal_encondings > being UTF, where it requires exactly what you implemented, and > makes it implementation defined for all other cases. > I have checked the transcoding of native inside string<_CharT>, > and the implementation already assumes that char encoding is UTF8. > The only special scenario is transcoding from char to wchar. > > Given that wchar_t is either UCS-2 or UTF-32, I think we are OK. > Could you summarize above in the commit message? > >> + { > > + if (__spec._M_debug) >> + { >> + _Str_sink<_ValueT> __sink; >> + basic_string_view<_ValueT> __sv(__s); >> + __format::__write_escaped(__sink.out(), __sv, _Term_quote); >> + __s = std::move(__sink).get(); >> > String sink has a local 256 characters buffer, which is much larger than SSO. If we move _Str_sink declaration before __spec._M_debug check, we can make __sv reffer to the internal buffer directly. _Str_sink<_ValueT> __sink; if (__spec._M_debug) { __format::__write_escaped(__sink.out(), __sv, _Term_quote); __sv = __sink.view(); } > + __spec._M_debug = 0; >> > + } >> + basic_string<_CharT> __out_str; >> + using _View = basic_string_view<_ValueT>; >> + __out_str.assign_range(__unicode::_Utf_view<_CharT, _View>(__s)); >> + return _FmtStrT(__spec).format(__out_str, __fc); >> + } >> + } >> + >> + constexpr void >> + set_debug_format() noexcept >> + { _M_spec._M_debug = true; } >> + >> + private: >> + _Spec<_CharT> _M_spec{}; >> + }; >> +} // namespace __format >> + >> + template<__format::__char _CharT> >> + struct formatter<filesystem::path, _CharT> >> + { >> + formatter() = default; >> + >> + [[__gnu__::__always_inline__]] >> + constexpr typename basic_format_parse_context<_CharT>::iterator >> + parse(basic_format_parse_context<_CharT>& __pc) >> + { return _M_f.parse(__pc); } >> + >> + template<typename _Out> >> + typename basic_format_context<_Out, _CharT>::iterator >> + format(const filesystem::path& __p, >> + basic_format_context<_Out, _CharT>& __fc) const >> + { return _M_f.format(__p, __fc); } >> + >> + constexpr void set_debug_format() noexcept { >> _M_f.set_debug_format(); } >> + >> + private: >> + __format::__formatter_fs_path<_CharT> _M_f; >> + }; >> +#endif // __cpp_lib_format_path >> + >> _GLIBCXX_END_NAMESPACE_VERSION >> } // namespace std >> >> diff --git a/libstdc++-v3/include/bits/version.def >> b/libstdc++-v3/include/bits/version.def >> index 4b8e9d43ec2..c7709ba3a07 100644 >> --- a/libstdc++-v3/include/bits/version.def >> +++ b/libstdc++-v3/include/bits/version.def >> @@ -1561,6 +1561,16 @@ ftms = { >> }; >> }; >> >> +ftms = { >> + name = format_path; >> + // 202403 P2845R8 Formatting of std::filesystem::path >> + values = { >> + v = 202403; >> + cxxmin = 26; >> + hosted = yes; >> + }; >> +}; >> + >> ftms = { >> name = freestanding_algorithm; >> values = { >> diff --git a/libstdc++-v3/include/bits/version.h >> b/libstdc++-v3/include/bits/version.h >> index 7602225cb6d..c72cda506f1 100644 >> --- a/libstdc++-v3/include/bits/version.h >> +++ b/libstdc++-v3/include/bits/version.h >> @@ -1721,6 +1721,16 @@ >> #endif /* !defined(__cpp_lib_format_ranges) */ >> #undef __glibcxx_want_format_ranges >> >> +#if !defined(__cpp_lib_format_path) >> +# if (__cplusplus > 202302L) && _GLIBCXX_HOSTED >> +# define __glibcxx_format_path 202403L >> +# if defined(__glibcxx_want_all) || defined(__glibcxx_want_format_path) >> +# define __cpp_lib_format_path 202403L >> +# endif >> +# endif >> +#endif /* !defined(__cpp_lib_format_path) */ >> +#undef __glibcxx_want_format_path >> + >> #if !defined(__cpp_lib_freestanding_algorithm) >> # if (__cplusplus >= 202100L) >> # define __glibcxx_freestanding_algorithm 202311L >> diff --git a/libstdc++-v3/include/std/filesystem >> b/libstdc++-v3/include/std/filesystem >> index f902c6feb77..b9900f49c33 100644 >> --- a/libstdc++-v3/include/std/filesystem >> +++ b/libstdc++-v3/include/std/filesystem >> @@ -37,6 +37,7 @@ >> #include <bits/requires_hosted.h> >> >> #define __glibcxx_want_filesystem >> +#define __glibcxx_want_format_path >> #include <bits/version.h> >> >> #ifdef __cpp_lib_filesystem // C++ >= 17 && HOSTED >> diff --git a/libstdc++-v3/testsuite/std/format/fs_path.cc >> b/libstdc++-v3/testsuite/std/format/fs_path.cc >> new file mode 100644 >> index 00000000000..5105e73f611 >> --- /dev/null >> +++ b/libstdc++-v3/testsuite/std/format/fs_path.cc >> @@ -0,0 +1,88 @@ >> +// { dg-do run { target c++26 } } >> > As discussed the patch conversion assume currently that the char > is encoded as UTF-8, so to avoid the test failing on config with different > literal encoding, we should add: > // { dg-options "-fexec-charset=UTF-8" } > >> + >> +#include <filesystem> >> +#include <format> >> +#include <testsuite_hooks.h> >> + >> +using std::filesystem::path; >> + >> +template<typename... Args> >> +bool >> +is_format_string_for(const char* str, Args&&... args) >> +{ >> + try { >> + (void) std::vformat(str, std::make_format_args(args...)); >> + return true; >> + } catch (const std::format_error&) { >> + return false; >> + } >> +} >> + >> +template<typename... Args> >> +bool >> +is_format_string_for(const wchar_t* str, Args&&... args) >> +{ >> + try { >> + (void) std::vformat(str, std::make_wformat_args(args...)); >> + return true; >> + } catch (const std::format_error&) { >> + return false; >> + } >> +} >> + >> +void >> +test_format_spec() >> +{ >> + // [fs.path.fmtr.funcs] >> + // \nontermdef{path-format-spec}\br >> + // \opt{fill-and-align} \opt{width} \opt{\terminal{?}} >> \opt{\terminal{g}} >> + path p; >> + VERIFY( is_format_string_for("{}", p) ); >> + VERIFY( is_format_string_for("{:}", p) ); >> + VERIFY( is_format_string_for("{:?}", p) ); >> + VERIFY( is_format_string_for("{:g}", p) ); >> + VERIFY( is_format_string_for("{:?g}", p) ); >> + VERIFY( is_format_string_for("{:?g}", p) ); >> + VERIFY( is_format_string_for("{:F^32?g}", p) ); >> + VERIFY( is_format_string_for("{:G<{}?g}", p, 32) ); >> + VERIFY( is_format_string_for(L"{:G<{}?g}", p, 32) ); >> + >> + VERIFY( ! is_format_string_for("{:g?}", p) ); >> +} >> + >> +void >> +test_format_path() >> > Could you take a look on libstdc++-v3/testsuite/std/format/debug.cc, > and make this test template on _CharT. We could also have two > macros to patch and add string. Something like: > #define WIDEN_(C, S) ::std::__format::_Widen<C>(S, L##S) > #define WFMT(S) WIDEN(_FCharT, S) > #define WPATH(S) WIDEN(_PCharT, S) > > template<typename _FCharT, typename _PCharT> > test_format() > { > basic_string<_FCharT> res; > res = std::format(WFMT("{}"), path(WPATH("/usr/include"))); > VERIFY ( res == WFMT("/usr/include") ); > // I like to assign the result of fmt to a local variable, and then calling > // verify, because it makes it much easier to debug failures. > > And then call test_format<char, char>, test_format<char, wchar_t>, > test_format<wchar_t, char>, test_format<wchar_t, wchar_t> from the main. > > +{ >> + VERIFY( std::format("{}", path("/usr/include")) == "/usr/include" ); > > + VERIFY( std::format("{:.<10}", path("foo/bar")) == "foo/bar..." ); >> + VERIFY( std::format("{}", path("foo///bar")) == "foo///bar" ); >> + VERIFY( std::format("{:g}", path("foo///bar")) == "foo/bar" ); >> + VERIFY( std::format("{}", path("/path/with/new\nline")) == >> "/path/with/new\nline" ); >> + VERIFY( std::format("{:?}", path("multi\nline")) == "\"multi\\nline\"" >> ); >> + VERIFY( std::format("{:?g}", path("mu///lti\nli///ne")) == >> "\"mu/lti\\nli/ne\"" ); >> + VERIFY( std::format(L"{}", >> >> path(L"\u0428\u0447\u0443\u0447\u044B\u043D\u0448\u0447\u044B\u043D\u0430")) >> == L"\u0428\u0447\u0443\u0447\u044B\u043D\u0448\u0447\u044B\u043D\u0430" >> ); >> + >> + if constexpr (path::preferred_separator == L'\\') > > + { >> + VERIFY( std::format("{}", path("C:\\foo\\bar")) == "C:\\foo\\bar" ); >> + VERIFY( std::format("{:g}", path("C:\\foo\\bar")) == "C:/foo/bar" ); >> + >> + path p(L"\xd800"); >> + VERIFY( std::format("{}", p) == "\uFFFD" ); >> + VERIFY( std::format("{:?}", p) == "\"\\x{d800}\"" ); >> + VERIFY( std::format(L"{:?}", p) == L"\"\\x{d800}\"" ); >> > I think we can also check that on linux. Just use WPATCH("\uFFFD") to > initialize patch. >> >> + >> + path p2(L"///\xd800"); >> + VERIFY( std::format("{}", p2) == "///\uFFFD" ); >> + VERIFY( std::format("{:g}", p2) == "/\uFFFD" ); >> + VERIFY( std::format("{:?}", p2) == "\"///\\x{d800}\"" ); >> + VERIFY( std::format("{:?g}", p2) == "\"/\\x{d800}\"" ); >> + VERIFY( std::format("{:C>14?g}", p2) == "CCC\"/\\x{d800}\"" ); >> + } >> +} >> + >> +int main() >> +{ >> + test_format_spec(); >> + test_format_path(); >> +} >> -- >> 2.43.0 >> >>
