-
Notifications
You must be signed in to change notification settings - Fork 84
C++20 module support #104
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
base: develop
Are you sure you want to change the base?
C++20 module support #104
Conversation
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 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. |
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 Now, the above actually fails because apparently, in a module unit, imports are only allowed immediately after the module declaration:
We could work this around by making
I thought MSVC didn't like export using with names attached to a module. Let me double-check this.
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:
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:
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 |
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.
Perhaps, this sort of files should be made common, in a separate internal Boost library.
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.
They are in my Boost.Config branch (https://github.com/anarthal/config/tree/feature/cxx20-modules), but MP11 is supposed to have zero Boost dependencies, so some of them have been copied here. The Boost.Charconv PR uses the ones in Boost.Config.
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, I have a better idea of how this could be implemented, that does not require creating headers like this. See here.
#if defined(BOOST_USE_MODULES) && !defined(BOOST_MP11_INTERFACE_UNIT) | ||
|
||
#include <boost/mp11/version.hpp> | ||
import boost.mp11; |
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.
Say, if another library includes multiple MP11 headers and thus produces multiple import
statements. Would this be harmful in any way? For example, would this affect compilation performance? Would it make sense to move the above two lines in a separate internal header with its own include guard, so that only one import
is produced, no matter how many interface headers are included?
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 don't think there is any performance penalty in importing several times.
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, |
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):
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: boostorg/charconv#255 |
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.