3 ms·
From the article: > That means that impl Drop for Drain is responsible for making sure that Drain is sound. This is _wrong_ the Drop impl _must not_ be respon
by dathinab 2y ago
From the article:
> That means that impl Drop for Drain is responsible for making sure that Drain is sound.
This is _wrong_ the Drop impl _must not_ be responsible for making sure Drain is Sound. Drop is not guaranteed to run it must _never_ be relied on for soundness.
I emphasizes the wrong because that is one of the biggest pitfalls in current rust outside of doing some very clever/magic things.
In std the `drain` function guarantees soundness by setting len=0, i.e. making the vector forget about all it's content, i.e. semantically moves all of it's elements from the Vec into the Drain.
The Drop impl. semantically just moves the parts it didn't drain back into the Vec.
I.e. for Drain Drop it's isn't responsible for soundness but for not leaking the non drained elements.
EDIT: To be fair: Otherwise still a good article. Just misses one crucial paragraph.
- selfmodruntime 2y agoAny link to that magic? Sounds interesting.
- dathinab 2y agoThe code the blog presents copied from the std, ironically it does include the line where the len is set to 0 with a in code comment (but the article fails to go into it and seems to not be quite aware of the issue, it's otherwise quite good). Also: - up to date explanation in the nomicon: https://doc.rust-lang.org/nightly/nomicon/leaking.html https://doc.rust-lang.org/nightly/nomicon/leaking.html - from 2015 when it was realized leak must be sound: https://faultlore.com/blah/everyone-poops/ https://faultlore.com/blah/everyone-poops/
- deleted 2y ago[deleted]
- chriskrycho 2y agoAh, you’re totally right; I badly misstated that. I’ll update it tomorrow accordingly. Thank you for the correction. Your last comment before the edit is one part of what I was trying to get at; the other was that Drop itself must uphold the relevant constraints in its own operation. Like I said, great clarification/ correction and I’ll be glad to fix it!
- chriskrycho 2y agoI just published an update to the post which corrected the title, URL, and content, and also published an additional post calling attention to the error and explaining it (https://v5.chriskrycho.com/notes/corrections-using-drop-safely-not-for-safety/ https://v5.chriskrycho.com/notes/corrections-using-drop-safe...). Thanks again for flagging it up, @dathinab!