Fix drop order of JoinedCell fields

[steffahn]

Closes #20

[Voultapher]

Let me know if you agree with 2ca0660

[Voultapher]

Also do you think switching to ManuallyDrop would be worth it? We have pointers anyway, so the use seems limited to me. And I suspect this simple pointer based code is quite fast to compile.

[steffahn]

2ca0660 Seems like a valid approach as well.

I’m not sure if it’d be superflous, but I’d feel even better if the code used addr_of_mut!((*ptr).field) instead of the &mut (*ptr).field. I’m just being paranoid that &mut (*ptr).field might require that the whole *ptr is supposed to be valid, even though a different field is already dropped. (Probably not, but I’m not sure and the change wouldn’t hurt anyway I guess?)

[steffahn]

Note that even without 2ca0660, drop order of fields is clearly specified.

But with the addr_of_mut, I like it even better than the previous drop_in_place(joined_ptr.as_ptr());, because in my mind that one could also try to get mutable access to the whole *joined_ptr.as_ptr() first, which would invalidate the shared reference to owner that dependent might contain.

In other words: I’m not quite sure if it makes a difference but if any version is sound then the current one with drop_in_place(addr_of_mut!((*joined_ptr.as_ptr()).dependent)); drop_in_place(addr_of_mut!((*joined_ptr.as_ptr()).owner)); should be sound, too.

[steffahn]

I, too, don’t really think using ManuallyDrop adds anything useful.

[steffahn]

Squashed the commits a bit.

[steffahn]

Added another commit with more changes. I’m more confident that these operations are sound with addr_of_mut than before. In particular, the UnsafeSelfCell::borrow_mut seemed problematic because it returned a mutable reference to the whole JoinedCell even though owner must not be borrowed mutably while dependent can hold a reference to it.

[steffahn]

Another usage of addr_of_mut added. I’m not sure if &mut (*self.joined_ptr.as_ptr()).owner is allowed when the dependent field contains uninitialized data. Better safe than sorry.

[steffahn]

Did some more reading, turns out the addr_of[_mut] usage is unnecessary in most cases, but 2 things were legitimate AFAICT:

• the UnsafeSelfCell::borrow_mut was problematic, creating an intermediate mutable reference to the whole JoinedCell
• the

  let owner_ptr: *mut $Owner = core::ptr::addr_of_mut!((*joined_ptr.as_ptr()).owner);
  let dependent_ptr: *mut $Dependent = core::ptr::addr_of_mut!((*joined_ptr.as_ptr()).dependent);

  are important, because previously they would create intermediate references to uninitialized memory.

[steffahn]

Reduced usage of addr_of[_mut] to those cases now :-), squashed again

[Voultapher]

Thanks, lgtm.