FWIW, I feel pretty strongly that a comment is a much better solution than ManuallyDrop for this use-case. I would have leaned that way even without unsafe (Drop impl is significantly more code, and you could get it wrong). But unsafe, however trivial, is just a mountain of a cost for cases which don‘t need need unsafe otherwise.
Another idea is to add metadata attributes, which are ignored by the compiler, but can be parsed by IDEs and other tools. For example, adding #[meta(drop_before = "thread")] to a field tells the IDE that it should preserve the field order when refactoring. It would also be useful for other things, like syntax highlighting of other languages inside of Rust strings.
1 Like
Often times this can be done with just specially formatted comments. Obviously structured attributes are more structured, but IDE support for them would not be particularly more widespread than conventional comments.
What kind of automatic refactoring would break this? Shouldn't the IDE be aware of semantically-significant stuff like drop order? I would expect it to avoid implicit changes in behavior.
If the comment says "this field must be dropped after bar", and then you use automated refactor to rename bar to baz, the IDE won't necessarily update the comment. A decent IDE should try, but none of the existing Rust IDEs can do that yet, and that's a heuristic that works only most of the times.
5 Likes
OK, yes, refactoring is a source of comment bitrot. I thought you meant that the IDE might actually change drop order.
Sure, attributes are more explicit than using ManuallyDrop. (Not sure why one would have comments in the struct declaration that duplicate the drop order information though, that seems just redundant.)
What I am not convinced of is that this passes the bar to justify a new language feature. Are the benefits bigger than the cost (mostly in terms of implementation, specification and learning complexity just from the increased language surface)?
I think there's a misunderstanding on someone's side here.
Assuming that this is a reply to this comment, my point is that today, without attributes, I find "just order the fields and add a comment" a better solution for drop order than wrapping some fields into ManuallyDrop and implementing Drop.
That is, I disagree that documentation endorses ManuallyDrop as a way to control drop order. I personally would write this as
struct FruitBox {
melon: Melon,
// XXX: the drop order of fields below is significant.
peach: Peach,
banana: Banana,
}
1 Like
Hm, doesn't that example from the docs also have a bug that, if peach dtor panics, we won't even attempt dropping banana?
Oh, I see. I misunderstood then.
The text below the code discusses this.
Oh, indeed, it also mentions additional pinned gotcha...
Do you think it would be a good idea to send a PR removing the example and adding something along the lines of "while one could use ManuallyDrop to control drop order of fields, it is preferable to rely on standard drop order instead, as it might be tricky to implent Drop correctly in the presense of panics"?
That's not a judgment call I feel ready to make. I don't write such code often enough to say which version is actually preferable. Also the text should be more concrete about why this is "tricky".
I'd bring up the question with t-libs, maybe in Zulip.
1 Like
Submitted https://github.com/rust-lang/rust/pull/76150 for targeted discussion about best-practices here.
2 Likes
This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.