-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Prepare move_only_function machinery for future C++26 interactions
#5849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+96
−84
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Does not match clang-format exactly, but still closer
StephanTLavavej
approved these changes
Nov 18, 2025
Member
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Member
🏚️ 🏠 🏙️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards #5504. Towards #3803.
Will help sharing the implementation with
copyable_functionimplementation sharing and interaction withfunction_ref.Also a minor enhancement on its own.
Steps
Rename to drop
_Move_onlypartIt would be silly to derive
copyable_functionfrom_Move_only_function_call.Ditto for
packaged_taskfrom_Move_only_function_baseandfunctionfrom_Move_only_function_call. in vNext.Avoid saying fields for the data member
This is not directly related to future integrations.
Just prioritizing proper C++ terminology over OOP terminology.
Recognition that OOP is far from being universal, and call wrappers are not necessarily implemented with OOP.
Using void object pointer
This is helpful for
function_ref<type>{move_only_function<type>{...}}andfunction_ref<type>{copyable_function<type>{...}}call unwrapping. Specifically,bound-entityexposition-only member can be of void object pointer type, then the type of_Invokematches the type ofthunk-ptrinfunction_ref, so we can easily do call unwrapping with thunk elimination.@frederick-vs-ja has pointed out in #5504 (comment) that the unwrapping involving
function_refis LWG-4264, and the unwrapping with outerfunction_refhas unexpected effects. I've clarified this on email with Tomasz Kaminski. The idea is that in the case of:will not track the changes of
f1value in case of double indirection avoidance, so looks unacceptable, but for this:not tracking the changes of
f1may be acceptable, as this imitates a copy offunction_ref,For the reverse unwrapping, that is
move_only_function<type>{function_ref<type>{...}}orcopyable_function<type>{function_ref<type>{...}}, this change is not enough. I think there are no good options to do the call unwrapping with thunk elimination, will discuss that below. This reverse unwrapping is not problematic semantically though, as the heavy wrappers are supposed to copy/move, and hopefully LWG-4264 will enable it.Use void object pointer to a non-constant
We have to pass both constant and non-constant objects through the same thunk. For the constant ones the
operator()is const. Due to having small functions, we cannot utilize shallow-but-not-deep constant, and have to putconst_castsomewhere. This is not a problem for const correctness, as all calls are checked withis_invokable...family, and we don't do anything besides the forwarding and the invoke in the call operator.Prior to this change, the object was passed as constant, the
const_castwas on the thunk side.The change makes the passed object pointer non-constant, and moves the
const_castto theoperator()side.This makes
bound-entitysimplyvoid*in case the above part is kept.Quasi-ABI-break
We can break ABI, but we actually don't do this here if you think of it:
Still I want to merge this before ABI changes closure to be extra safe
What's up with unwrapping of inner
function_refThis is about
move_only_function<type>{function_ref<type>{...}}orcopyable_function<type>{function_ref<type>{...}}unwrapping with thunk elimination.TL;DR: The win is small and there's no way to make it without penalizing more common use cases.
Whereas the current
move_only_functionthunks adjust pointer to the beginning of the object internally to find the real callable, the straightforwardfunction_refimplementation would need to pass the direct pointer to a callable, because there's nothing much offunction_ref.The extra thunk not only means extra code, but also extra pointer saved inside
move_only_functionholdingfunction_ref. Still this looks like the best options.There are ways to unify
move_only_functionandfunction_refthunk by:move_only_functionas well. Will make smallmove_only_functionthunk the same as large, and make small only function self-referencing, that is containing a pointer inside its own data. Will spend extra pointer. Will cause the revert of avoid allocations part from Unwrap call offunctionpassed tomove_only_function#5808, as that will not fit even in x64move_only_functioncall. Penalizes the code size and run time of normalmove_only_functionusagefunction_ref. Penalizes the code size and run time of normalfunction_refusageUltimately, the thunk elimination is not very big win, comparing to extra invoke elimination. And we can eliminate extra invoke even without unifying thunks.