Skip to content
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

Support C++20 module. #49

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

stripe2933
Copy link

Thank you for great library.

Since it targets to ≥ C++20, using module can reduce the compilation time significant faster. reflect.cppm file define reflect module that export the symbols.

Also according to this issue, the major 3 compiler vendors agreed to support standard library module from C++20, I add the macro option REFLECT_USE_STD_MODULE to make user can decide whether use module or not (default is 1).

@krzysztof-jusiak
Copy link
Contributor

Thanks @stripe2933 . It's looking good, though I'm am wondering whether additional file is required? Can that work with a single reflect file which is either a module if the flag is defined or a header otherwise? That has been my approach so far as it keep things as simple as possible and less maintenance and/or integration is required, but not sure whether there are some other reasons to do so I'm not aware of?

@krzysztof-jusiak
Copy link
Contributor

On the side note, one thing I'm extremely interested with modules is whether the static assert tests will be called just once per app. That's my assumption based on pch but I haven't tested it, yet.

@stripe2933
Copy link
Author

To my knowledge, since module file must be consist of:

module; // optional

// Global module fragment. Only preprocessing directive can exist in here.

export module <module-name>; // module declaration

// export declaration

module :private; // optional

// Private module fragment. Changing code in here will not trigger recompilation.

(cppreference)
Therefore, the header-only file cannot coexist with a module in a single file. Note that macro tricks like this example are not allowed due to this paper (P1857R3).

For the second question: yes, if static assertions are not depend on template type given by user, all assertions can be verified in module compilation time. Therefore the macro region DISABLE_STATIC_ASSERT_TESTS will be tested only once.

@krzysztof-jusiak
Copy link
Contributor

Thanks for the information and links @stripe2933. Indeed it looks like it's not possible to do it correctly within a single header. I have no issues with the MR though I have issues with coupling modules are introducing for example in the reflect header. Anyway, let me just do a few experiments with modules and will get back with my findings shortly. I'm not sure what I'm looking for yet but wanna be sure that complexity added is required due to C++ design. Thanks again for your work and help.

@krzysztof-jusiak
Copy link
Contributor

Some more thoughts on module support. I've decided to remove any external dependencies from reflect as that make it more portable and easier to deal with and will allow to remove REFLECT_USE_STD_MODULE for the include part. So will get back shortly with that. One thing I wanted to ask is whether it's more beneficial to have a module per library or a common module per multiple libraries. Would the latter allow importing import libs or import libs.reflect import libs.sml etc like in STL and would that require other libraries to compile/exist or would that actually do ifdef kinda way just with modules. I'm thinking about it as that would allow to release multiple libraries torgether with a single module common across them. but I think that can be also done with sub-modules so each library has a module itself and than there is additional module on top, which has different set of trade-offs. Not sure, what is working best?

@stripe2933
Copy link
Author

I've decided to remove any external dependencies from reflect as that make it more portable and easier to deal with and will allow to remove REFLECT_USE_STD_MODULE for the include part.

What you mean is removing standard library module (std) dependency? It seems the library depends on many std things (some type traits, string_view and source_location), so I think implementing them is such like reinventing the wheel.

I also have doubts about the REFLECT_USE_STD_MODULE macro. Recently, Clang has started officially supporting standard library module, MSVC already supports it, CMake 3.30 will support it natively, and vendors including GCC have promised to include the feature in C++20. However, it is uncertain when GCC will support this. I consider this to be a transitional period for this feature, and I think the macro is just a kind of stopgap solution that should be removed in future releases.


One thing I wanted to ask is whether it's more beneficial to have a module per library or a common module per multiple libraries.

I'm also like your library boost-ext/ut and hope both of them are shipped in a single module, but this should be carefully considered. If your boost-ext module have the submodules, boost-ext.ut, boost-ext.mp, boost-ext.di, boost-ext.reflect, ... then the best of my knowledge all of them would be compiled (at least if it uses CMake build system), which may be an undesired result. Shipping them in a single module should be reasonable, e.g. your boost-ext/ut library needs reflection feature provided by boost-ext/reflect.

But this is just my opinion. If I think about it another way, I would not complain much if boost ships boost.asio and boost.graph (which are completely independent) in a single module... because they're sharing their common identity (or brand) boost! It's up to you to decide.

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.

None yet

2 participants