Drop .clone() on closed-union string() methods#1
Merged
Conversation
The Category / GraphemeBreak / Script / BinaryProperty / AllValid
primitives all had
fun string(): String iso^ => "X".clone()
which allocates a fresh String iso on every call. The values are
literal string constants — returning them directly as String val
is shareable, cheaper, and clearer:
fun string(): String val => "X"
Side-effect: these primitives no longer structurally satisfy
Pony's `Stringable` interface (which requires `String iso^`). None
of them declare `is Stringable` and no internal caller passes
them to a `Stringable`-expecting position. If a future caller
needs Stringable, wrap with `.clone()` at the call site.
The errors.pony types (InvalidUtf8, OutOfRange, InvalidScalar)
and Codepoint.string() build their output dynamically via
`s.append(...)` — they still return `String iso^`.
Codegen sources updated:
unicode_build/binary_props_table.pony
unicode_build/script_table.pony
Generated files regenerated via `make ucd-generate`.
CI was using unicode.org/UCD/latest (currently 16.0.0) while the committed tables were generated from the local UCD snapshot (14.0.0). This mismatch caused 281 NormalizationTest.txt failures on codepoints added in 15.x/16.0 (Todhri script, Tulu-Tigalari script, MODIFIER LETTER CAPITAL S, etc.). Pinning the UCD version in the Makefile keeps CI and local in sync. Bumping UCD_VERSION is now a deliberate, reviewed step: update the variable, `make ucd-download && make ucd-generate`, verify `make conform` passes, commit the regenerated tables. Local + CI now run against Unicode 16.0.0: 146 unit tests pass 19,965 / 19,965 NormalizationTest.txt cases pass (100%)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
Category/GraphemeBreak/Script/BinaryProperty/AllValidprimitives all had:```pony
fun string(): String iso^ => "X".clone()
```
which allocates a fresh `String iso` on every call. The values are literal string constants — returning them directly as `String val` is shareable, cheaper, and clearer:
```pony
fun string(): String val => "X"
```
Side effect: Stringable
These primitives no longer structurally satisfy Pony's builtin `Stringable` interface (which requires `String iso^`). None declare `is Stringable` explicitly, and no internal caller passes them to a `Stringable`-expecting position. If a future caller needs `Stringable`, wrap with `.clone()` at the call site.
The `errors.pony` types (`InvalidUtf8`, `OutOfRange`, `InvalidScalar`) and `Codepoint.string()` build their output dynamically via `s.append(...)` — those still return `String iso^`.
Files
Test plan