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

use feature test macro to check feature available #15

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

cauliyang
Copy link

Reason

thread-pool is a great modern and tiny thread pool. However, using the compiler version to check move_only_function is not solid. For example:

I used g++-12 (Ubuntu 12.1.0-2ubuntu1~22.04) 12.1.0. The library could not compile due to:

error: ‘move_only_function’ in namespace ‘std’ does not name a template type
25 | using default_function_type = std::move_only_function<void()>;
| ^~~~~~~~~~~~~~~~~~

Here is the ci for detailed information.

Hence, I tried to use feature testing instead and it works well.

I am sure that I may have missed something. :)

@DeveloperPaul123
Copy link
Owner

Hi 👋

Thank you for the PR! I didn't realize that the feature macros were standardized in C++20, thanks for the links!

Also it seems that there was a possibly in a bug in my GNU version detection. It seems that this approach should be better but I'd like to do more testing locally.

Also nice to see others are using my thread pool 😀

@cauliyang
Copy link
Author

Hi ✋ 

This modern C++ library is handy and powerful! I like it. We do need to conduct more testing to ensure the quality of the code.

I really appreciate your work, and I'm learning a lot!

@DeveloperPaul123
Copy link
Owner

PR looks great and works great, thanks again!

@DeveloperPaul123 DeveloperPaul123 merged commit dad69c1 into DeveloperPaul123:master Oct 5, 2022
@cauliyang
Copy link
Author

Great! 🚀 I hope this library will be best! Many thanks for your hard work!

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.

None yet

2 participants