BTreeMap: revert a part of #78015 and test what it broke

[ssomers]

Just after waiting for #78015 to make it through, I figured out it can cause a leak if a drop panics, and I don't know how to fix it, other than to revert that part.

r? @Mark-Simulacrum

[ssomers]

This has nothing to do with the tree's MaybeUninits. After #78015, append drops kv pairs in b_next.or(a_next). Before (and again after this PR), it drops pairs in the first half of self.left.next(); self.right.next(). I would think the general ownership rules clean up evenly but apparently b_next leaks if the dropped argument panics. This conclusion may be somewhat presidential.

I'm more sure that this isn't a problem for the use of MergeIterInner in BTreeSet, because the items are always references and I think these don't do anything while dropping, let alone panic. And if it is, I bet #78015 didn't change that.

[Mark-Simulacrum]

Can you clarify exactly what leaks and where? I'm not sure I'm quite following -- it might be a bug with drop elaboration or something like that if I am, in which case we shouldn't fix it here.

[ssomers]

The symptom I ran into is that in the new test case, without rolling back the use of MergeIterInner, only 4 of the 5 elements are dropped.

It baffles me. I'm about to fire a question about playground, When a panic occurs dropping an element of a container, Rust still tries to drop the other elements. Same for a tuple, as shown in the example. But apparently not for function arguments?

[tmiasko]

The playground code looks like an instance of bug #47949.

[ssomers]

@tmiasko Thanks, yes indeed, that seems to be the culprit. It's the return value that is forgotten, not the arguments, as more purposefully demonstrated in this example.

I'll keep the test case for a future PR then.