Skip to content

Add UTF-16 Strings #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoltedJon wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add UTF-16 Strings #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c99515
Add UTF-16 Strings
JoltedJon May 30, 2026
dd57179
Coderabbit suggestions
JoltedJon Jun 14, 2026
1d53e8f
Update Error
JoltedJon Jun 17, 2026
9039c3c
Add Const qualifier
JoltedJon Jun 17, 2026
c2047f9
Fix Size
JoltedJon Jun 17, 2026
27175ef
Bytes are funny
JoltedJon Jun 17, 2026
98b3fc3
I may be special
JoltedJon Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Engine/cpp/runtime/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ add_modules_library(definitions PIC)
add_modules_library(math)
add_modules_library(io)
add_modules_library(memory)
add_modules_library(containers)

target_link_libraries(math PUBLIC definitions bx)
target_link_libraries(io PUBLIC definitions stb_image)
target_link_libraries(memory PUBLIC definitions math)
target_link_libraries(containers PUBLIC definitions memory math)
3 changes: 3 additions & 0 deletions Engine/cpp/runtime/core/containers/containers.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module;
export module core.containers;
export import core.containers.string;
101 changes: 101 additions & 0 deletions Engine/cpp/runtime/core/containers/string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
module;

#include <cstring>

module core.containers.string;

namespace draco::containers {

StringBuilder::Error StringBuilder::Reserve(isize newCapacity) {
if (newCapacity <= capacity) {
return Error::Okay;
}

Slice newDst;
StringBuilder::Error err = (StringBuilder::Error)allocator.alloc(&newDst, newCapacity * sizeof(utf16), alignof(utf16));
if (err != Error::Okay) {
return err;
}

memcpy(newDst.data, buffer, size * sizeof(utf16));

@VictorSohier VictorSohier Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note for future issue: Make slice version of memcpy.


Slice oldDst = {
.data = buffer,
.size = (usize)capacity * sizeof(utf16)
};
allocator.free(oldDst);

capacity = newDst.size / sizeof(utf16);
buffer = (utf16 *)newDst.data;

return Error::Okay;
}

StringBuilder::Error StringBuilder::GrowCapacity(isize minCapacity) {
if (capacity >= minCapacity) {
return Error::Okay;
}

isize newCapacity = capacity + draco::math::max(capacity / 2, minCapacity);
return Reserve(newCapacity);
}

StringBuilder::Error StringBuilder::Write(utf16 c) {
Error err = GrowCapacity(size + 1);
if (err != Error::Okay) {
return err;
}

buffer[size++] = c;
return Error::Okay;
}

StringBuilder::Error StringBuilder::Write(rune r) {
if (r > 0x10FFFF || (r >= 0xD800 && r <= 0xDFFF)) {
return Error::InvalidRune;
}

Error err = GrowCapacity(size + 2);
if (err != Error::Okay) {
return err;
}

if (r <= 0xFFFF) {
buffer[size++] = (utf16)r;
return Error::Okay;
}

r -= 0x10000;

utf16 high = 0xD800 + (r >> 10);
utf16 low = 0xDC00 + (r & 0x3FF);

buffer[size++] = high;
buffer[size++] = low;

return Error::Okay;
}

StringBuilder::Error StringBuilder::Write(utf16 const *str) {
// TODO(Jon) assert not null
isize length = StringLength(str);
Error err = GrowCapacity(size + length);
Comment on lines +79 to +82

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing null handling in Write(utf16 const* str) can dereference null.

At Line 81, StringLength(str) is called unconditionally. A null input will crash before returning an error.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Engine/cpp/runtime/core/containers/string.cpp` around lines 79 - 82, The
StringBuilder::Write method with the utf16 const *str parameter calls
StringLength(str) without first checking if the input pointer is null, which
will cause a null pointer dereference and crash. Add a null check at the
beginning of the StringBuilder::Write function (before calling StringLength) and
return an appropriate error code if str is null pointer, similar to how the TODO
comment suggests an assertion should be added.

if (err != Error::Okay) {
return err;
}
memcpy(buffer + size, str, length * sizeof(utf16));
size += length;
return Error::Okay;
}

StringBuilder::Error StringBuilder::Write(String str) {
Error err = GrowCapacity(size + str.size);
if (err != Error::Okay) {
return err;
}
memcpy(buffer + size, str.text, str.size * sizeof(utf16));

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use overlap-safe copy when appending from a String view.

At Line 96, memcpy is undefined for overlapping regions. If str.text points into the builder’s own buffer, this can corrupt output or crash. memmove is the safe primitive here.

Suggested fix 
-	memcpy(buffer + size, str.text, str.size * sizeof(utf16));
+	memmove(buffer + size, str.text, str.size * sizeof(utf16));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
memcpy(buffer + size, str.text, str.size * sizeof(utf16));
memmove(buffer + size, str.text, str.size * sizeof(utf16));
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Engine/cpp/runtime/core/containers/string.cpp` at line 96, In the string
append operation at line 96 where memcpy is called to copy str.text into the
buffer, replace memcpy with memmove to safely handle the case where str.text
points into the builder's own buffer, preventing undefined behavior and data
corruption from overlapping memory regions.

size += str.size;
return Error::Okay;
}

} //namespace draco::containers
Loading
Loading