[libc-commits] [libc] [libc] Reworked CharacterConverter isComplete into isFull and isEmpty (PR #144799)

Uzair Nawaz via libc-commits libc-commits at lists.llvm.org
Wed Jun 18 14:45:52 PDT 2025


https://github.com/uzairnawaz updated https://github.com/llvm/llvm-project/pull/144799

>From b3304ed83ec4c7de0d583c2c067af40af08256e7 Mon Sep 17 00:00:00 2001
From: Uzair Nawaz <uzairnawaz at google.com>
Date: Wed, 18 Jun 2025 21:37:31 +0000
Subject: [PATCH 1/2] Reworked isComplete into isFull and isEmpty that are used
 consistently for both conversions

---
 .../__support/wchar/character_converter.cpp   | 33 +++++++------
 .../src/__support/wchar/character_converter.h |  3 +-
 libc/src/__support/wchar/mbstate.h            |  6 +--
 .../src/__support/wchar/utf32_to_8_test.cpp   | 48 +++++++++++--------
 .../src/__support/wchar/utf8_to_32_test.cpp   | 20 ++++----
 5 files changed, 61 insertions(+), 49 deletions(-)

diff --git a/libc/src/__support/wchar/character_converter.cpp b/libc/src/__support/wchar/character_converter.cpp
index 5ab0447bb08b2..708af357d15d9 100644
--- a/libc/src/__support/wchar/character_converter.cpp
+++ b/libc/src/__support/wchar/character_converter.cpp
@@ -30,18 +30,21 @@ CharacterConverter::CharacterConverter(mbstate *mbstate) { state = mbstate; }
 
 void CharacterConverter::clear() {
   state->partial = 0;
-  state->bytes_processed = 0;
+  state->bytes_stored = 0;
   state->total_bytes = 0;
 }
 
-bool CharacterConverter::isComplete() {
-  return state->bytes_processed == state->total_bytes;
+bool CharacterConverter::isFull() {
+  return state->bytes_stored == state->total_bytes &&
+         state->total_bytes != 0;
 }
 
+bool CharacterConverter::isEmpty() { return state->bytes_stored == 0; }
+
 int CharacterConverter::push(char8_t utf8_byte) {
   uint8_t num_ones = static_cast<uint8_t>(cpp::countl_one(utf8_byte));
   // Checking the first byte if first push
-  if (state->bytes_processed == 0) {
+  if (isEmpty()) {
     // UTF-8 char has 1 byte total
     if (num_ones == 0) {
       state->total_bytes = 1;
@@ -58,21 +61,21 @@ int CharacterConverter::push(char8_t utf8_byte) {
     }
     // Invalid first byte
     else {
-      // bytes_processed and total_bytes will always be 0 here
+      // bytes_stored and total_bytes will always be 0 here
       state->partial = static_cast<char32_t>(0);
       return -1;
     }
     state->partial = static_cast<char32_t>(utf8_byte);
-    state->bytes_processed++;
+    state->bytes_stored++;
     return 0;
   }
   // Any subsequent push
   // Adding 6 more bits so need to left shift
-  if (num_ones == 1 && !isComplete()) {
+  if (num_ones == 1 && !isFull()) {
     char32_t byte = utf8_byte & MASK_ENCODED_BITS;
     state->partial = state->partial << ENCODED_BITS_PER_UTF8;
     state->partial |= byte;
-    state->bytes_processed++;
+    state->bytes_stored++;
     return 0;
   }
   // Invalid byte -> reset the state
@@ -82,11 +85,10 @@ int CharacterConverter::push(char8_t utf8_byte) {
 
 int CharacterConverter::push(char32_t utf32) {
   // we can't be partially through a conversion when pushing a utf32 value
-  if (!isComplete())
+  if (!isEmpty())
     return -1;
 
   state->partial = utf32;
-  state->bytes_processed = 0;
 
   // determine number of utf-8 bytes needed to represent this utf32 value
   constexpr char32_t MAX_VALUE_PER_UTF8_LEN[] = {0x7f, 0x7ff, 0xffff, 0x10ffff};
@@ -94,6 +96,7 @@ int CharacterConverter::push(char32_t utf32) {
   for (uint8_t i = 0; i < NUM_RANGES; i++) {
     if (state->partial <= MAX_VALUE_PER_UTF8_LEN[i]) {
       state->total_bytes = i + 1;
+      state->bytes_stored = i + 1;
       return 0;
     }
   }
@@ -107,7 +110,7 @@ int CharacterConverter::push(char32_t utf32) {
 ErrorOr<char32_t> CharacterConverter::pop_utf32() {
   // If pop is called too early, do not reset the state, use error to determine
   // whether enough bytes have been pushed
-  if (!isComplete() || state->bytes_processed == 0)
+  if (!isFull())
     return Error(-1);
   char32_t utf32 = state->partial;
   // reset if successful pop
@@ -116,7 +119,7 @@ ErrorOr<char32_t> CharacterConverter::pop_utf32() {
 }
 
 ErrorOr<char8_t> CharacterConverter::pop_utf8() {
-  if (isComplete())
+  if (isEmpty())
     return Error(-1);
 
   constexpr char8_t FIRST_BYTE_HEADERS[] = {0, 0xC0, 0xE0, 0xF0};
@@ -126,8 +129,8 @@ ErrorOr<char8_t> CharacterConverter::pop_utf8() {
 
   // Shift to get the next 6 bits from the utf32 encoding
   const size_t shift_amount =
-      (state->total_bytes - state->bytes_processed - 1) * ENCODED_BITS_PER_UTF8;
-  if (state->bytes_processed == 0) {
+      (state->bytes_stored - 1) * ENCODED_BITS_PER_UTF8;
+  if (isFull()) {
     /*
       Choose the correct set of most significant bits to encode the length
       of the utf8 sequence. The remaining bits contain the most significant
@@ -141,7 +144,7 @@ ErrorOr<char8_t> CharacterConverter::pop_utf8() {
              ((state->partial >> shift_amount) & MASK_ENCODED_BITS);
   }
 
-  state->bytes_processed++;
+  state->bytes_stored--;
   return static_cast<char8_t>(output);
 }
 
diff --git a/libc/src/__support/wchar/character_converter.h b/libc/src/__support/wchar/character_converter.h
index c4ba7cf6b689f..be0e6129df236 100644
--- a/libc/src/__support/wchar/character_converter.h
+++ b/libc/src/__support/wchar/character_converter.h
@@ -26,7 +26,8 @@ class CharacterConverter {
   CharacterConverter(mbstate *mbstate);
 
   void clear();
-  bool isComplete();
+  bool isFull();
+  bool isEmpty();
 
   int push(char8_t utf8_byte);
   int push(char32_t utf32);
diff --git a/libc/src/__support/wchar/mbstate.h b/libc/src/__support/wchar/mbstate.h
index fb08fb4eaa188..fea693f73c3b5 100644
--- a/libc/src/__support/wchar/mbstate.h
+++ b/libc/src/__support/wchar/mbstate.h
@@ -22,10 +22,10 @@ struct mbstate {
 
   /*
   Progress towards a conversion
-    For utf8  -> utf32, increases with each CharacterConverter::push(utf8_byte)
-    For utf32 ->  utf8, increases with each CharacterConverter::pop_utf8()
+    Increases with each push(...) until it reaches total_bytes
+    Decreases with each pop(...) until it reaches 0
   */
-  uint8_t bytes_processed;
+  uint8_t bytes_stored;
 
   // Total number of bytes that will be needed to represent this character
   uint8_t total_bytes;
diff --git a/libc/test/src/__support/wchar/utf32_to_8_test.cpp b/libc/test/src/__support/wchar/utf32_to_8_test.cpp
index f4c5cb863ff38..a6a7bc4aa6f4c 100644
--- a/libc/test/src/__support/wchar/utf32_to_8_test.cpp
+++ b/libc/test/src/__support/wchar/utf32_to_8_test.cpp
@@ -20,17 +20,19 @@ TEST(LlvmLibcCharacterConverterUTF32To8Test, OneByte) {
   // utf8 1-byte encodings are identical to their utf32 representations
   char32_t utf32_A = 0x41; // 'A'
   cr.push(utf32_A);
+  ASSERT_TRUE(cr.isFull());
   auto popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<char>(popped.value()), 'A');
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   char32_t utf32_B = 0x42; // 'B'
   cr.push(utf32_B);
+  ASSERT_TRUE(cr.isFull());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<char>(popped.value()), 'B');
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // should error if we try to pop another utf8 byte out
   popped = cr.pop_utf8();
@@ -45,26 +47,28 @@ TEST(LlvmLibcCharacterConverterUTF32To8Test, TwoByte) {
   // testing utf32: 0xff -> utf8: 0xc3 0xbf
   char32_t utf32 = 0xff;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   auto popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xc3);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xbf);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // testing utf32: 0x58e -> utf8: 0xd6 0x8e
   utf32 = 0x58e;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xd6);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x8e);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // should error if we try to pop another utf8 byte out
   popped = cr.pop_utf8();
@@ -79,34 +83,36 @@ TEST(LlvmLibcCharacterConverterUTF32To8Test, ThreeByte) {
   // testing utf32: 0xac15 -> utf8: 0xea 0xb0 0x95
   char32_t utf32 = 0xac15;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   auto popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xea);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xb0);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x95);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // testing utf32: 0x267b -> utf8: 0xe2 0x99 0xbb
   utf32 = 0x267b;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xe2);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x99);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xbb);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // should error if we try to pop another utf8 byte out
   popped = cr.pop_utf8();
@@ -121,42 +127,44 @@ TEST(LlvmLibcCharacterConverterUTF32To8Test, FourByte) {
   // testing utf32: 0x1f921 -> utf8: 0xf0 0x9f 0xa4 0xa1
   char32_t utf32 = 0x1f921;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   auto popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xf0);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x9f);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xa4);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xa1);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // testing utf32: 0x12121 -> utf8: 0xf0 0x92 0x84 0xa1
   utf32 = 0x12121;
   cr.push(utf32);
+  ASSERT_TRUE(cr.isFull());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xf0);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x92);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0x84);
-  ASSERT_TRUE(!cr.isComplete());
+  ASSERT_TRUE(!cr.isEmpty());
   popped = cr.pop_utf8();
   ASSERT_TRUE(popped.has_value());
   ASSERT_EQ(static_cast<int>(popped.value()), 0xa1);
