-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Remove _For_each_tuple_element in scoped_lock #760
Conversation
scoped_lock::~scoped_lock uses _For_each_tuple_element which is not needed in C++17 because fold expressions can be used. This PR removes _For_each_tuple_element from tuple and changes the dtor in scoped_lock to use fold expression with std::apply. To minimize the changes in test (VSO_0000000_instantiate_containers) using _For_each_tuple_element, we copied this function to the test.
Address PR comments by adding _STD to scoped_lock::~scoped_lock() and make the functions in VSO_0000000_instantiate_containers/test.cpp not _Ugly.
Changing as per suggestion on PR review. Co-Authored-By: Casey Carter <cartec69@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I found a few typos and other stylistic nitpicks; I'll just push changes to fix them, then I'll bundle this up with some other PRs that I'm going to merge into the MSVC-internal repo today. Thanks for cleaning this up!
Thanks for this compiler throughput improvement! We've got unrelated test failures caused by other PRs colliding, so I went ahead and merged your PR with administrator powers. |
scoped_lock::~scoped_lock uses _For_each_tuple_element which is not
needed in C++17 because fold expressions can be used. This PR removes
_For_each_tuple_element from tuple and changes the dtor in scoped_lock
to use fold expression with std::apply.
To minimize the changes in test (VSO_0000000_instantiate_containers)
using _For_each_tuple_element, we copied this function to the test.
Closes #738