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

reject #[zeroize(drop)] on struct/enum fields, enum variants #847

Merged

Conversation

jessa0
Copy link
Contributor

@jessa0 jessa0 commented Aug 26, 2021

Reject the #[zeroize(drop)] attribute on struct and enum fields, as users may be missing a zeroizing Drop implementation
they expected to be generated for them.

For consistency, the attribute is now rejected on enum variants as well, which was previously accepted and generated a
Drop impl for the entire enum, but which may not be what users may have expected, as it could be interpreted that such
an attribute would zeroize only the variants containing the attribute on Drop.

Closes #837.

Reject the #[zeroize(drop)] attribute on struct and enum fields, as users may be missing a zeroizing Drop implementation
they expected to be generated for them.

For consistency, the attribute is now rejected on enum variants as well, which was previously accepted and generated a
Drop impl for the entire enum, but which may not be what users may have expected, as it could be interpreted that such
an attribute would zeroize only the variants containing the attribute on Drop.
@jessa0
Copy link
Contributor Author

jessa0 commented Aug 26, 2021

hmm right, ok. I'm processing the #[zeroize(drop)] attribute twice for structs. i guess the existing unit tests didn't test the parsing at all, so i guess i'll also add a success test case here.

@codecov-commenter
Copy link

Codecov Report

Merging #847 (52f3c65) into main (6d9b224) will increase coverage by 1.14%.
The diff coverage is 85.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
+ Coverage   52.75%   53.90%   +1.14%     
==========================================
  Files          82       82              
  Lines        2942     2985      +43     
==========================================
+ Hits         1552     1609      +57     
+ Misses       1390     1376      -14     
Impacted Files Coverage Δ
zeroize/derive/src/lib.rs 83.52% <85.41%> (+50.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d9b224...52f3c65. Read the comment docs.

@jessa0 jessa0 force-pushed the reject-zeroize-attribute-on-fields branch from 52f3c65 to fa56971 Compare August 26, 2021 16:41
@jessa0
Copy link
Contributor Author

jessa0 commented Aug 26, 2021

ugh ok, ran cargo fmt on the latest commit and force-pushed

@tony-iqlusion
Copy link
Member

Looks good. Thank you!

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.

zeroize attribute accepted on struct fields, to no effect
3 participants