feat: add ZeroCopyMut enum support #1915
Conversation
WalkthroughThe PR documents ZeroCopyMut enum support, refactors enum codegen helpers to be const-generic over a MUT flag, wires both immutable and mutable paths into derive macros, adds ZeroCopyNew generation for enums, and extends tests to cover mutable deserialization and config-based initialization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User Code
participant Macro as #[derive(ZeroCopy)]
participant ZEnum as z_enum::<MUT=false>
participant Gen as Generated Impl
User->>Macro: derive on Enum
Macro->>ZEnum: generate_z_enum::<false>(...)
ZEnum-->>Macro: ZEnum type (immutable)
Macro->>ZEnum: generate_enum_deserialize_impl::<false>(...)
ZEnum-->>Macro: impl ZeroCopyAt::zero_copy_at
Macro->>ZEnum: generate_enum_zero_copy_struct_inner::<false>(...)
ZEnum-->>Macro: inner structs/aliases
Macro-->>User: Enum::zero_copy_at(&[u8]) available
sequenceDiagram
autonumber
participant User as User Code
participant Macro as #[derive(ZeroCopyMut)]
participant ZEnum as z_enum::<MUT=true>
participant New as generate_enum_zero_copy_new
participant Gen as Generated Impl (mut)
User->>Macro: derive on Enum
Macro->>ZEnum: generate_z_enum::<true>(...)
ZEnum-->>Macro: ZEnumMut type (mutable)
Macro->>ZEnum: generate_enum_deserialize_impl::<true>(...)
ZEnum-->>Macro: impl ZeroCopyAtMut::zero_copy_at_mut
Macro->>ZEnum: generate_enum_zero_copy_struct_inner::<true>(...)
ZEnum-->>Macro: inner mut structs/aliases
Macro->>New: generate_enum_zero_copy_new(...)
New-->>Macro: Config enum + impl ZeroCopyNew
Macro-->>User: Enum::{zero_copy_at_mut,new_zero_copy,byte_len} available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
ananas-block
force-pushed
the
jorrit/feat-zero-copy-mut-enum-support
branch
from
7866817 to
654128f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
program-libs/zero-copy-derive/README.md (1)
51-51: Documentation consistency issue: redundant note about ZeroCopyMut enum supportThe same information about
ZeroCopyMutsupporting enums with unit variants or single unnamed field variants is already mentioned in line 37 under the "Custom Types" section. Consider removing this duplicate to avoid redundancy.program-libs/zero-copy-derive/tests/ui/pass/basic_enum.rs (2)
28-34: Consider verifying the mutated value persists in the underlying bufferWhile the test mutates the value within the enum variant, it doesn't verify that the change is reflected in the underlying byte buffer. Consider adding an assertion to ensure the mutation is properly written through.
Add verification after the mutation:
// Can mutate data within existing variant (discriminant remains immutable) match &mut enum_mut { ZBasicEnumMut::SingleField(ref mut data) => { **data = 100u32.into(); // Modify the u32 value } _ => panic!("Expected SingleField variant"), } + + // Verify the mutation is reflected in the underlying buffer + let modified_enum = BasicEnum::SingleField(100); + let expected_bytes = modified_enum.try_to_vec().unwrap(); + assert_eq!(bytes_mut, expected_bytes, "Mutation should be reflected in buffer");
45-50: Consider adding verification for the initialized valueSimilar to the mutation test above, after initializing the data field to 42, consider verifying that the bytes reflect the expected value.
Add verification after initialization:
// Initialize the data field match &mut enum_new { ZBasicEnumMut::SingleField(ref mut data) => { **data = 42u32.into(); } _ => panic!("Expected SingleField variant"), } + + // Verify the initialized bytes match expected + let expected_enum = BasicEnum::SingleField(42); + let expected_bytes = expected_enum.try_to_vec().unwrap(); + assert_eq!(new_bytes, expected_bytes, "Initialized bytes should match expected");program-libs/zero-copy-derive/tests/ui/pass/40_enum_discriminant_order.rs (1)
32-36: Strengthen the discriminant-order check with an explicit tag assertion.Since this test targets discriminant handling, it’s useful to assert the encoded tag is 5 for V5. This catches regressions in variant ordering.
Apply this diff:
assert!(remaining.is_empty()); + // Discriminant should be one byte with value 5 for V5 + assert_eq!(bytes[0], 5u8);program-libs/zero-copy-derive/tests/ui/pass/complex_enum.rs (2)
24-36: Add a round-trip assertion after mutation.After setting the U64 value to 99999, assert the mutated bytes deserialize back to the expected enum. This protects against write-path regressions.
Apply this diff:
assert!(remaining.is_empty()); // Can mutate data within existing variant (discriminant remains immutable) match &mut enum_mut { ZComplexEnumMut::U64Field(ref mut data) => { **data = 99999u64.into(); // Modify the u64 value } _ => panic!("Expected U64Field variant"), } + + // Round-trip check + let round_tripped = ComplexEnum::try_from_slice(&bytes_mut).unwrap(); + assert!(matches!(round_tripped, ComplexEnum::U64Field(99999)));
37-51: Assert the constructed bytes match the canonical borsh encoding.Once you set the value to 12345 in the newly constructed enum, verify new_bytes equals the earlier borsh bytes for the same value.
Apply this diff:
match &mut enum_new { ZComplexEnumMut::U64Field(ref mut data) => { **data = 12345u64.into(); } _ => panic!("Expected U64Field variant"), } + + // new_zero_copy should produce the same canonical bytes as borsh + assert_eq!(new_bytes, bytes);program-libs/zero-copy-derive/tests/ui/pass/39_enum_with_array.rs (1)
29-46: Assert canonical bytes after initialization.After filling the 32-byte array with 42s, assert the new bytes match the borsh-encoded baseline. This guarantees correctness of both discriminant and payload layout.
Apply this diff:
_ => panic!("Expected WithArray variant"), } + + // Ensure canonical encoding matches the reference + assert_eq!(new_bytes, bytes);program-libs/zero-copy-derive/src/shared/zero_copy_new.rs (4)
579-583: Derive PartialEq/Eq for the generated config enum.This makes tests and downstream code simpler when matching or comparing configs. Harmless extra bounds.
Apply this diff:
- #[derive(Debug, Clone)] + #[derive(Debug, Clone, PartialEq, Eq)] pub enum #config_name { #(#config_variants,)* }
529-533: Use checked_add when composing discriminant + payload length.Avoids theoretical usize overflow in byte_len calculations, consistent with the defensive style used elsewhere.
Apply this diff inside the generated match arm:
- <#field_type as ::light_zero_copy::traits::ZeroCopyNew>::byte_len(config) - .map(|len| #discriminant_size + len) + <#field_type as ::light_zero_copy::traits::ZeroCopyNew>::byte_len(config) + .and_then(|len| len + .checked_add(#discriminant_size) + .ok_or(::light_zero_copy::errors::ZeroCopyError::Size))
598-605: Align error type for empty buffer with the rest of the API.Other paths use InsufficientMemoryAllocated(current, required). Consider using the same for consistency and clearer diagnostics.
Apply this diff:
- if bytes.is_empty() { - return Err(::light_zero_copy::errors::ZeroCopyError::ArraySize(1, bytes.len())); - } + if bytes.is_empty() { + return Err(::light_zero_copy::errors::ZeroCopyError::InsufficientMemoryAllocated(bytes.len(), 1)); + }
540-576: Optional: preflight capacity check using byte_len(config).Not required, but calling byte_len(config) and ensuring bytes.len() >= that value up front can return a precise error earlier, especially for large inner payloads.
If desired, augment new_zero_copy as follows:
fn new_zero_copy( bytes: &'a mut [u8], config: Self::ZeroCopyConfig, ) -> Result<(Self::Output, &'a mut [u8]), ::light_zero_copy::errors::ZeroCopyError> { - if bytes.is_empty() { + if bytes.is_empty() { return Err(::light_zero_copy::errors::ZeroCopyError::InsufficientMemoryAllocated(bytes.len(), 1)); } + // Preflight required length for better error messages + let required = Self::byte_len(&config)?; + if bytes.len() < required { + return Err(::light_zero_copy::errors::ZeroCopyError::InsufficientMemoryAllocated(bytes.len(), required)); + } match config { #(#new_arms,)* } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
program-libs/zero-copy-derive/README.md(1 hunks)program-libs/zero-copy-derive/src/lib.rs(1 hunks)program-libs/zero-copy-derive/src/shared/z_enum.rs(9 hunks)program-libs/zero-copy-derive/src/shared/zero_copy_new.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy.rs(1 hunks)program-libs/zero-copy-derive/src/zero_copy_mut.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/09_enum_unit_variants.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/10_enum_mixed_variants.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/21_enum_single_variant.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/39_enum_with_array.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/40_enum_discriminant_order.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/68_enum_containing_option.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/basic_enum.rs(2 hunks)program-libs/zero-copy-derive/tests/ui/pass/complex_enum.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
program-libs/zero-copy-derive/src/shared/zero_copy_new.rs (1)
program-libs/zero-copy-derive/src/shared/z_enum.rs (1)
enum_data(22-82)
program-libs/zero-copy-derive/src/zero_copy.rs (1)
program-libs/zero-copy-derive/src/shared/z_enum.rs (1)
enum_data(22-82)
program-libs/zero-copy-derive/src/shared/z_enum.rs (3)
program-libs/zero-copy-derive/src/zero_copy.rs (3)
generate_z_enum(307-307)generate_enum_deserialize_impl(309-309)generate_enum_zero_copy_struct_inner(311-311)program-libs/zero-copy-derive/src/zero_copy_mut.rs (3)
generate_z_enum(142-142)generate_enum_deserialize_impl(144-144)generate_enum_zero_copy_struct_inner(146-146)program-libs/zero-copy-derive/src/shared/zero_copy_new.rs (3)
enum_data(490-508)enum_data(511-537)enum_data(540-575)
program-libs/zero-copy-derive/src/zero_copy_mut.rs (6)
program-libs/zero-copy-derive/src/shared/z_enum.rs (4)
generate_enum_deserialize_impl(118-242)generate_enum_zero_copy_struct_inner(246-284)generate_z_enum(7-114)enum_data(22-82)program-libs/zero-copy-derive/src/zero_copy.rs (6)
generate_enum_deserialize_impl(309-309)generate_enum_zero_copy_struct_inner(311-311)generate_z_enum(307-307)generate_z_struct(276-276)generate_deserialize_impl(147-215)generate_deserialize_impl(287-287)program-libs/zero-copy-derive/src/shared/utils.rs (2)
process_input_generic(67-100)process_fields(102-121)program-libs/zero-copy-derive/src/shared/z_struct.rs (3)
name(28-44)generate_z_struct(369-471)analyze_struct_fields(166-179)program-libs/zero-copy-derive/src/shared/meta_struct.rs (1)
generate_meta_struct(9-57)program-libs/zero-copy-derive/src/shared/zero_copy_new.rs (5)
generate_init_mut_impl(19-132)enum_data(490-508)enum_data(511-537)enum_data(540-575)generate_enum_zero_copy_new(482-608)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-v2
- GitHub Check: lint
- GitHub Check: cli-v2
- GitHub Check: cli-v1
- GitHub Check: Test program-libs-slow
- GitHub Check: Test sdk-libs
- GitHub Check: Test batched-merkle-tree-simulate
🔇 Additional comments (30)
program-libs/zero-copy-derive/src/lib.rs (1)
51-51: Documentation update looks goodThe addition correctly documents that
ZeroCopyMutnow supports enums with unit variants or single unnamed field variants, aligning with the implementation changes in this PR.program-libs/zero-copy-derive/src/shared/z_enum.rs (6)
6-16: Well-designed const generic approach for code reuseThe introduction of the
const MUT: boolparameter elegantly unifies the mutable and immutable code generation paths, avoiding code duplication while maintaining type safety through compile-time branching.
42-57: Clean type alias generation with proper trait selectionThe conditional generation of type aliases (
TypeMutvsType) with corresponding trait selection (ZeroCopyAtMutvsZeroCopyAt) is well-implemented and maintains consistency across the mutable/immutable divide.
85-89: Correct derive attribute selection based on mutabilityThe conditional derive attributes correctly exclude
Clonefor mutable enums (since mutable references cannot be cloned), while preserving it for immutable variants. This is the right design decision.
131-145: Comprehensive trait and method selection for deserializationThe tuple destructuring pattern for selecting the appropriate trait, mutability keyword, method name, and associated type based on the
MUTflag is clean and maintainable.
224-225: Good safety note about discriminant immutabilityThe comment correctly documents that discriminants remain immutable even in mutable deserialization for safety reasons. This is an important invariant to maintain.
271-283: Proper trait implementation branching for struct innerThe conditional generation of
ZeroCopyStructInnerMutvsZeroCopyStructInnerimplementations correctly follows the same pattern established in the other functions.program-libs/zero-copy-derive/tests/ui/pass/09_enum_unit_variants.rs (1)
1-40: Comprehensive test coverage for unit variant enumsThe test thoroughly exercises the new mutable zero-copy functionality for unit-only enums, including deserialization, initialization, and byte length verification. The assertions properly validate the round-trip behavior.
program-libs/zero-copy-derive/tests/ui/pass/40_enum_discriminant_order.rs (2)
4-8: Enums now derive ZeroCopyMut and tests import the right traits — LGTM.Imports and derives are consistent with the new mutable/init API. Good.
37-46: ZeroCopyNew path for unit variants looks correct.Config, byte_len, initialization, and “remaining” checks align with the generated API; comparing new_bytes to the borsh-encoded reference is a good end-to-end assertion.
program-libs/zero-copy-derive/src/zero_copy.rs (1)
306-312: Refactor Verified: Generic enum helpers correctly wired for both MUT=false and MUT=true
- zero_copy.rs (lines 306–312) uses
generate_z_enum::<false>,generate_enum_deserialize_impl::<false>, andgenerate_enum_zero_copy_struct_inner::<false>as intended.- zero_copy_mut.rs (lines 142, 144, 146) instantiates the corresponding
::<true>variants.- A repository-wide search confirms no remaining calls to the old non-generic helpers.
The migration unifies the enum derive paths without altering existing behavior—approving these changes.
program-libs/zero-copy-derive/tests/ui/pass/complex_enum.rs (1)
3-7: Enum mutability and init support — LGTM.Imports and derives match the new API surface (ZeroCopyAtMut, ZeroCopyNew, ZeroCopyMut).
program-libs/zero-copy-derive/tests/ui/pass/39_enum_with_array.rs (2)
4-8: Expanded imports and derives — LGTM.The test is aligned with the new ZeroCopyMut and ZeroCopyNew support for enums.
24-28: Mutable deserialization smoke check — LGTM.Good minimal assertion that the whole buffer is consumed.
program-libs/zero-copy-derive/tests/ui/pass/68_enum_containing_option.rs (5)
4-5: LGTM! Import additions are appropriate for the new functionality.The added imports for
ZeroCopyAtMutandZeroCopyNewcorrectly support the mutable zero-copy and initialization pathways being tested.
7-7: LGTM! Derive macro addition is consistent with PR objectives.Adding
ZeroCopyMutto the derive list properly enables mutable enum support as intended.
22-23: Good practice: Proper tuple destructuring and validation.The explicit destructuring with
remainingvalidation ensures complete data consumption, which is crucial for zero-copy correctness.
25-28: LGTM! Mutable deserialization test is properly implemented.The test correctly exercises the
ZeroCopyMutpath and validates that all bytes are consumed.
30-36: LGTM! ZeroCopyNew initialization test is comprehensive.The test properly covers the full initialization flow with config setup, byte length calculation, buffer allocation, and remaining bytes validation.
program-libs/zero-copy-derive/tests/ui/pass/21_enum_single_variant.rs (3)
4-5: LGTM! Import additions align with new trait requirements.The imports correctly support the new mutable and initialization capabilities being tested.
22-25: LGTM! Mutable deserialization test follows consistent pattern.The test properly validates the mutable deserialization path for the single-variant enum case.
27-35: Excellent test coverage for initialization flow.The test comprehensively validates:
- Config creation for single variant
- Byte length calculation correctness
- Successful initialization with proper remaining bytes handling
- Final bytes matching the original serialized data
This provides strong confidence in the implementation.
program-libs/zero-copy-derive/tests/ui/pass/10_enum_mixed_variants.rs (3)
2-3: LGTM! Feature gate and derive addition are correct.The
mutfeature gate andZeroCopyMutderive properly enable the mutable functionality.
29-35: LGTM! Proper demonstration of mutable variant field modification.The test correctly shows that variant data can be modified while the discriminant remains immutable, which is the expected behavior for enum safety.
37-51: LGTM! Comprehensive initialization test with field mutation.The test properly exercises the
ZeroCopyNewinitialization path and demonstrates field initialization capabilities.program-libs/zero-copy-derive/src/zero_copy_mut.rs (5)
8-10: LGTM! Appropriate imports for enum support.The imports correctly bring in the const-generic enum generation functions needed for mutable support.
21-22: Good improvement: Generic validation message.The updated validation message correctly references both structs and enums, improving clarity for users.
26-27: LGTM! Clean abstraction with generic input processing.Using
process_input_genericelegantly handles both structs and enums through a unified interface.
29-136: Well-structured struct processing with proper hygiene isolation.The struct processing logic is comprehensive and well-organized:
- Correctly separates meta and struct fields
- Generates all necessary components with
MUT=trueconst generic- Properly handles both dynamic and fixed-size field configurations
- Uses hygiene isolation to prevent namespace pollution while keeping types public
137-171: Clean enum implementation with consistent const-generic approach.The enum processing correctly:
- Reuses existing const-generic functions with
MUT=true- Generates all necessary components including ZeroCopyNew support
- Maintains consistent hygiene isolation pattern with struct processing
- Properly organizes public types and isolated implementations
| /// Generate ZeroCopyNew for enums with fixed variant selection | ||
| pub fn generate_enum_zero_copy_new( | ||
| enum_name: &syn::Ident, | ||
| enum_data: &syn::DataEnum, | ||
| ) -> syn::Result<proc_macro2::TokenStream> { | ||
| let config_name = quote::format_ident!("{}Config", enum_name); | ||
| let z_enum_mut_name = quote::format_ident!("Z{}Mut", enum_name); | ||
|
|
There was a problem hiding this comment.
Guard against enums with > 256 variants (u8 discriminant overflow).
The code writes the variant tag as a u8. If an enum has more than 256 variants, the tag would silently truncate. Add a compile-time (macro-time) check to fail derivation early.
Apply this diff:
pub fn generate_enum_zero_copy_new(
enum_name: &syn::Ident,
enum_data: &syn::DataEnum,
) -> syn::Result<proc_macro2::TokenStream> {
+ // Borsh encodes enum variant index as a single u8; enforce this invariant at macro time.
+ let variant_count = enum_data.variants.len();
+ if variant_count > 256 {
+ return Err(syn::Error::new_spanned(
+ enum_name,
+ format!(
+ "ZeroCopyNew for enums supports at most 256 variants (found {}). Borsh encodes the tag as u8.",
+ variant_count
+ ),
+ ));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Generate ZeroCopyNew for enums with fixed variant selection | |
| pub fn generate_enum_zero_copy_new( | |
| enum_name: &syn::Ident, | |
| enum_data: &syn::DataEnum, | |
| ) -> syn::Result<proc_macro2::TokenStream> { | |
| let config_name = quote::format_ident!("{}Config", enum_name); | |
| let z_enum_mut_name = quote::format_ident!("Z{}Mut", enum_name); | |
| /// Generate ZeroCopyNew for enums with fixed variant selection | |
| pub fn generate_enum_zero_copy_new( | |
| enum_name: &syn::Ident, | |
| enum_data: &syn::DataEnum, | |
| ) -> syn::Result<proc_macro2::TokenStream> { | |
| // Borsh encodes enum variant index as a single u8; enforce this invariant at macro time. | |
| let variant_count = enum_data.variants.len(); | |
| if variant_count > 256 { | |
| return Err(syn::Error::new_spanned( | |
| enum_name, | |
| format!( | |
| "ZeroCopyNew for enums supports at most 256 variants (found {}). \ | |
| Borsh encodes the tag as u8.", | |
| variant_count | |
| ), | |
| )); | |
| } | |
| let config_name = quote::format_ident!("{}Config", enum_name); | |
| let z_enum_mut_name = quote::format_ident!("Z{}Mut", enum_name); | |
| // …rest of the implementation… | |
| } |
🤖 Prompt for AI Agents
program-libs/zero-copy-derive/src/shared/zero_copy_new.rs around lines 481 to
488: The function generates a u8 discriminant for enum variants but doesn't
guard against enums with more than 256 variants which would silently overflow;
add a macro-time check at the start of generate_enum_zero_copy_new that inspects
enum_data.variants.len() and if it is greater than 256 returns
Err(syn::Error::new_spanned(enum_name, "enum has more than 256 variants; u8
discriminant would overflow")); this ensures derivation fails early with a clear
compile error.
Summary by CodeRabbit