Skip to content

Commit

Permalink
(stdlib) Support dynamically resizing StringBuilder buffer
Browse files Browse the repository at this point in the history
It would previously use a fixed buffer of 1024 bytes; trying to use more
than this would cause exceptions to be thrown. Growing the buffer using
`realloc()` would have been possible, but the problem is that this would
have required the memory to have been allocated with `malloc()` in the
first place (i.e. not `new[]`).

Because of this, and because of suggestions seen on Stack Overflow, I
rewrote the code to use `std::vector` instead, which have a built-in
`resize()` operation. Also added a C++-based unit test to validate the
new functionality.

https://gitlab.perlang.org/perlang/perlang/-/merge_requests/550
  • Loading branch information
perlun committed Nov 2, 2024
1 parent 12d7956 commit f507373
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 10 deletions.
2 changes: 2 additions & 0 deletions release-notes/v0.6.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
- Avoid unnecessary `strdup()` call in `stdlib_wrappers.cc` [[!538][538]]
- Fix bug in string comparison of dynamic strings [[!540][540]]
- Return non-`const` strings [[!544][544]]
- Support dynamically resizing `StringBuilder` buffer [[!550][550]]

#### CI
- Add `dotnet test` CI job [[!499][499]]
Expand Down Expand Up @@ -129,3 +130,4 @@
[546]: https://gitlab.perlang.org/perlang/perlang/merge_requests/546
[547]: https://gitlab.perlang.org/perlang/perlang/merge_requests/547
[548]: https://gitlab.perlang.org/perlang/perlang/merge_requests/548
[550]: https://gitlab.perlang.org/perlang/perlang/merge_requests/550
1 change: 1 addition & 0 deletions src/stdlib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ install(
if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(TEST_SRC
test/print.cc
test/string_builder.cc
)

add_executable(
Expand Down
18 changes: 9 additions & 9 deletions src/stdlib/src/text/string_builder.cc
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
#include <stdexcept>
#include <cstring>

#include "ascii_string.h"
#include "utf8_string.h"
#include "text/string_builder.h"
#include "perlang_stdlib.h"

namespace perlang::text
{
StringBuilder::StringBuilder()
{
buffer_ = new char[DEFAULT_BUFFER_SIZE]{ 0 };
buffer_capacity_ = DEFAULT_BUFFER_SIZE;
buffer_ = new std::vector<char>(buffer_capacity_);
}

StringBuilder::~StringBuilder()
{
delete[] buffer_;
delete buffer_;
}

void StringBuilder::append(const String& str)
{
if (current_position_ + str.length() >= DEFAULT_BUFFER_SIZE) {
throw std::runtime_error("StringBuilder buffer_ is full. Dynamically resizing it is not yet implemented.");
if (current_position_ + str.length() >= buffer_capacity_) {
buffer_capacity_ = buffer_capacity_ * 2;
buffer_->resize(buffer_capacity_);
}

memcpy((void*)&buffer_[current_position_], str.bytes(), str.length());
memcpy((void*)&buffer_->data()[current_position_], str.bytes(), str.length());
current_position_ += str.length();
length_ += str.length();
}
Expand All @@ -42,6 +42,6 @@ namespace perlang::text
std::unique_ptr<perlang::String> StringBuilder::to_string()
{
// TODO: Make this return ASCIIString for cases when the string contains ASCII-only content.
return UTF8String::from_copied_string(buffer_, length_);
return UTF8String::from_copied_string(buffer_->data(), length_);
}
}
5 changes: 4 additions & 1 deletion src/stdlib/src/text/string_builder.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

#include <vector>

#include "perlang_string.h"

namespace perlang::text
Expand All @@ -13,7 +15,8 @@ namespace perlang::text
// TODO: to make the implementation be thread safe.
const uint DEFAULT_BUFFER_SIZE = 1024;

char* buffer_;
std::vector<char>* buffer_;
uint buffer_capacity_;
uint current_position_ = 0;
uint length_ = 0;

Expand Down
20 changes: 20 additions & 0 deletions src/stdlib/test/string_builder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// string_builder.cc - tests for the perlang::text::StringBuilder class

#include <catch2/catch_test_macros.hpp>

#include "perlang_stdlib.h"

TEST_CASE( "perlang::text::StringBuilder::append, resizing the string beyond its initial capacity" )
{
// Arrange & Act
perlang::text::StringBuilder sb;

for (int i = 0; i < 100; i++) {
sb.append(*perlang::ASCIIString::from_static_string("this is an ASCII string"));
}

uint expected_length = 100 * strlen("this is an ASCII string");

// Assert
REQUIRE(sb.length() == expected_length);
}

0 comments on commit f507373

Please sign in to comment.