C++20 module support
Adds support for import boost.mp11.
This pull request is intended to get feedback and support discussion in the ML. It depends on changes in Boost.CMake, Boost.Core, Boost.ThrowException and Boost.Assert that aren't in develop.
Any feedback is welcome.
Looks nicer.
Why do we have to include mp11.hpp in the GMF though? Can't we attach things to the named module?
For mp11 in concrete, yes, we can. It would require removing the cassert include and placing it in the GMF. It likely requires reverting to the BOOST_MP11_EXPORT macros we used to have.
For libraries that need to access entities in the detail namespace in tests (charconv being the one I've experimented with), this is not possible, since it causes redefinition errors for the tests that include detail headers.
Is there a benefit to having names attached to the named module? (Genuine question that I don't know the answer to).
I don't understand why we'd need <cassert> in the GMF, and I also don't understand why we'd need to revert to the export macros. Are we aiming to support configurations where BOOST_USE_MODULES is inconsistently defined in TUs? Should we?
IIUC the compiler needs to perform less work for names in the named module, because ODR is guaranteed and there's no need to merge declarations.
I don't understand why we'd need
in the GMF
If we do:
export module boost.mp11;
#define BOOST_MP11_INTERFACE_UNIT
#include <boost/mp11.hpp> // Has an #include <cassert>
export namespace boost::mp11 { /* ... */ }
Any implementation-defined C++ entity declared in cassert get incorrectly attached to the boost.mp11. If we used the new boost/config/std/cassert.hpp header that just imports std, we don't get the assert macro.
Now, the above actually fails because apparently, in a module unit, imports are only allowed immediately after the module declaration:
[build] In file included from [redacted]/boost-with-modules/libs/mp11/modules/boost_mp11.cppm:9:
[build] In file included from [redacted]/boost-with-modules/libs/mp11/include/boost/mp11.hpp:21:
[build] In file included from [redacted]/boost-with-modules/libs/mp11/include/boost/mp11/tuple.hpp:24:
[build] [redacted]/boost-with-modules/libs/mp11/include/boost/mp11/detail/std/type_traits.hpp:2:1: error: imports must immediately follow the module declaration
[build] 2 | import std;
[build] | ^
[build] In file included from [redacted]/boost-with-modules/libs/mp11/modules/boost_mp11.cppm:9:
[build] In file included from [redacted]/boost-with-modules/libs/mp11/include/boost/mp11.hpp:21:
[build] In file included from [redacted]/boost-with-modules/libs/mp11/include/boost/mp11/tuple.hpp:25:
[build] [redacted]/boost-with-modules/libs/mp11/include/boost/mp11/detail/std/cstddef.hpp:2:1: error: imports must immediately follow the module declaration
[build] 2 | import std;
We could work this around by making boost/config/std/$HEADER.hpp do nothing for module units (e.g. if BOOST_CONFIG_MODULE_UNIT is defined or something like this).
I also don't understand why we'd need to revert to the export macros.
I thought MSVC didn't like export using with names attached to a module. Let me double-check this.
Are we aiming to support configurations where BOOST_USE_MODULES is inconsistently defined in TUs? Should we?
No, and I don't think so.
I was right, entities attached to modules can't be exported with export using. This is true even in clang:
[build] [redacted]/boost-with-modules/libs/mp11/modules/boost_mp11.cppm:16:13: error: using declaration referring to 'mp_transform' with module linkage cannot be exported
[build] 16 | using mp11::mp_transform;
So if we get down this path, we need to revert to the BOOST_MP11_EXPORT macros.
What would you finally like to do with this? I see three options:
- Throw it away and forget about it
- Wait until modules are more mature - e.g. until MSVC fixes the two bugs I raised and CMake's import std becomes stable.
- Merge it for this or next release as experimental - I'd put some time in cleaning up CIs and writing docs.
IMO 2 is the more reasonable. 3's only point would be pushing modules forward.
(2), I think.
@anarthal Great idea, I also came up with mostly the same thing, but for a general purpose app that does this automatically, this may be able to help you: https://github.com/msqr1/importizer
Look at transitional modularization
Although not needed for MP11, what would be the strategy if the library needs a third-party include, for which there is no module? For example, #include <unistd.h> or #include <windows.h>? Note that some of these includes may depend on predefined macros, such as _POSIX_SOURCE or _WIN32_WINNT.
Although not needed for MP11, what would be the strategy if the library needs a third-party include, for which there is no module? For example,
#include <unistd.h>or#include <windows.h>? Note that some of these includes may depend on predefined macros, such as_POSIX_SOURCEor_WIN32_WINNT.
The canonical approach is including these in the global module fragment. Depending on where the include is, and the modularization technique we're using (see https://anarthal.github.io/cppblog/modules3 for the techniques - Boost.Mp11 uses technique 1, while Boost.Charconv uses technique 2):
- If we're including the third-party header in a cpp file, we need to make sure that it's included before the
export module boost.xyzstatement. Should be relatively trivial. - If we're including the third-party header in a public header, and using modularization technique 1 (the cppm includes everything after
export module boost.xyz), then you need to ifdef-out all third-party includes, then include them again in the global module fragment. - If we're using technique 2 (the cppm includes everything before the
export module boost.xyz), then you leave the include as-is, and don't need to duplicate anything.
Having a lot of third-party includes might be a good reason to go for 2. 1 is the ideal one because it's better for compile times and allows better ODR checks, but might be infeasible for reasons like this, or to support the test suite (as is charconv's case).
Charconv PR: https://github.com/boostorg/charconv/pull/255
To avoid wasting CI resources on the boostorg account, I'll close this PR and reopen once everything works on my fork.