[libc] Remove btowc / wctob from wctype_utils internal header.

[vonosmas]

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 with wchar_t, since it's a fundamental type in C++.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

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 with wchar_t, since it's a fundamental type in C++.

---

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

6 Files Affected:

• (modified) libc/src/__support/CMakeLists.txt (-1)
• (modified) libc/src/__support/wctype_utils.h (-26)
• (modified) libc/src/wchar/CMakeLists.txt (-2)
• (modified) libc/src/wchar/btowc.cpp (+2-6)
• (modified) libc/src/wchar/wctob.cpp (+6-6)
• (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (-4)

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",
     ],

[ldionne]

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 wint_t, which would break libc++ again. So I think we should still discuss the general topic of "carve-out coordination" between LLVM libc and libc++. I'll add that to the libc++ monthly tomorrow, in case you folks can make it.

[vonosmas]

Great, I'll merge after receiving approval from anyone.