-
Notifications
You must be signed in to change notification settings - Fork 472
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
Added CMake build system #110
base: master
Are you sure you want to change the base?
Conversation
|
||
add_library(cppcoro::coroutines INTERFACE IMPORTED) | ||
if(Coroutines_SUPPORTS_MS_FLAG) | ||
target_compile_options(cppcoro::coroutines INTERFACE /await) |
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.
Under msvc optimised x64 builds we also want to add the "/await:heapelide" flag to take advantage of the coroutine frame heap-elision optimisations.
include(FindPackageHandleStandardArgs) | ||
|
||
check_cxx_compiler_flag(/await Coroutines_SUPPORTS_MS_FLAG) | ||
check_cxx_compiler_flag(-fcoroutines-ts Coroutines_SUPPORTS_GNU_FLAG) |
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.
I think that since the adoption of coroutines into C++20 that clang (trunk?) now also enables coroutines when -std=c++2a
is enabled.
Thanks for the PR and especially for adding docs! I've added a few minor notes. |
I tried build with LLVM
|
@denchat This seems like an issue with mixing clang-cl and the MSVC standard library headers rather than with cppcoro. Has it been reported to Microsoft? |
@lewissbaker After more googling around, I found that the problematic line of c++ prohibited non-definition anonymous struct in
uses the Microsoft non-standard extension, which is turned on by default by Compiler Warning (level 4) C4201 - nonstandard extension used : nameless struct/union Unfortunately, rightnow it seems
Using
|
Any chance this could be updated and merged? Once there is CMake support, it should be trivial to publish the library for vcpkg (#112). |
Any updates on this? @lewissbaker are you interested in providing a way to build and use your library with "default" build-system for C++ now instead of your hand-written stuff? I cannot use your library only because of custom build system. P.S. Thank you a lot for the library! |
Same here for us @zamazan4ik, I use another (old fork) with CMake support for now. But we build for many platforms that this this build system do not support. |
Hi. I forgot to mention microsoft/vcpkg#10693 here. I wish you can try with the |
Is there a chance of this getting merged in the near future? I've got the library working with GCC 10.1 based on the current master branch and this commit (using cmake, I didn't try to understand how the cake script works), I would open a pull request once this gets merged. |
This looks good. @mmha Remove the draft status so @lewissbaker can merge it. |
@mmha has never responded after the initial commit here. How can we proceed with this pull request? I'd love to see it merged. |
Are people generally happy that this PR is good to go? There were a few comments that hadn't been responded to, yet.
Is there a more general CMake solution to turning on coroutines support than The other thing that would be good to add soon is CI support, so that we don't regress on the build side of things. |
Hi, thanks for having a look at this pull request. Concerning the CI support, I started creating some github actions in https://github.com/andreasbuhr/cppcoro/tree/add_github_actions . Have a look at https://github.com/andreasbuhr/cppcoro/tree/master, README.md, first section. I wrote a little about my initiative to help cppcoro maintenance. If you have any questions, don't hesitate to reach out to me. |
@lewissbaker The libunifex script is longer and more complex, but I don't think it accomplishes more? It certainly isn't more generic, it uses hand specified flags, same as this script. The only addition is that it includes a test C++ source to verify that coroutine compilation works, and it tries to distinguish between final and experimental coroutine support. For the clang-cl issue, that's not related to this PR. That's one of the possible errors you can get if you use a new version of VS and an old version of clang. (For this cmake pr: I have ideas on how to improve it, but at least it works, and I can't edit this pull request. Once it's merged, I plan to open another with some improvements) |
RFC:
Should the CMake package check for coroutine support when being invoked by
find_package()
?TODO: