Implement initial version of C++20 module boost.any
#30
Conversation
|
@anarthal I'd appreciate comments on this PR on modules (as you proposed at boostorg/core#191 (comment)) Note that this PR should also work well with modular |
CMakeLists.txt
Outdated
| add_library(boost_any) | ||
| target_sources(boost_any PUBLIC | ||
| FILE_SET modules_public TYPE CXX_MODULES FILES | ||
| ${CMAKE_CURRENT_LIST_DIR}/modules/any.cppm |
modules/any.cppm
Outdated
| # pragma clang diagnostic ignored "-Winclude-angled-in-module-purview" | ||
| #endif | ||
|
|
||
| #include <boost/any.hpp> |
There was a problem hiding this comment.
I think (please check it) that, in module units, imports must be the first thing after the export module declaration (as opposed to other cpp files, where imports can be anywhere). This means that you might need to add another ifdef in your boost/type_index.hpp files, so that you can perform the import directly here. Note that this does not apply for tests, because they are not module units.
| # any_test.cpp # Ambiguous with modules, because all the anys now available | ||
| ) | ||
|
|
||
| foreach (testsourcefile ${RUN_TESTS_SOURCES}) |
There was a problem hiding this comment.
I think this belongs to test/CMakeLists.txt, not to test/cmake_subdir_test/CMakeLists.txt. The current convention is that cmake_subdir_test tests how a user would consume your library by calling add_subdirectory to your project. It should contain a simple usage example, not the entire test suite.
There was a problem hiding this comment.
I tried to to that at first, but failed to make it work.
mkdir build_module
cd build_module
cmake ../test # fails to find dependencies
What is the recommended way to run such tests in CI?
There was a problem hiding this comment.
The usual workflow is
cd $BOOST_ROOT
mkdir __build
cd __build
cmake -DBOOST_INCLUDE_LIBRARIES=any -DBUILD_TESTING=ON .. # add whatever flag you need here
cmake --build . --target tests
ctest --output-on-failure --no-tests=error
So you configure and build the Boost superproject, and select your library with BOOST_INCLUDE_LIBRARIES.
Another convention Peter uses (which you need for the above to work) is making test targets dependent on a tests convenience target in your CMake:
set_target_properties(boost_any_whatever_test PROPERTIES EXCLUDE_FROM_ALL ON)
add_dependencies(tests boost_any_whatever_test)This guide has some other useful content: https://github.com/boostorg/cmake/blob/develop/developer.md
There was a problem hiding this comment.
@anarthal nice! But with -DBOOST_USE_MODULES=1 it prodices the following error:
CMake Error at tools/cmake/include/BoostInstall.cmake:300 (install):
install TARGETS target boost_any is exported but not all of its interface
file sets are installed
Call Stack (most recent call first):
tools/cmake/include/BoostInstall.cmake:548 (boost_install_target)
tools/cmake/include/BoostRoot.cmake:187 (boost_install)
tools/cmake/include/BoostRoot.cmake:330 (__boost_auto_install)
CMakeLists.txt:20 (include)
There was a problem hiding this comment.
Ah. This change in Boost.CMake should fix it: https://github.com/anarthal/boost-cmake/tree/feature/cxx20-modules
If I have the time, I'll create a PR for Boost.CMake next week.
There was a problem hiding this comment.
So for now, it might be okay to keep it as you're doing it and change it later.
There was a problem hiding this comment.
IMO it's worth merging the experimental modules functionality even if modules work only on a single compiler. There are enthusiasts and big companies that are interested in experimenting with modules
There was a problem hiding this comment.
I'm gonna try to move this forward along the next month.
| unique_any/base.cpp | ||
|
|
||
| basic_any_test_mplif.cpp | ||
| basic_any_test_rv.cpp |
There was a problem hiding this comment.
Looks like a subset of the test suite - are the other files causing trouble?
There was a problem hiding this comment.
Other tests are compile-fail tests. What is the right way of writing those in CMake?
There was a problem hiding this comment.
Fair. You can do it, but it's not straightforward. AFAIK Peter has it in his Boost Test Jamfile: https://github.com/boostorg/cmake/blob/81b08178c629b94b67e3baff5b1b9bee25db7f3a/include/BoostTest.cmake#L189-L198
In essence, you create a test that invokes cmake itself to build the target and assert that this fails.
To be fair, I don't think it's worth the effort here.
CMakeLists.txt
Outdated
| ) | ||
| else() | ||
| add_library(boost_any INTERFACE) | ||
| target_include_directories(boost_type_index INTERFACE include) |
#include <boost/any...is now implicitly doesimport boost.anyif the modules are supportedAll the library internals now have unconditional module level linkage.
Significant differences from https://anarthal.github.io/cppblog/modules3:
BOOST_ANY_USE_STD_MODULEmacro switch forimport std;/includeswhile building module. This allows to use module in C++20 and even without usablestdmodule.