-  ASSERT_TRUE(cr.isComplete());
+  ASSERT_TRUE(cr.isEmpty());
 
   // should error if we try to pop another utf8 byte out
   popped = cr.pop_utf8();
diff --git a/libc/test/src/__support/wchar/utf8_to_32_test.cpp b/libc/test/src/__support/wchar/utf8_to_32_test.cpp
index 9cb059faa9374..36ae7d689cc00 100644
--- a/libc/test/src/__support/wchar/utf8_to_32_test.cpp
+++ b/libc/test/src/__support/wchar/utf8_to_32_test.cpp
@@ -13,7 +13,7 @@
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, OneByte) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   char ch = 'A';
 
@@ -28,7 +28,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, OneByte) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, TwoBytes) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[2] = {static_cast<char>(0xC2),
                       static_cast<char>(0x8E)}; // Ž car symbol
@@ -44,7 +44,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, TwoBytes) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, ThreeBytes) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[3] = {static_cast<char>(0xE2), static_cast<char>(0x88),
                       static_cast<char>(0x91)}; // ∑ sigma symbol
@@ -61,7 +61,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, ThreeBytes) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, FourBytes) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[4] = {static_cast<char>(0xF0), static_cast<char>(0x9F),
                       static_cast<char>(0xA4),
@@ -80,7 +80,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, FourBytes) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidByte) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch = static_cast<char>(0x80); // invalid starting bit sequence
 
