Skip to content

Conversation

@moleium
Copy link

@moleium moleium commented Oct 28, 2025

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

Related PR #164602
Fixes #164074

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

Related PR llvm#164602
Fixes llvm#164074
@moleium moleium requested a review from a team as a code owner October 28, 2025 20:06
@github-actions
Copy link

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 llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 28, 2025

@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


_LIBCPP_BEGIN_NAMESPACE_FILESYSTEM

# if !defined(_LIBCPP_WIN32API) || _LIBCPP_HAS_LOCALIZATION
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frederick-vs-ja Thank you for the suggestion. This is much cleaner. I'll update the PR with your proposed implementation.

@github-actions
Copy link

github-actions bot commented Oct 29, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions h -- libcxx/include/__filesystem/path.h libcxx/include/__filesystem/u8path.h --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/libcxx/include/__filesystem/path.h b/libcxx/include/__filesystem/path.h
index 990833116..fc27414ea 100644
--- a/libcxx/include/__filesystem/path.h
+++ b/libcxx/include/__filesystem/path.h
@@ -376,7 +376,7 @@ struct _PathExport<char8_t> {
   }
 };
 #      endif // _LIBCPP_HAS_CHAR8_T
-#    endif // _LIBCPP_HAS_LOCALIZATION
+#    endif   // _LIBCPP_HAS_LOCALIZATION
 #  endif   /* _LIBCPP_WIN32API */
 
 class _LIBCPP_EXPORTED_FROM_ABI path {

Co-authored-by: Hxnter999 <Hxnter999@users.noreply.github.com>
@moleium moleium force-pushed the libcxx-freestanding-fs-guards branch from de27e34 to 9d49d76 Compare October 29, 2025 16:02
@ldionne ldionne changed the title [libcxx] proper guarding for locale usage in filesystem on Windows [libc++] proper guarding for locale usage in filesystem on Windows Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. platform:windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[libcxx][CMake] LIBCXX_ENABLE_FILESYSTEM=OFF is not respected on Windows for freestanding runtimes build

4 participants