cppcoro
cppcoro copied to clipboard
GCC 10.1 support
This pull requests adds GCC support on top of @mmha's CMake pull request.
I cherry picked his commit to the current master branch for this, I will rebase my pull request once that PR is merged.
I opened this for now only so my changes can be reviewed.
As I explained in my last commit message GCC 10.1 seems to have remaining bugs causing test failures. I found workarounds for some of them, but I had to completely remove one testcase for GCC (not supported syntax), and leave a test failure as is (missing destructor call). Otherwise the library seems to be working.
Great job! When can this be merged?
I've done similar CMake+GCC support for local experiments and implemented several things differently, not quite clean though.
Consider replacing for co_await with manual implementation because it hasn't been approved for C++20. Also, why cppcoroConfig.cmake
and FindCppcoroCoroutines.cmake
are needed there? IIRC, they should be generated outside of project directory where others can reach them. Here's my installation part:
include(GNUInstallDirs)
# install headers
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../include/cppcoro
TYPE INCLUDE
)
# install binary
install(TARGETS cppcoro_lib
EXPORT coro
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)
# install config for find_package()
install(EXPORT coro
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/cppcoro
NAMESPACE cppcoro::
FILE cppcoroConfig.cmake
)
# generate export file
export(EXPORT coro
NAMESPACE cppcoro::
FILE cppcoroExport.cmake
)
It's usually good to set default CMAKE_BUILD_TYPE to Release. People then can use canonical CMake installation mkdir build && cd build && cmake .. && cmake --build . && cmake --install .
Just want to say that I'm excited about this.
@OleksandrKvl the cmake files are from #110, I didn't change those. And I'm not sure if defaulting to release is a good idea, as that would effect multi-buildtype IDE setups also (like visual studio or xcode). If a projects wants to set a default build type correctly, it usually requires several conditions (don't do anything if using a multi config generator ; use debug if the .git directory is present ; use relwithdebinfo otherwise)
@lewissbaker I'll updat my PR based on your comments, but do you have plans about #110? As currently this PR includes that commit too.
I've added some comments to #110. Perhaps @OleksandrKvl 's comments above could be added to that PR and CMake-related discussion moved there?
@dutow Would it be possible to separate this PR out into just the code-changes needed for GCC support and not have it build upon the CMake changes, so that they can be merged independently?
@lewissbaker Then we don't have a build system I'm actually familiar with. Cake seems to be a custom tool written by you, and the cake script currently forces clang on linux/osx. Seems like adding gcc there would require more changes than the cmake script (adding a config to choose the compiler, then refactoring the entire linux/darwin section to handle both clang and gcc)
Also, then we'll have to add d99be58 to the cmake pullrequest instead.