Fix Vec::map_in_place not doing what is written in the comment

[tbu-]

No description provided.

[rust-highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[alexcrichton]

Can a test be added for this as well?

[tbu-]

@alexcrichton I'm not sure how to do this for the zero-sized types.

When I add a destructor to a zero-sized it gets a drop flag and isn't zero-sized anymore.

[Gankra]

So basically this is dead code until we remove the drop flag? You could still add a test that should "still" work once the drop flag is gone.

[alexcrichton]

Would #[unsafe_no_drop_flag] work here?

[bstrie]

Is there a timeframe for removing drop flags?

[tbu-]

@alexcrichton I tried this, but my conclusion is that you can't test for it right now.

1. Suppose you make a normal struct with no members, and implement Drop for it -> not zero-sized anymore.
2. Suppose you add #[unsafe_no_drop_flag] to the struct with no members. Then mem::forget doesn't stop the destructor from running (I think by design of #[unsafe_no_drop_flag]), so it actually yields incorrect behavior in Vec::map_in_place.

For this to work, we must have stack-based drop flags.

[pnkfelix]

cc me.

[alexcrichton]

Hm ok, looks ok to me then!

[Gankra]

I still think it would be reasonable to add a test that is effectively "dead code" now, but will stop being dead when the drop-flags become stack based. That is, code that makes sure destructors aren't called here on zero-sized types. This is trivially true now because such an object doesn't actually exist, but they will, and we might as well write the test now while we remember.

[tbu-]

@gankro How would you want to do that? If you can tell me a way, I'll implement it.

[Gankra]

@tbu- I imagine something like:

• define two zero-sized types A, B
• define two statics a_deaths, b_deaths
• make the destructors of A and B increment those respectively
• make a Vec<A>, assert a_deaths and b_deaths are 0
• map_inplace to Vec<B>, assert a_deaths has increased to the right value, and b_deaths hasn't
• drop Vec<B>, assert b_deaths has increased

This should trivially be true before and after this PR, and non-trivially true when we get stack-flags