[libcxx] proper guarding for locale usage in filesystem on Windows

[moleium]

• Resolves build issues when localization support is disabled on Windows.
• Resolves dependencies on localization in filesystem header implementations.

Related PR #164602
Fixes #164074

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-libcxx

Author: None (moleium)

Changes

• Resolves build issues when localization support is disabled on Windows.
• Resolves dependencies on localization in filesystem header implementations.

Related PR #164602
Fixes #164074

---

Full diff: https://github.com/llvm/llvm-project/pull/165470.diff

2 Files Affected:

• (modified) libcxx/include/__filesystem/path.h (+4-2)
• (modified) libcxx/include/__filesystem/u8path.h (+23-19)

diff --git a/libcxx/include/__filesystem/path.h b/libcxx/include/__filesystem/path.h index b3f324342f800..990833116aebe 100644 --- a/libcxx/include/__filesystem/path.h +++ b/libcxx/include/__filesystem/path.h @@ -324,6 +324,7 @@ struct _PathCVT<char> { } }; +# if _LIBCPP_HAS_LOCALIZATION template <class _ECharT> struct _PathExport { typedef __narrow_to_utf8<sizeof(wchar_t) * __CHAR_BIT__> _Narrower; @@ -364,7 +365,7 @@ struct _PathExport<char16_t> { } }; -# if _LIBCPP_HAS_CHAR8_T +# if _LIBCPP_HAS_CHAR8_T template <> struct _PathExport<char8_t> { typedef __narrow_to_utf8<sizeof(wchar_t) * __CHAR_BIT__> _Narrower; @@ -374,7 +375,8 @@ struct _PathExport<char8_t> { _Narrower()(back_inserter(__dest), __src.data(), __src.data() + __src.size()); } }; -# endif // _LIBCPP_HAS_CHAR8_T +# endif // _LIBCPP_HAS_CHAR8_T +# endif // _LIBCPP_HAS_LOCALIZATION # endif /* _LIBCPP_WIN32API */ class _LIBCPP_EXPORTED_FROM_ABI path { diff --git a/libcxx/include/__filesystem/u8path.h b/libcxx/include/__filesystem/u8path.h index 885372b0d1f86..7077b96ce48e6 100644 --- a/libcxx/include/__filesystem/u8path.h +++ b/libcxx/include/__filesystem/u8path.h @@ -13,7 +13,10 @@ #include <__algorithm/unwrap_iter.h> #include <__config> #include <__filesystem/path.h> + +#if _LIBCPP_HAS_LOCALIZATION #include <__locale> +#endif #include <string> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) @@ -24,28 +27,30 @@ _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM +# if !defined(_LIBCPP_WIN32API) || _LIBCPP_HAS_LOCALIZATION template <class _InputIt, __enable_if_t<__is_pathable<_InputIt>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, _InputIt __l) { static_assert( -# if _LIBCPP_HAS_CHAR8_T - is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || -# endif - is_same<typename __is_pathable<_InputIt>::__char_type, char>::value, - "u8path(Iter, Iter) requires Iter have a value_type of type 'char'" - " or 'char8_t'"); -# if defined(_LIBCPP_WIN32API) - string __tmp(__f, __l); - using _CVT = __widen_from_utf8<sizeof(wchar_t) * __CHAR_BIT__>; - std::wstring __w; - __w.reserve(__tmp.size()); - _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); - return path(__w); -# else - return path(__f, __l); -# endif /* !_LIBCPP_WIN32API */ +# if _LIBCPP_HAS_CHAR8_T + is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || +# endif + is_same<typename __is_pathable<_InputIt>::__char_type, char>::value, + "u8path(Iter, Iter) requires Iter have a value_type of type 'char'" + " or 'char8_t'"); +# if defined(_LIBCPP_WIN32API) + string __tmp(__f, __l); + using _CVT = __widen_from_utf8<sizeof(wchar_t) * __CHAR_BIT__>; + std::wstring __w; + __w.reserve(__tmp.size()); + _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); + return path(__w); +# else + return path(__f, __l); +# endif /* !_LIBCPP_WIN32API */ } +# endif /* !_LIBCPP_WIN32API || _LIBCPP_HAS_LOCALIZATION */ -# if defined(_LIBCPP_WIN32API) +# if defined(_LIBCPP_WIN32API) && _LIBCPP_HAS_LOCALIZATION template <class _InputIt, __enable_if_t<__is_pathable<_InputIt>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, _NullSentinel) { static_assert( @@ -65,7 +70,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); return path(__w); } -# endif /* _LIBCPP_WIN32API */ +# endif /* _LIBCPP_WIN32API && _LIBCPP_HAS_LOCALIZATION */ template <class _Source, __enable_if_t<__is_pathable<_Source>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(const _Source& __s) { @@ -85,7 +90,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(const _Source& } _LIBCPP_END_NAMESPACE_FILESYSTEM - #endif // _LIBCPP_STD_VER >= 17 #endif // _LIBCPP___FILESYSTEM_U8PATH_H 

[compnerd]

I wonder if this is better as a more complex change.

