-
Notifications
You must be signed in to change notification settings - Fork 220
Add validate_with to derive(ULE) and derive(VarULE) #2733
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,10 @@ use proc_macro2::TokenStream as TokenStream2; | |
use syn::parse::{Parse, ParseStream}; | ||
use syn::punctuated::Punctuated; | ||
use syn::spanned::Spanned; | ||
use syn::{parenthesized, parse2, Attribute, Error, Field, Fields, Ident, Index, Result, Token}; | ||
use syn::{ | ||
parenthesized, parse2, Attribute, Error, Field, Fields, Ident, Index, Lit, Meta, MetaNameValue, | ||
Path, Result, Token, | ||
}; | ||
|
||
// Check that there are repr attributes satisfying the given predicate | ||
pub fn has_valid_repr(attrs: &[Attribute], predicate: impl Fn(&Ident) -> bool + Copy) -> bool { | ||
|
@@ -275,3 +278,43 @@ pub fn extract_attributes_common( | |
|
||
Ok(attrs) | ||
} | ||
|
||
pub(crate) fn find_validate_with_path( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: This should be a part of the attribute parsing above in |
||
attrs: &[Attribute], | ||
attr_path: &Path, | ||
) -> Result<Option<Path>> { | ||
let mut validate_with: Option<Path> = None; | ||
let validate_with_path: Path = syn::parse_str("validate_with").unwrap(); | ||
for attr in attrs.iter() { | ||
if &attr.path != attr_path { | ||
continue; | ||
} | ||
match attr.parse_args::<Meta>() { | ||
Ok(Meta::NameValue(MetaNameValue { | ||
path, | ||
lit: Lit::Str(lit_str), | ||
.. | ||
})) if path == validate_with_path => { | ||
if validate_with.is_some() { | ||
return Err(Error::new(attr.span(), "multiple varule are not allowed")); | ||
} | ||
validate_with = match syn::parse_str(&lit_str.value()) { | ||
Ok(p) => Some(p), | ||
Err(_) => { | ||
return Err(Error::new( | ||
attr.span(), | ||
"varule value must be a path to a function", | ||
)); | ||
} | ||
} | ||
} | ||
_ => { | ||
return Err(Error::new( | ||
attr.span(), | ||
"varule takes a single name value, validate_with = \"fn_path\"", | ||
)); | ||
} | ||
} | ||
} | ||
Ok(validate_with) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ use proc_macro2::Span; | |
use proc_macro2::TokenStream as TokenStream2; | ||
use quote::quote; | ||
use syn::spanned::Spanned; | ||
use syn::{Data, DeriveInput, Error, Ident}; | ||
use syn::{Data, DeriveInput, Error, Ident, Path}; | ||
|
||
/// Implementation for derive(VarULE). `custom_varule_validator` validates the last field bytes `last_field_bytes` | ||
/// if specified, if not, the VarULE implementation will be used. | ||
|
@@ -86,6 +86,19 @@ pub fn derive_impl( | |
quote!(<#unsized_field as zerovec::ule::VarULE>::validate_byte_slice(last_field_bytes)?;) | ||
}; | ||
|
||
let attr_path: Path = syn::parse_str("varule").unwrap(); | ||
let validate_with = match utils::find_validate_with_path(&input.attrs, &attr_path) { | ||
Ok(Some(fn_path)) => { | ||
quote! { | ||
#fn_path(unsafe { Self::from_byte_slice_unchecked(bytes) })?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought: for ULE this should be free but for VarULE this will be a couple extra instructions. i suppose we don't care about that? this way of doing it does make the derive code simpler. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are you comparing against? Note that the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, sorry, here we construct an |
||
} | ||
} | ||
Ok(None) => { | ||
quote! {} | ||
} | ||
Err(e) => return e.to_compile_error(), | ||
}; | ||
|
||
// Safety (based on the safety checklist on the ULE trait): | ||
// 1. #name does not include any uninitialized or padding bytes | ||
// (achieved by enforcing #[repr(transparent)] or #[repr(packed)] on a struct of only ULE types) | ||
|
@@ -111,6 +124,7 @@ pub fn derive_impl( | |
#[allow(clippy::indexing_slicing)] // TODO explain | ||
let last_field_bytes = &bytes[#remaining_offset..]; | ||
#last_field_validator | ||
#validate_with | ||
Ok(()) | ||
} | ||
#[inline] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this should be
zerovec::ule
. we do that consistently elsewhere.It's also fine for it to be
zerovec::validate_with
, orzerovec(validate_with =)
though that last pattern isn't used so far in this crate