Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve CompactZstd macro #13277

Merged
merged 3 commits into from
Dec 11, 2024
Merged

chore: improve CompactZstd macro #13277

merged 3 commits into from
Dec 11, 2024

Conversation

klkvr
Copy link
Collaborator

@klkvr klkvr commented Dec 10, 2024

Replaces some references to bytes crate with reth_codecs::__private::bytes

Changes CompactZstd macro to receive compressor and decompressor from user's input in reth_zstd attribute instead of expecting them to be available as {struct_name}_COMPRESSOR

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, tysm for coming up with a better solution

}

/// Adds `zstd` compression to derived [`Compact`].
#[proc_macro_derive(CompactZstd, attributes(maybe_zero, reth_codecs))]
#[proc_macro_derive(CompactZstd, attributes(maybe_zero, reth_codecs, reth_zstd))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding reth_zstd makes sense, this a lot smarter than adding it to reth_codecs because then we don't run into the issue I had on my pr

let is_zstd = true;
compact::derive(input, is_zstd)
let derive_input = {
let input = input.clone();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if we need to do this here or if we can do this

let DeriveInput { ident, data, generics, attrs, .. } = parse_macro_input!(input);

and get rid of the clone?
unsure though, we don't use zsdt derive a lot so this seems fine anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated compact::derive to just accept a pre-parsed DeriveInput, we don't need clone that way

@klkvr klkvr enabled auto-merge December 11, 2024 11:45
@klkvr klkvr added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit 394f973 Dec 11, 2024
42 checks passed
@klkvr klkvr deleted the klkvr/improve-zstd-derive branch December 11, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants