[libc] Remove btowc / wctob from wctype_utils internal header. #170200
+8 −45
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
They are not used anywhere except for the btowc/wctob entrypoints, so just move the implementation there. Internal code should probably be using a safer mbrtowc variants anyway, if applicable. This allows us to remove the use of wint_t, which is problematic for some uses through `libc/shared/` when a host system doesn't have wide-character support (see PR llvm#165884 comments). There's no such problems with `wchar_t`, since it's a fundamental type in C++.
vonosmas requested review from aaronmondal, keith and rupprecht as code owners 
Member
| @llvm/pr-subscribers-libc Author: Alexey Samsonov (vonosmas) ChangesThey are not used anywhere except for the btowc/wctob entrypoints, so just move the implementation there. Internal code should probably be using a safer mbrtowc variants anyway, if applicable. This allows us to remove the use of wint_t, which is problematic for some uses through Full diff: https://github.com/llvm/llvm-project/pull/170200.diff 6 Files Affected:
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt index d33e7ae45c068..c7f127d6934a0 100644 --- a/libc/src/__support/CMakeLists.txt +++ b/libc/src/__support/CMakeLists.txt @@ -162,7 +162,6 @@ add_header_library( wctype_utils.h DEPENDS libc.hdr.types.wchar_t - libc.hdr.types.wint_t ) add_header_library( diff --git a/libc/src/__support/wctype_utils.h b/libc/src/__support/wctype_utils.h index 7041470adc2f4..7f17224104ffb 100644 --- a/libc/src/__support/wctype_utils.h +++ b/libc/src/__support/wctype_utils.h @@ -10,8 +10,6 @@ #define LLVM_LIBC_SRC___SUPPORT_WCTYPE_UTILS_H #include "hdr/types/wchar_t.h" -#include "hdr/types/wint_t.h" -#include "src/__support/CPP/optional.h" #include "src/__support/macros/attributes.h" // LIBC_INLINE #include "src/__support/macros/config.h" @@ -584,30 +582,6 @@ is_char_or_wchar(wchar_t ch, [[maybe_unused]] char, wchar_t wc_value) { return (ch == wc_value); } -// ------------------------------------------------------ -// Rationale: Since these classification functions are -// called in other functions, we will avoid the overhead -// of a function call by inlining them. -// ------------------------------------------------------ - -LIBC_INLINE cpp::optional<int> wctob(wint_t c) { - // This needs to be translated to EOF at the callsite. This is to avoid - // including stdio.h in this file. - // The standard states that wint_t may either be an alias of wchar_t or - // an alias of an integer type, different platforms define this type with - // different signedness. This is equivalent to `(c > 127) || (c < 0)` but also - // works without -Wtype-limits warnings when `wint_t` is unsigned. - if ((c & ~127) != 0) - return cpp::nullopt; - return static_cast<int>(c); -} - -LIBC_INLINE cpp::optional<wint_t> btowc(int c) { - if (c > 127 || c < 0) - return cpp::nullopt; - return static_cast<wint_t>(c); -} - } // namespace internal } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/wchar/CMakeLists.txt b/libc/src/wchar/CMakeLists.txt index 9ca7295118a11..ce57199b0837a 100644 --- a/libc/src/wchar/CMakeLists.txt +++ b/libc/src/wchar/CMakeLists.txt @@ -40,7 +40,6 @@ add_entrypoint_object( DEPENDS libc.hdr.types.wint_t libc.hdr.stdio_macros - libc.src.__support.wctype_utils ) add_entrypoint_object( @@ -52,7 +51,6 @@ add_entrypoint_object( DEPENDS libc.hdr.types.wint_t libc.hdr.wchar_macros - libc.src.__support.wctype_utils ) add_entrypoint_object( diff --git a/libc/src/wchar/btowc.cpp b/libc/src/wchar/btowc.cpp index c69f77d06c5c5..6bc7526fac164 100644 --- a/libc/src/wchar/btowc.cpp +++ b/libc/src/wchar/btowc.cpp @@ -9,7 +9,6 @@ #include "src/wchar/btowc.h" #include "src/__support/common.h" #include "src/__support/macros/config.h" -#include "src/__support/wctype_utils.h" #include "hdr/types/wint_t.h" #include "hdr/wchar_macros.h" // for WEOF. @@ -17,12 +16,9 @@ namespace LIBC_NAMESPACE_DECL { LLVM_LIBC_FUNCTION(wint_t, btowc, (int c)) { - auto result = internal::btowc(c); - if (result.has_value()) { - return result.value(); - } else { + if (c > 127 || c < 0) return WEOF; - } + return static_cast<wint_t>(c); } } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/wchar/wctob.cpp b/libc/src/wchar/wctob.cpp index 45240d6052eb4..9b6773bf6428e 100644 --- a/libc/src/wchar/wctob.cpp +++ b/libc/src/wchar/wctob.cpp @@ -9,7 +9,6 @@ #include "src/wchar/wctob.h" #include "src/__support/common.h" #include "src/__support/macros/config.h" -#include "src/__support/wctype_utils.h" #include "hdr/stdio_macros.h" // for EOF. #include "hdr/types/wint_t.h" @@ -17,12 +16,13 @@ namespace LIBC_NAMESPACE_DECL { LLVM_LIBC_FUNCTION(int, wctob, (wint_t c)) { - auto result = internal::wctob(c); - if (result.has_value()) { - return result.value(); - } else { + // The standard states that wint_t may either be an alias of wchar_t or + // an alias of an integer type, different platforms define this type with + // different signedness. This is equivalent to `(c > 127) || (c < 0)` but also + // works without -Wtype-limits warnings when `wint_t` is unsigned. + if ((c & ~127) != 0) return EOF; - } + return static_cast<int>(c); } } // namespace LIBC_NAMESPACE_DECL diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel index e3962d9b5f95b..e063de015506b 100644 --- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel +++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel @@ -1799,11 +1799,9 @@ libc_support_library( name = "__support_wctype_utils", hdrs = ["src/__support/wctype_utils.h"], deps = [ - ":__support_cpp_optional", ":__support_macros_attributes", ":__support_macros_config", ":types_wchar_t", - ":types_wint_t", ], ) @@ -7472,7 +7470,6 @@ libc_function( deps = [ ":__support_common", ":__support_macros_config", - ":__support_wctype_utils", ":hdr_wchar_macros", ":types_wint_t", ], @@ -7704,7 +7701,6 @@ libc_function( ":__support_common", ":__support_macros_config", ":__support_macros_null_check", - ":__support_wctype_utils", ":hdr_stdio_macros", ":types_wint_t", ], |
Member
| I pulled down this patch and it does appear to resolve the issue I was seeing. So clearly I'm fine with this patch, thank you! However, any other patch would come around and legitimately add new mentions of |
Contributor Author
| Great, I'll merge after receiving approval from anyone. |
lntue approved these changes Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
They are not used anywhere except for the btowc/wctob entrypoints, so just move the implementation there. Internal code should probably be using a safer mbrtowc variants anyway, if applicable.
This allows us to remove the use of wint_t, which is problematic for some uses through
libc/shared/when a host system doesn't have wide-character support (see PR #165884 comments). There's no such problems withwchar_t, since it's a fundamental type in C++.