#if _LIBCPP_HAS_LOCALIZATION template <class _InputIt, __enable_if_t<__is_pathable<_InputIt>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, #if _LIBCPP_WIN32API _InputIt __l #else _NullSentinel #endif ) { static_assert( # if _LIBCPP_HAS_CHAR8_T is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || # endif is_same<typename __is_pathable<_InputIt>::__char_type, char>::value, "u8path(Iter, Iter) requires Iter have a value_type of type 'char'" " or 'char8_t'"); #if _LIBCPP_WIN32API string __tmp(__f, __l); #else string __tmp; #endif const char __sentinel = char{}; for (; *__f != __sentinel; ++__f) __tmp.push_back(*__f); using _CVT = __widen_from_utf8<sizeof(wchar_t) * __CHAR_BIT__>; std::wstring __w; __w.reserve(__tmp.size()); _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); return path(__w); } #endif

Of course this needs to be clang-formatted, but that is the general idea.

[moleium]

That would cause a regression by disabling u8path on non Windows platforms where it works correctly without localization. fix is targeted to disable the function only on the broken configuration (Windows without localization), preserving functionality elsewhere

[compnerd]

Okay, but the general idea still holds; we can guard it with #if !_LIBCPP_WIN32 || _LIBCPP_HAS_LOCALIZATION instead, and adjust the body as well.

#if !_LIBCPP_WIN32 || _LIBCPP_HAS_LOCALIZATION template <class _InputIt, __enable_if_t<__is_pathable<_InputIt>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, #if _LIBCPP_HAS_LOCALIZATION _InputIt __l #else _NullSentinel #endif ) { static_assert( # if _LIBCPP_HAS_CHAR8_T is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || # endif is_same<typename __is_pathable<_InputIt>::__char_type, char>::value, "u8path(Iter, Iter) requires Iter have a value_type of type 'char'" " or 'char8_t'"); #if !_LIBCPP_HAS_LOCALIZATION return path(__f, __l); #else string __tmp(__f, __l); using _CVT = __widen_from_utf8<sizeof(wchar_t) * __CHAR_BIT__>; std::wstring __w; __w.reserve(__tmp.size()); _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); return path(__w); #endif } #endif

Sorry, doing this during the LLVM conference, so there might be more tweaks required.

[frederick-vs-ja]

I strongly think that it's unconforming to use the _NullSentinel parameter in the function of a standard name (u8path).

I think it's better to reform the code like this:

Collapse/Expand

_LIBCPP_BEGIN_NAMESPACE_FILESYSTEM # if defined(_LIBCPP_WIN32API) && _LIBCPP_HAS_LOCALIZATION # define _LIBCPP_FS_U8PATH_CONVERTS_ENCODING 1 # else # define _LIBCPP_FS_U8PATH_CONVERTS_ENCODING 0 # endif # if _LIBCPP_FS_U8PATH_CONVERTS_ENCODING template <class _InputIt, class _Sentinel> _LIBCPP_HIDE_FROM_ABI string __make_tmp_string_for_u8path(_InputIt __f, _Sentinel __l) { static_assert(__is_pathable<_InputIt>::value); static_assert( # if _LIBCPP_HAS_CHAR8_T is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || # endif is_same<typename __is_pathable<_InputIt>::__char_type, char>::value); if constexpr (is_same_v<_InputIt, _NullSentinel>) { string __tmp; constexpr char __sentinel{}; for (; *__f != __sentinel; ++__f) __tmp.push_back(*__f); return __tmp; } else { static_assert(is_same_v<_InputIt, _Sentinel>); return string(__f, __l); } } # endif // _LIBCPP_FS_U8PATH_CONVERTS_ENCODING template <class _InputIt, class _Sentinel> _LIBCPP_HIDE_FROM_ABI path __u8path(_InputIt __f, _Sentinel __l) { # if _LIBCPP_FS_U8PATH_CONVERTS_ENCODING auto __tmp = std::filesystem::__make_tmp_string_for_u8path(__f, __l); using _CVT = __widen_from_utf8<sizeof(wchar_t) * __CHAR_BIT__>; std::wstring __w; __w.reserve(__tmp.size()); _CVT()(back_inserter(__w), __tmp.data(), __tmp.data() + __tmp.size()); return path(__w); # else return path(__f, __l); # endif // _LIBCPP_FS_U8PATH_CONVERTS_ENCODING } template <class _InputIt, __enable_if_t<__is_pathable<_InputIt>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(_InputIt __f, _InputIt __l) { static_assert( # if _LIBCPP_HAS_CHAR8_T is_same<typename __is_pathable<_InputIt>::__char_type, char8_t>::value || # endif is_same<typename __is_pathable<_InputIt>::__char_type, char>::value, "u8path(Iter, Iter) requires Iter have a value_type of type 'char'" " or 'char8_t'"); return std::filesystem::__u8path(__f, __l); } template <class _Source, __enable_if_t<__is_pathable<_Source>::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_WITH_CHAR8_T path u8path(const _Source& __s) { static_assert( # if _LIBCPP_HAS_CHAR8_T is_same<typename __is_pathable<_Source>::__char_type, char8_t>::value || # endif is_same<typename __is_pathable<_Source>::__char_type, char>::value, "u8path(Source const&) requires Source have a character type of type " "'char' or 'char8_t'"); # if _LIBCPP_FS_U8PATH_CONVERTS_ENCODING using _Traits = __is_pathable<_Source>; return std::filesystem::__u8path( std::__unwrap_iter(_Traits::__range_begin(__s)), std::__unwrap_iter(_Traits::__range_end(__s))); # else return path(__s); # endif // _LIBCPP_FS_U8PATH_CONVERTS_ENCODING } # undef _LIBCPP_FS_U8PATH_CONVERTS_ENCODING _LIBCPP_END_NAMESPACE_FILESYSTEM