@@ -92,7 +92,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidByte) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidMultiByte) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[4] = {
       static_cast<char>(0x80), static_cast<char>(0x00), static_cast<char>(0x80),
@@ -112,7 +112,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidMultiByte) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidLastByte) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   // Last byte is invalid since it does not have correct starting sequence.
   // 0xC0 --> 11000000 starting sequence should be 10xxxxxx
@@ -132,7 +132,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidLastByte) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, ValidTwoByteWithExtraRead) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[3] = {static_cast<char>(0xC2), static_cast<char>(0x8E),
                       static_cast<char>(0x80)};
@@ -153,7 +153,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, ValidTwoByteWithExtraRead) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, TwoValidTwoBytes) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   const char ch[4] = {static_cast<char>(0xC2), static_cast<char>(0x8E),
                       static_cast<char>(0xC7), static_cast<char>(0x8C)};
@@ -179,7 +179,7 @@ TEST(LlvmLibcCharacterConverterUTF8To32Test, TwoValidTwoBytes) {
 
 TEST(LlvmLibcCharacterConverterUTF8To32Test, InvalidPop) {
   LIBC_NAMESPACE::internal::mbstate state;
-  state.bytes_processed = 0;
+  state.bytes_stored = 0;
   state.total_bytes = 0;
   LIBC_NAMESPACE::internal::CharacterConverter char_conv(&state);
   const char ch[2] = {static_cast<char>(0xC2), static_cast<char>(0x8E)};

>From c491baa2b52b3916d9d7e1e9f65730547bdb0f4a Mon Sep 17 00:00:00 2001
From: Uzair Nawaz <uzairnawaz at google.com>
Date: Wed, 18 Jun 2025 21:45:27 +0000
Subject: [PATCH 2/2] formatting

---
 libc/src/__support/wchar/character_converter.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libc/src/__support/wchar/character_converter.cpp b/libc/src/__support/wchar/character_converter.cpp
index 708af357d15d9..1f81de4248ff0 100644
--- a/libc/src/__support/wchar/character_converter.cpp
+++ b/libc/src/__support/wchar/character_converter.cpp
@@ -35,8 +35,7 @@ void CharacterConverter::clear() {
 }
 
 bool CharacterConverter::isFull() {
-  return state->bytes_stored == state->total_bytes &&
-         state->total_bytes != 0;
+  return state->bytes_stored == state->total_bytes && state->total_bytes != 0;
 }
 
 bool CharacterConverter::isEmpty() { return state->bytes_stored == 0; }
@@ -128,8 +127,7 @@ ErrorOr<char8_t> CharacterConverter::pop_utf8() {
   char32_t output;
 
   // Shift to get the next 6 bits from the utf32 encoding
-  const size_t shift_amount =
-      (state->bytes_stored - 1) * ENCODED_BITS_PER_UTF8;
+  const size_t shift_amount = (state->bytes_stored - 1) * ENCODED_BITS_PER_UTF8;
   if (isFull()) {
     /*
       Choose the correct set of most significant bits to encode the length



More information about the libc-commits mailing list