-
Notifications
You must be signed in to change notification settings - Fork 129
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
zeroize: breaking change for bounds #878
Comments
If I would have to guess the change of logic in attr_skip/filter_skip is responsible, in particular in generate_fields I see a lot of logic change. |
cc @maurer |
My understanding of the behavior of the previous code was that if a type variable was present in the generics, a However, it must be suppressed elsewhere, since I just tested the original version and it doesn't inject the bound. I've sent up a PR that never injects the bound but it might be worth considering making the bound get injected at some point if the type parameter is used in an unskipped field, since that will just fail to compile without a manual bounds annotation at the moment. I've also added an explicit testcase for this issue to avoid making this error again. |
@maurer awesome, thank you! |
@ETKNeil can you confirm this was fixed by |
I confirm that 1.4.1 did fix it, thank you very much for reacting so quickly |
I am still seeing issues with v1.4.1 for the following code: #[derive(Zeroize)]
pub struct Key<T>(T);
This code is able to compile with |
cc @maurer |
The where clauses I was previously injecting *were* load bearing, but they needed to be filtered for skipped fields. Fixes RustCrypto#878
Sorry about that, the where clauses I pulled out for the previous user shouldn't have been pulled out all the way. I've added another test for this side of things and am automatically generating Zeroize bounds for used fields now. |
The where clauses I was previously injecting *were* load bearing, but they needed to be filtered for skipped fields. Fixes #878
Released changes from #882 as |
I ran into another breakage related to this. However, it seems that it was mistakenly not broken before? Here is an example:
This compiles with
It seems like it shouldn't really work before since I don't see how it could derive Zeroize for |
@conradoplg it should work if a |
Indeed that works, I'm just curious about how it compiled without the bound before |
My guess would be it used to infer the bound /cc @maurer |
Consider the following code
This code used to compile with:
But now by updating the derive
I get
Which is due to #858
I bisected and downgrading derive to 81536ee works perfectly with my example, upgrading to syn v2 broke something
The text was updated successfully, but these errors were encountered: