Skip to content

Adjust StringBuilder size hint semantics#3627

Open
bobzhang wants to merge 1 commit into
mainfrom
codex/stringbuilder-size-hint
Open

Adjust StringBuilder size hint semantics#3627
bobzhang wants to merge 1 commit into
mainfrom
codex/stringbuilder-size-hint

Conversation

@bobzhang

Copy link
Copy Markdown
Contributor

Summary

  • change StringBuilder() to use an eager default capacity hint of 8 UTF-16 code units
  • define size_hint consistently as UTF-16 code-unit capacity rather than bytes
  • keep StringBuilder(size_hint=0) as the explicit empty-buffer path and make buffer growth handle zero capacity
  • reject negative size_hint values on both buffer-backed and JS concat-backed implementations
  • add tests for explicit zero-capacity growth and negative hint rejection

Validation

  • moon fmt builtin/stringbuilder_buffer.mbt builtin/stringbuilder_concat.mbt builtin/stringbuilder_test.mbt
  • moon check builtin
  • moon test builtin
  • moon info
  • moon ide doc StringBuilder::StringBuilder
  • git diff --check
  • moon check
  • moon test

@bobzhang bobzhang marked this pull request as ready for review May 24, 2026 14:36
Copilot AI review requested due to automatic review settings May 24, 2026 14:36
@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 4484

Coverage remained the same at 94.089%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 3 of 3 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 15853
Covered Lines: 14916
Line Coverage: 94.09%
Coverage Strength: 198865.29 hits per line

💛 - Coveralls

Copilot AI left a comment

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.

Pull request overview

Updates StringBuilder constructor semantics to treat size_hint consistently as a UTF-16 code-unit capacity hint, with a small eager default to reduce early reallocations, and aligns behavior across the buffer-backed (non-JS) and concat-backed (JS) implementations.

Changes:

  • Changed StringBuilder() default size_hint from 0 to 8 (UTF-16 code units) and updated docstrings accordingly.
  • Made size_hint=0 a supported “empty initial buffer” path by allowing zero-capacity buffers and updating growth logic to handle it.
  • Rejected negative size_hint values in both implementations and added tests for zero-capacity growth and negative-hint rejection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
builtin/stringbuilder_buffer.mbt Switches to UTF-16 code-unit capacity hints, allows 0-capacity buffers, and updates growth logic to handle zero capacity.
builtin/stringbuilder_concat.mbt Aligns constructor defaults/docs and rejects negative size_hint even though the hint is ignored on JS.
builtin/stringbuilder_test.mbt Adds coverage for size_hint=0 growth and negative size_hint constructor panics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants