Skip to content
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

Uncopyable work queue #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

syntheticmagus
Copy link
Contributor

Modified dispatcher.h to use uncopyable inplace_functions. This required some significant modification to the uncopyable features added to inplace_function previously, as that implementation had some serious flaws. In particular, while the implementation prevented copying and prevented the copy constructor from existing, it didn't actually delete the copy constructor. This meant that code which was simply looking to see whether uncopyable inplace_functions had a copy constructor -- notably std::is_copy_constructible -- to see that the copy constructor still existed and to conclude, therefore, that the type was copyable. This caused problems when it led MSVC's implementation of certain STL containers to elect to copy rather than move, only to fail when the copy constructor turned out to be unusable.

To solve this problem, the copy constructor needed to be deleted, not merely unusable; and to do that, the uncopyable version of the inplace_function needed to be a specialization of the entire type, not just of specific functions. Thus, inplace_function.h now contains two inplace_function specializations, one copyable and one not, with the shared behavior factored out as much as possible into a separate impl. With this change, std::is_copy_constructible is able to correctly determine the copyability of inplace_functions, leading MSVC's STL to make the right decisions and allowing us to use uncopyable inplace_functions in dispatcher.h.

@syntheticmagus syntheticmagus marked this pull request as draft March 15, 2022 00:55
@syntheticmagus syntheticmagus marked this pull request as ready for review March 28, 2022 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant