⚙️ move_only_function representation

_Move_only_function_data is a union with the following representation:

_Move_only_function_base then stores _Move_only_function_data _Data; and has:

❓ Questions

• Why does _Construct_with_null() do the extra work of _Data._Set_large_fn_ptr(nullptr);?
• What does the comment of // initialize, since we'll be copying it actually mean?

🕵️ Analysis

There are three calls to _Construct_with_null():

They're the default constructor and the constructors from nullptr_t and null callables (function pointers, PMFs, PMDs, and other move_only_functions).

What happens when we perform such a construction, then move the move_only_function? It has a defaulted move ctor, so we need to look at _Move_only_function_base:

_Checked_move() starts with const auto _Impl = _Get_impl(_Src);. That doesn't get the null previously set by _Data._Impl = nullptr;. It does something fancy:

Because _Data._Impl is null, we get &_Null_move_only_function instead, which has null function pointers for _Move and _Destroy:

Going back to _Checked_move(), because _Impl->_Move is null, we call _Function_move_large(_Data, _Src);. That is, when we move a default constructed (etc.) move_only_function, we treat it as "large" (dynamically allocated). What happens now?

This is a memcpy() of the first two pointers of _Move_only_function_data, i.e. the _Data._Impl that we previously set to null (and definitely had to), followed by the _Data._Pointers[1] that _Data._Set_large_fn_ptr(nullptr); previously assigned to (i.e. the assignment in question here):

💡 Conclusions

Because memcpy() can copy anything, including uninitialized bytes, @AlexGuteniev (the original author) and I now believe that _Construct_with_null() does not need to do the extra work of _Data._Set_large_fn_ptr(nullptr);.

The comment of // initialize, since we'll be copying it was written under the impression that if we left the _Large_fn_ptr (i.e. _Pointers[1]) uninitialized, that would spell trouble when a later move of the move_only_function attempted to copy that pointer's bits (followed by nulling out the source). But since we copy this pointer with memcpy(), leaving it uninitialized would have been fine.

When _Data._Impl is null, nothing else in the code should care about the _Large_fn_ptr (i.e. _Pointers[1]). We inspect _Data._Impl to determine whether the move_only_function is empty:

And if someone tries to call an empty move_only_function, we go through _Get_invoke() which will pick up the _Null_move_only_function's _Function_not_callable<_Rx, _Types...>:

This makes sense, because _Data._Impl is a pseudo-vtable, and needs to control what the move_only_function does with its representation. It switches between dealing with the representation in null mode, small mode, or large mode (and we've seen that null mode bypasses looking at _Large_fn_ptr). We can't possibly "punch through" and directly inspect _Large_fn_ptr without going through _Data._Impl, because that would be wrong if we were in small mode.

The only thing that manipulates _Large_fn_ptr in null mode is _Checked_move() as we've already seen - which unifies the null mode and large mode cases into a memcpy() of two function pointers.

🛠️ Potential change

I believe that we can remove _Construct_with_null() and replace its usage with _Reset_to_null(), avoiding the unnecessary work of nulling out the _Large_fn_ptr which we don't need to use. This would be strictly simpler, and remove the confusing line/comment that prompted my investigation (it made me worry that we actually did need to care about _Large_fn_ptr in null mode).

It could be possible to split how we handle null mode and large mode in _Function_move_large() (copying only one null pointer for null mode), but that would be a lot of logic change for a minimal improvement. (How often are we moving empty move_only_functions?) I recommend against exploring that path.

(I noticed this while implementing Destructor Tombstoning but I'm filing a followup issue since I don't want to randomize that PR with unnecessary logic changes.)