-
Notifications
You must be signed in to change notification settings - Fork 127
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
Don't drop twice on panic. #4
Conversation
This is a fix for #3 but not necessarily the best fix. |
I'm working on a fix too :) Let's compare.. |
I wonder which of these fixes are classified as the most fragile. Thanks for submitting this PR by the way, made me understand this much better as a real panic safety bug. |
I think the sane solution is to go back to using the external NoDrop with its own destructor (like arrayvec pre 0.3.8). That works, but do you think that's an accident? |
That's what I did first. However, it makes the arrayvec one byte larger due to the drop flag... |
One byte is just for the Arrayvec of u8. It makes it a whole element larger due to padding / alignment. It's an ok tradeoff for now.. |
Also, that doesn't fix the IntoIter case (unless you put a NoDrop around the ArrayVec inside the IntoIter). |
See PR /pull/6, it covers IntoIter too with some tweaks |
It is probably the case that panic on drop rules are simply not well defined. I'd prefer this guard case if it wasn't for the weird bug, which makes it feel very hacky. |
No description provided.