I was reading a topic on main forum, when I realised that neither Drain types in std nor drain functions that return them are marked as #[must_use]. This makes following code compile without warnings:
fn main() {
let mut s = "hi there".to_owned();
s.drain(2..6);
println!("{s}");
}
I'm quite surprised by this. It is of course perfectly fine to drop Drain type. Unless it is forgotten there is no memory leak, so the only consequence of dropping it is that drained items are dropped as well.
My worry comes from the teaching/discoverability perspective. I think that "drain API" is unique to Rust. There are languages with similar constructs, but they mostly handle a specific case (like only arrays, or not being lazy), in contrast to generalised API in Rust. Being general and flexible is very nice for users who know standard library well, but differences in behaviour caused by subtle code changes might be surprising to new users.
Rust took much care into building a reputation of a language that explains to its users how to fix incorrect code and proactively steers them away from potential mistakes. I understand that being forced to explicitly mark that returned types are dropped might be annoying for some people (although I know that there are many who support this style of programming), but right now we are missing an opportunity to teach new users about drain API in self-explaining way. Imagine that the code snippet would compile with a unused_must_use lint warning with following notes:
note: "drained items will be dropped when returned Drain type is dropped"
note: "consider iterating over them, or explicitly mark dropping with let _ = ...".
I think that this could nudge new users into reading docs of drain functions more carefully, which would help them discover how drain API works. It would also force more experienced users to explicitly mark dropping of Drain, which might be a good thing, or a bad thing, depending how you look at this.
Isn't this request backwards? The issue in the linked thread seems to be that someone didn't realise that it's useful to create a Drain and immediately drop it. Marking it #[must_use] would only help if people thought that it was useful but it wasn't, but that was the opposite of the actual misconception.
I agree that there's potentially an issue with discoverability here, but the proposed solution doesn't seem to solve it.
I still don't agree – what situation would trigger the lint to come up? If it's anything involving drain, it doesn't solve the problem because the problem is that the user wanted to delete a range and didn't realise that drain would help, so they would have no reason to try it.
The lint should, instead, possibly suggest using drain instead if someone tries to replace a range of a String/Vec with an empty structure (assuming that drain is the best way to do this). But that would have substantially different wording.
Another possibility would be to add delete_range methods, which would make the operation more discoverable, although I'm not sure whether that would be worth it.
That would make sense, but if so it would make sense to have some functions opt out of it. I remember this has been discussed before here on IRLO and some of us tried it.
My memory is that there were some common annoying functions that you might want to ignore most times. Such as if an insertion in a HashMap threw out an old value or not.
Perhaps similar to how there has been a push to tag many things with #[must_use], there could be a top level attribute for #[allow_unused]. Or since I would prefer #![warn(unused_results)] to continue being absolute, a new pair of an attribute #[may_use] and a #![warn(unused_except_may_use_🚲)]. Then libraries (especially std) could be annotated and there would be a migration path towards a bright future where unused_except_may_use_🚲 is enabled by default.
From memory I think the gripe here is that many APIs would have been designed differently in light of a #[must_use] default. HashMap specifically would probably have a pair of insert_overwrite and insert_and_give_me_the_old_value (update?) methods. At present insert is covering both cases. Still not an argument against attempting to migrate towards that default.