Add a borsh feature to implement Borsh traits - #448
Conversation
Adds an optional 'borsh' feature implementing BorshSerialize and BorshDeserialize for SmallVec, matching the existing serde support in shape. Schema support (BorshSchema) is gated behind an additional 'borsh-unstable__schema' feature, mirroring borsh's own unstable__schema flag, since that flag pulls in borsh's derive machinery and is explicitly documented as unstable upstream. Closes servo#291
|
Hi maintainers — this is my first PR to this project. Could someone approve the CI workflow run so it can execute ( |
|
Hi @BugenZhao @ErisianArchitect — first contribution here. The CI workflow is pending maintainer approval (action_required) since I'm a first-time contributor. Could someone approve the run so the borsh feature tests execute? The change adds an optional |
The previous BorshDeserialize impl built a Vec<T> and converted via from_vec(), which always keeps the Vec's heap allocation - so every borsh-deserialized SmallVec spilled to the heap regardless of length, defeating the type's purpose. Deserialize element-by-element instead (same wire format: u32 length prefix + elements), matching how the existing serde Deserialize impl above already does it via push(). Added a regression test asserting a result that fits inline actually stays inline (spilled() == false), plus verified with real cargo: - cargo test --all-features (nightly, matching CI's all-features job) - cargo build --target thumbv7m-none-eabi --no-default-features --features borsh (verifies the no_std claim in the doc comment) - cargo +nightly fmt --all --check (matching CI's style check) All 76 tests pass; ran cargo fmt to fix formatting CI would have flagged on the new use-statements and BorshSchema impl.
|
AI contributions are not allowed in any @servo repository as indicated on the contributing guidelines https://book.servo.org/contributing/getting-started.html#ai-contributions this PR will subsequently be closed |
|
I'm not sure why you tagged me. I made one pull request to this repository. I don't have any authority here. |
|
@ErisianArchitect AI is AI |
3 participants
Summary
Adds an optional
borshfeature soSmallVecimplementsborsh::BorshSerializeandborsh::BorshDeserialize, matching the shape of the existingserdesupport.BorshSchemasupport is gated behind an additionalborsh-unstable__schemafeature, mirroring borsh's ownunstable__schemaflag — that flag is explicitly documented as unstable upstream and pulls in borsh's derive machinery, so I didn't want to make it part of the baseborshfeature.Implementation notes
serialize/deserialize_readerdelegate to the existingVec<T>/[T]impls inborsh, so the wire format is identical to a plainVec<T>(verified in the added test).test_borshmirroring the existingtest_serdetest, plus a check that the encoded bytes matchVec<T>'s encoding exactly.borshis pulled in withdefault-features = false, consistent with howserde_coreis used here, so this staysno_std-compatible.Status
First-time contributor here — I don't have a local Rust toolchain to pre-validate against, so I'm relying on this repo's CI (
cargo test --all-featuresetc.) to confirm the approach. Happy to iterate on any failures.Closes #291