Added CMake build system by mmha · Pull Request #110 · lewissbaker/cppcoro

@mmha

  • Added a CMake build scripts + packaging
  • Updated to README

RFC:
Should the CMake package check for coroutine support when being invoked by find_package()?

TODO:

  • Update .travis.yml
alexv-ds, santisan, MattEding, zamazan4ik, egor-spk, JohelEGP, fvannee, etorth, and Tadaboody reacted with thumbs up emoji alexv-ds and etorth reacted with hooray emoji denchat, luncliff, alexv-ds, etorth, and Toxe reacted with heart emoji

@mmha

lewissbaker


add_library(cppcoro::coroutines INTERFACE IMPORTED)
if(Coroutines_SUPPORTS_MS_FLAG)
target_compile_options(cppcoro::coroutines INTERFACE /await)

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.

lewissbaker

include(FindPackageHandleStandardArgs)

check_cxx_compiler_flag(/await Coroutines_SUPPORTS_MS_FLAG)
check_cxx_compiler_flag(-fcoroutines-ts Coroutines_SUPPORTS_GNU_FLAG)

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.

@lewissbaker

Thanks for the PR and especially for adding docs!

I've added a few minor notes.
I won't be able to comment much on the CMake style/structure, though, as I'm a CMake newbie.

@denchat

I tried build with LLVM clang-cl. It doesn't compile cppcoro/file.hpp which include experimental/filesystem due to anonymous struct declaration struct char8_t;

[19/69] Building CXX object lib\CMakeFiles\cppcoro.dir\read_only_file.cpp.obj
FAILED: lib/CMakeFiles/cppcoro.dir/read_only_file.cpp.obj
C:\PROGRA~1\LLVM\bin\clang-cl.exe  /nologo -TP -Dcppcoro_EXPORTS -IC:\Users\User\AppData\Roaming\cppcoro-master\include -Xclang -std=c++2a -Xclang -fno-char8_t -Wno-gnu-anonymous-struct -fms-compatibility-version=19.20.27508 --target=x86_64-pc-windows-msvc19.20.27508 /EHsc /Zc:__cplusplus /MD /O2 /Ob2 /DNDEBUG   /await -std:c++latest /showIncludes /Folib\CMakeFiles\cppcoro.dir\read_only_file.cpp.obj /Fdlib\CMakeFiles\cppcoro.dir\ -c C:\Users\User\AppData\Roaming\cppcoro-master\lib\read_only_file.cpp
clang-cl: warning: argument unused during compilation: '/await' [-Wunused-command-line-argument]
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\lib\read_only_file.cpp:6:
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\include\cppcoro\read_only_file.hpp:8:
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\include\cppcoro/readable_file.hpp:8:
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\include\cppcoro/file.hpp:18:
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.20.27508\include\experimental/filesystem(45,13): error: declaration of anonymous struct must be a definition
            struct char8_t; // flag for UTF8
            ^
1 error generated.

@lewissbaker

I tried build with LLVM clang-cl. It doesn't compile cppcoro/file.hpp which include experimental/filesystem due to anonymous struct declaration struct char8_t;

@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?
Would the same issue occur if cppcoro were to use <filesystem> instead of <experimental/filesystem>?

@denchat

@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?
Would the same issue occur if cppcoro were to use <filesystem> instead of <experimental/filesystem>?

@lewissbaker
I looked at <filesystem>. It is a very short header and it includes <experimental/filesystem>. It seems all microsoft implementation defined in <experimental/filesystem> as of VC++ 2019.


After more googling around, I found that the problematic line of c++ prohibited non-definition anonymous struct in <experimental/filesystem> of msvc standard library:

uses the Microsoft non-standard extension, which is turned on by default by cl.exe and can be explitcitly activated by /Ze

Compiler Warning (level 4) C4201 - nonstandard extension used : nameless struct/union
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4201?view=vs-2019

Unfortunately, rightnow it seems clang-cl.exe don't let us use /Ze and it seems to ignore the flag.

C:\Users\User\AppData\Roaming\cppcoro>ninja -j 2
[19/69] Building CXX object lib\CMakeFiles\cppcoro.dir\writable_file.cpp.obj
FAILED: lib/CMakeFiles/cppcoro.dir/writable_file.cpp.obj
C:\PROGRA~1\LLVM\bin\clang-cl.exe  /nologo -TP -Dcppcoro_EXPORTS -IC:\Users\User\AppData\Roaming\cppcoro-master\include -Xclang -std=c++2a -Xclang -Wno-gnu-anonymous-struct -Wno-unused-command-line-argument -fms-compatibility-version=19.20.27508 --target=x86_64-pc-windows-msvc19.20.27508 /EHsc /Zc:__cplusplus /Ze /MD /O2 /Ob2 /DNDEBUG   /await -std:c++latest /showIncludes /Folib\CMakeFiles\cppcoro.dir\writable_file.cpp.obj /Fdlib\CMakeFiles\cppcoro.dir\ -c C:\Users\User\AppData\Roaming\cppcoro-master\lib\writable_file.cpp
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\lib\writable_file.cpp:6:
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\include\cppcoro/writable_file.hpp:8:
In file included from C:\Users\User\AppData\Roaming\cppcoro-master\include\cppcoro/file.hpp:18:
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.20.27508\include\experimental/filesystem(45,13): error: declaration of anonymous struct must be a definition
            struct char8_t; // flag for UTF8
            ^
1 error generated.

Using clang-cl.exe is still a road-block, at least when we are including <cppcoro/file.hpp>, if

  • MSVC <filesystem> and <experimental/filesystem> still use anonymous struct and the compiler extension.
  • clang-cl.exe won't compile anonymous struct despite /Ze explicitly stated by users.

@chausner

Any chance this could be updated and merged? Once there is CMake support, it should be trivial to publish the library for vcpkg (#112).

@zamazan4ik

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!

@digimatic

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.
CMake is de-facto standard in C++ now..

@luncliff

Hi. I forgot to mention microsoft/vcpkg#10693 here. I wish you can try with the find_package with it. If you meet some port bug, please let me know.

@dutow

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.

@JohelEGP

@dutow dutow mentioned this pull request

Sep 9, 2020

@andreasbuhr

@mmha has never responded after the initial commit here. How can we proceed with this pull request? I'd love to see it merged.

@lewissbaker

Are people generally happy that this PR is good to go?

There were a few comments that hadn't been responded to, yet.

  • Note about /await:heapelide flag for MSVC Release
  • Issue with char8_t on clang-cl (I assume that's a general issue and not specific to cppcoro, though)

Is there a more general CMake solution to turning on coroutines support than
eg. maybe something like in libunifex CMake?
https://github.com/facebookexperimental/libunifex/blob/master/cmake/FindCoroutines.cmake

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.

@andreasbuhr

Hi, thanks for having a look at this pull request.
Garcia6l20 proposed some changes to this pull request in andreasbuhr#1 . I will have a look at them during the weekend and can then create a new pull request here, with Garcia6l20's suggestions included. I will also have a look at the await:heapelide issue.

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.

@dutow

@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)