Skip to content

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

Draft
wants to merge 84 commits into
base: develop
Choose a base branch
from

Conversation

anarthal
Copy link

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.

@pdimov
Copy link
Member

pdimov commented Jan 15, 2025

Looks nicer.

Why do we have to include mp11.hpp in the GMF though? Can't we attach things to the named module?

@anarthal
Copy link
Author

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

@pdimov
Copy link
Member

pdimov commented Jan 16, 2025

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.

@anarthal
Copy link
Author

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.

@anarthal
Copy link
Author

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.

@anarthal
Copy link
Author

What would you finally like to do with this? I see three options:

  1. Throw it away and forget about it
  2. Wait until modules are more mature - e.g. until MSVC fixes the two bugs I raised and CMake's import std becomes stable.
  3. 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.

@pdimov
Copy link
Member

pdimov commented Jan 24, 2025

(2), I think.

@msqr1
Copy link

msqr1 commented Feb 26, 2025

@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

Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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;
Copy link
Member

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?

Copy link
Author

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.

@Lastique
Copy link
Member

Lastique commented Apr 7, 2025

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.

@anarthal
Copy link
Author

anarthal commented Apr 7, 2025

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.

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.xyz statement. 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: boostorg/charconv#255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants