-
Notifications
You must be signed in to change notification settings - Fork 134
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
Implement #[zeroize(bound = "T: MyTrait")]
#663
Conversation
@daxpedda needs a rebase |
8c41276
to
6f4849d
Compare
Done! |
panic!( | ||
concat!( | ||
"The #[zeroize(bound)] attribute is not allowed on {} fields. ", | ||
"Use it on the containing {} instead.", | ||
), | ||
item_kind, item_kind, | ||
) | ||
} | ||
(Some(_variant), None) => panic!(concat!( | ||
"The #[zeroize(bound)] attribute is not allowed on enum variants. ", | ||
"Use it on the containing enum instead.", | ||
)), | ||
(None, None) => { | ||
if let Meta::NameValue(meta_name_value) = meta { | ||
if let Lit::Str(lit) = &meta_name_value.lit { | ||
if lit.value().is_empty() { | ||
self.bound = Some(Bounds(Punctuated::new())); | ||
} else { | ||
self.bound = Some(lit.parse().unwrap_or_else(|e| { | ||
panic!("error parsing bounds: {:?} ({})", lit, e) | ||
})); | ||
} | ||
|
||
return; | ||
} | ||
} | ||
|
||
panic!(concat!( | ||
"The #[zeroize(bound)] attribute expects a name-value syntax with a string literal value.", | ||
"E.g. #[zeroize(bound = \"T: MyTrait\")]." | ||
)) |
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.
This much error handling really makes me wish we were using proc-macro-error
, although I haven't figured out how to make it play nicely with synstructure
.
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.
I've tended to not use a lot of libraries when working with proc-macros exactly because of interoperability. I was close to suggesting to drop synstrucutre completely in favor of just using plain syn to skip parsing the user input and let rustc just say what's wrong with the where bounds instead of trying to convert them to WherePredicate
s.
I can look into it if you deem it necessary.
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.
Yeah, I recently dropped synstructure
from another proc macro crate, although zeroize_derive
is better leveraging it.
proc-macro-error
though I think is definitely worth it, since it allows emitting errors with diagnostic information that includes the original source spans, which makes error messages generated from proc macros substantially better.
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.
I did it without proc-macro-error by just using to_compile_error
until now. But our current strategy using panic isn't ideal, true enough.
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.
I'm assuming you meant elsewhere outside the scope of this PR?
With proc-macro-error
I've managed to make sure that practically every error case is tied to the input code that caused it.
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.
I'm assuming you meant elsewhere outside the scope of this PR?
Sorry, lack of context indeed 😄.
With
proc-macro-error
I've managed to make sure that practically every error case is tied to the input code that caused it.
That sounds great! I will take a look at it then.
Will go ahead and merge this, although if you'd like to look into removing |
Will do! |
À la
#[serde(bound = "T: MyTrait")]
.