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

Modernize mutex #802

Merged
merged 5 commits into from
Aug 3, 2020
Merged

Conversation

kdt3rd
Copy link
Contributor

@kdt3rd kdt3rd commented Aug 1, 2020

This patch removes the custom implementations of mutex in favor of the c++11 provided thread utility provided version. It retains the header file with deprecation warnings, but otherwise removes use of this from the code

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Also deprecates Lock

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
@@ -146,7 +70,7 @@ class ILMTHREAD_EXPORT Lock
}
}

~Lock ()
inline ~Lock ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't class methods with implementations right in the class declaration implicitly inline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I was originally trying to remove this entirely, then realized I couldn't and didn't rollback my attempt to make the functions declared later. Will kill those inline

@lgritz
Copy link
Contributor

lgritz commented Aug 1, 2020

LGTM, pending whatever fixes make the CI pass.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as Larry vis a vis inline. Sure is nice to see all that platform code evaporate! Thanks!

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
@kdt3rd kdt3rd merged commit 22c3582 into AcademySoftwareFoundation:RC-3 Aug 3, 2020
@kdt3rd
Copy link
Contributor Author

kdt3rd commented Aug 3, 2020

fyi, CI still failing pending the imath external project changes, merging this in ahead of that

@kdt3rd kdt3rd deleted the modernize_mutex branch February 12, 2022 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants