feat: tighten serde-shape for 0.1.0 - #14
Merged
Merged
Conversation
Why: Serde implements arrays only through length 32 and deliberately omits element bounds for length zero. The previous const-generic shape impl both advertised unsupported larger arrays and rejected valid empty arrays whose element type has no shape. Mirror Serde's coverage and represent the unobserved empty-array element explicitly.
Why: ShapeId stores only a graph-local index, so lookups cannot distinguish a foreign id whose index is valid in the current graph. Documenting automatic rejection encouraged silent cross-graph misuse; make the caller's pairing responsibility explicit without adding nondeterministic graph identity machinery.
Why: Serde remote derives already declare a foreign type's wire fields, names, and tagging, so treating the entire helper as opaque discards known metadata and blocks a standard foreign-type workflow. Reuse the normal derive path and retain opacity only for genuinely unknowable custom functions.
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
#[serde(remote = "...")]helpers so foreign-type adapters are usable in shape graphsShapeIdas an index-only graph-local handle instead of claiming that lookups can identify foreign graph idsReview structure
The three commits are independent and are the intended review order. Each commit message has a
Why:paragraph tying the change to an observable Serde or public-API contract.Deliberate non-changes
This audit found no redundant test boundary worth deleting. It also does not add a generic graph walker, enable derive by default, introduce speculative constructors, or mix version/tag/publish mechanics into the API review. Those changes do not have enough current evidence and would increase the false-positive surface.
Validation
cargo x build --lockedcargo x testcargo x lintcargo x package --lockedcargo +1.85.0 x testcargo +1.85.0 check -p serde-shape --lib --no-default-features --target thumbv7em-none-eabihfcargo +1.85.0 check -p serde-shape-test-no-std --lib --target thumbv7em-none-eabihf