Feat/improve apis - #48
Merged
Merged
Conversation
added 3 commits
September 8, 2026 14:35
Applies the same private-field + Box<str> + accessor treatment used previously for Ref/XStr to the remaining scalar value types that held a public String field: Str, Symbol, Uri. - Str/Symbol/Uri: value field changed from `pub value: String` to a private `value: Box<str>`, with a new `value(&self) -> &str` accessor on all three (Str additionally keeps its pre-existing as_str()/Deref<Target=str>/AsRef<str>). - Updated every direct field-access/struct-literal-construction call site across c_api, defs, json/brio/zinc/trio encode+decode, filter, dict.rs/dis_macro.rs, and tests to use the new accessor/constructors. - Fixed one doctest and one PartialEq impl pair (Str vs str/String) that needed adjusting for the new Box<str> field type. Verified via a throwaway size-probe test: Str/Symbol/Uri 24 -> 16 bytes each; Value stays at 40 bytes (Dict/Ref/XStr remain the tied largest variants at 32B). cargo build --all-targets, cargo test --all-targets (816 tests), cargo test --doc (131), and cargo clippy --all-targets all clean.
Str's Display impl goes through the generic write!/format_args! machinery. Since Str just wraps a Box<str>, add an inherent to_string() that derefs to &str first and delegates to str's own specialized ToString impl (a direct byte copy via String::from), which the blanket ToString impl for Box<str> does not get (that specialization only applies to str/String/char/Cow<str>, not Box<str> itself). The inherent method intentionally shadows ToString::to_string (same output, verified by a new test), so clippy::inherent_to_string_shadow_display is allowed with a comment. Also routes the existing TryFrom<&Value> for String impl through it for consistency.
|
Coverage after merging feat/improve-apis into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging feat/improve-apis into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging feat/improve-apis into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
added 3 commits
September 9, 2026 18:55
Lets consumers take ownership of the internal Box<str> directly without going through a String round-trip.
…/Symbol/Ref, with tests Mirrors the Str precedent from the previous commits: - Uri/Symbol: into_inner(self) -> Box<str>, From<Box<str>> - Ref: into_inner(self) -> (Box<str>, Option<Box<str>>), From<Box<str>> (dis: None) - XStr: into_inner(self) -> (Box<str>, Box<str>) for (type, value); no From<Box<str>> since XStr requires two fields Adds 8 new tests (329 total, up from 321) covering the new methods. Validated with cargo build --all-targets, cargo test --all-targets, cargo test --doc, and cargo clippy --all-targets (all clean).
|
Coverage after merging feat/improve-apis into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
garethj2
approved these changes
Sep 10, 2026
…into_inner to into_parts
Several decode paths (zinc scalar, brio, trio, filter lexer) built a
Str/Uri/Symbol/Ref/XStr from a byte buffer or another owned String by
first borrowing it as &str (sometimes via from_utf8_lossy(..).into_owned(),
which always allocates even when the bytes are already valid UTF-8), even
though the source buffer/String was locally owned and about to be dropped.
Fixed by moving the owned data in directly:
- zinc scalar decoders (str/uri/symbol/reference/id): use
String::from_utf8(buf) to reuse the already-owned buffer in the common
valid-UTF8 case, falling back to from_utf8_lossy only on invalid input.
- zinc reference decoder: build via Ref::from(value) + set_dis(dis) instead
of Ref::make(&str, Option<&str>), avoiding a clone of both the id and dis.
- zinc xstr decoder: use Str::into_inner() to move the parsed value's Box<str>
straight into XStr::make instead of re-cloning via value.as_str().
- brio decoder: CTRL_STR/CTRL_SYMBOL now move the decoded String directly via
From<String> instead of Value::from(&str)/Symbol::make(&str); CTRL_REF_STR/
CTRL_REF_I8 use a new make_ref() helper built on Ref::from()+set_dis();
CTRL_XSTR and decode_buf_as_xstr pass owned Strings directly.
- trio decoder: Str::from(lines.join("\n")) instead of Str::make(&lines.join(..)).
- filter lexer: Id gained a Box<str>-backed into_inner(); the Rel-symbol path
uses Symbol::from(id.into_inner()) for a true zero-copy handoff instead of
going through Display/to_string().
Widened XStr::make() to accept impl Into<Box<str>> for both parameters (was
&str only) so callers can pass an already-owned String/Box<str> without an
extra allocation; existing &str call sites are unaffected since &str already
implements Into<Box<str>>.
Renamed Ref::into_inner/XStr::into_inner to into_parts (they return a tuple
of fields, unlike the single-Box<str> into_inner on Str/Uri/Symbol), added
Ref::from_parts as the inverse constructor, and added tests for
into_parts/from_parts and their round-trip on Ref, plus into_parts on XStr.
Validated with cargo build --all-targets, cargo test --all-targets (331
passed), cargo test --doc (131 passed), and cargo clippy --all-targets (clean).
|
Coverage after merging feat/improve-apis into master will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
2 participants
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.
No description provided.