skypjack
Add entt as C++ module
- Make EnTT consumable as C++ module
- Added
ENTT_MODULECMake option enabling EnTT compilation as C++ module - Added
ENTT_USER_CONFIGCMake option to be able to specify C++ config defines
I've noticed that similar pull rq is already ongoing but seems to be dead for a few months.
Instead of exporting symbols in separate files, export macros are used (similar to how fmt does it).
By setting ENTT_MODULE CMake option to ON the EnTT::EnTT target becomes module library allowing EnTT to be consumed as import entt;.
Another option ENTT_USER_CONFIG is added to CMake. It can be used to configure entt as on can't simply #define all configurations inside source with modules.
Usage example:
entt_config.hpp
#define ENTT_ID_TYPE std::uint64_t
#define ENTT_USE_ATOMIC
CMakeLists.txt
...
set(ENTT_MODULE ON)
cmake_path(APPEND PROJECT_SOURCE_DIR user_config.hpp OUTPUT_VARIABLE ENTT_USER_CONFIG)
add_subdirectory(entt)
...
target_link_libraries(exe PUBLIC EnTT::EnTT)
Some explanation of this would be nice to put in wiki, unfortunately I can't pull request changes to wiki AFAIK.
I've added free operators into exports as unlike member operators they're not reachable just by aliasing the according class.
Also declaring unqualified friend class results in forced injection under modules resulting in ambiguous symbols later. I've made sure that all friend class declarations uses qualified names to avoid that.
This looks interesting. Quite a lot actually. Thanks for contributing! Can I ask you why this solution should be better than the one in the other PR? Don't get me wrong, I'm not saying it isn't. I'm just trying to understand pros and cons. Try explaining this to me like I'm 3 years old. 🙂 I would like to involve @elvisdukaj who implemented the first draft. Do you see any issues with this approach? You've done more research on the topic than I have, so your opinion might be helpful.
I was going to test this PR myself but I noticed that when -DENTT_MODULE=ON it causes an error when -DENTT_INSTALL=ON is also set.
CMake Error at CMakeLists.txt:307 (install):
install TARGETS target EnTT is exported but not all of its interface file
sets are installed
CMake Error at CMakeLists.txt:327 (export):
export Export set "EnTTTargets" not found.
@skypjack Hi, I see several benefits in this approach compared to the other one.
I'll start right away with the one I see as the most important. There is an important difference between where all the definitions are located.
In the other PR all the symbols are defined inside global module fragment (GMF 10.4). This fragment is meant for preprocessor directives, including includes of legacy code. When the module is built all symbols that are not decl-reachable (10.4.3) are dumped. This might lead to a situation described in the 2nd example (10.4.6), where a such defined symbol, might be "incorrectly" marked as unreachable and thus dumped away, resulting in compilation error. Due to this I'm highly concerned about entt::internal definitions, that might get dumped that way.
You can see similar behavior even with STL being used in entt. When using import entt you might be forced to use also import std/#include <put-your-favorite-stl-lib-here> even though not using it directly, but through entt. But unlike with STL that you might include separately from entt, you can't simply #include <entt/entt.hpp> along with import entt just to get to the internal symbols.
It doesn't mean this approach is bad, for me it just seems more challenging to maintain (and thus error-prone) as you have to follow extra set of rules to ensure all symbols are decl-reachable.
In this PR, all the entt symbols are defined in the module itself and are not dumped by the compiler the same way. All the intenal symbols are still reachable (but not visible) for the caller without the need to include the internals in another way.
Another benefit I see is potentially less boilerplate (but that's somewhat questionable) as you don't need to re-export all the symbols again as in .inc files in the other PR.
On the other hand, with the other approach you have nice concise list of all exported symbols at hand, but you have to manage it more carefully (when adding new symbol you need to think about adding it to an extra file).
Also with the other PR's approach you don't have to care about dependencies again. As you just include the entt/entt.hpp which already has its dependencies sorted, you then include the .inc files in any order you want (let's say alphabetically). With this PR's approach as you can see in entt.ixx the dependecies needs to be resolved for the second time and includes cannot be reordered arbitrarily.
Also all the includes inside all the source files has to be suppressed to avoid linking unwanted symbols (like STL ones) to the module (as it will result in symbol ambiguity if STL and entt is included in some project). Technically, one can suppress only non-entt headers (+ few of the config headers) in modules and it might be possible to also include just entt/entt.hpp, might be worth the investigation.
And last thing to mention is that the GMF approach is meant mostly for the transition to modules and is more fit to wrap external library if one wants to use it in own project and doesn't have much control over the library's source. For the actual implementation the symbol definitions directly inside module seems more fit.
I would really love to hear @elvisdukaj's opinion as I'm still not absolutely sure about everything personally. Even though modules are there for a few years already, they're still kinda new and niche and hard to put into existing codebases at work so hard to get to first hand experience with them still :)
Hi! Thanks for keeping me in the loop.
@MilanBorovy great work — I really like your approach, and I’m aligned with your points.
When I started my PR, my goal was to wrap EnTT so it could be consumed as a module by my project, so I went with GMF. I used the glm module file as a starting point. At the time, I didn’t consider the decl-reachability issue because the library lived inside my source tree. In that regard, your approach looks more robust and less error-prone.
I still have two reservations:
Ergonomics around configuration. Requiring ENTT_USER_CONFIG to configure the library feels a bit clunky. Having two places to configure things — entt/config.h and a custom file passed to CMake — can be confusing. Better documentation and a short “consuming EnTT via modules” guide would help a lot, but I think there’s still room for improvement.
File extensions for C++ modules. I’d prefer avoiding a new extension just for modules and sticking with *.cpp. Different compilers assign special meanings to various extensions (e.g., MSVC treats *.icxx as module units), which can introduce friction and inconsistency across toolchains.
Overall, I think is a more maintainable approach in the long term.
Ok, sounds reaonsable. @MilanBorovy what do you think about the suggestions/comments by @elvisdukaj? To be clear, I'd like to refine and merge this PR as soon as I cut a new release. The last one is the last one in the C++17 land, then I'll move the project to C++20. This makes the PR a perfect fit. The idea is to make it happen by the end of the month, or early next month in the worst case. Sounds like a plan? Are you willing to help and keep it in sync until then @MilanBorovy or should I take over the PR in the meantime?
@elvisdukaj
ad 1) I'm not sure wdym by the entt/config.h as an option to configure EnTT.
Maybe I'm missing out smth, but the standard way of configuring EnTT (non-module) is
#define ENTT_ID_TYPE std::uint64_t
/* imagine any other configurations */
#include <entt/entt.hpp>
or
// user-config.hpp
#define ENTT_ID_TYPE std::uint64_t
/* imagine any other configurations */
// some file.cpp
#include "user-config.hpp"
#include <entt/entt.hpp>
Unfortunately the first one cannot be done using modules and the second one I've implemented by the ENTT_USER_CONFIG passed in via CMake.
If there's any other config option please enlighten me and I may try to make it work with it.
Anyway, I agree that this must be documented, but as I've stated in the PR I cannot include wiki changes in PR AFAIK and will need @skypjack to document it, I guess.
ad 2) I understand your point and kind of agree with that, as module should be kind of a new standard for C++ so why to treat it differently from other implementation files. I just remember VS/MSVC being pretty aggressive about enforcing use of .ixx extension for modules, whereas other non-MS-tainted toolchains were much more relaxed and even though recommending different extensions (like .cppm for LLVM), they did not care really.
I'm not sure if this still applies (hopefully not) and would gladly change the extension to .cpp if someone is able to test it would compile under Win/VS/MSVC. I can try under VM to run that setup later in case no one is willing to do that.
as I've stated in the PR I cannot include wiki changes in PR AFAIK
You can actually 🙂 the wiki is part of this repo too, see the doc dir. I know it's uncommon, but I like to embed it here.
As for merging your PR, I'm all about accepting it. Gimme the time of a couple of changes, and I'll cut the last C++17 release.
Without rushing (because it's open source after all), can I ask you to keep the PR in sync and fix conflicts in the meantime? 🙏
Thank you in advance for all your help!!
I've added documentation of the module configuration.
Also rebased to the current version of master.