-
Notifications
You must be signed in to change notification settings - Fork 152
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
C++ module interface #1586
Comments
I'd love to get a branch working with module support. Would it work across GCC, MSVC, and Clang? If you want to try and submit a pull request I'd love to see how much progress can be made. I do expect we might need to submit some bug reports because this is a large and complex project. FYI, right now some of the Glaze tests do not build with the latest MSVC due to a recent compiler bug they introduced in 17.2. You can see #1448. We're pending the release of a fix. |
It can easy be done with better quality by hand than any auto converter to work with clang and gcc with CMake. C++ part is easy to do so. msvc AFIK does not have fully working modules at this time. CMake 3.31 works ok with modules. such case works ok on clang shared lib module ;
#include <string>
export module my_library;
export void do_something();
export template<typename T>
struct MyTemplate
{
T value;
};
export struct MyStruct
{
int data;
std::string str;
}; my_library.cpp module;
#include <print>
module my_library;
void do_something() { std::print("Doing something!\n"); } add_library(my_library SHARED)
target_sources(
my_library
PRIVATE src/my_library.cpp
PUBLIC FILE_SET my_module_interface TYPE CXX_MODULES FILES
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/my_library.cxx>"
)
set_target_properties(
my_library
PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED YES
CXX_EXTENSIONS OFF
CXX_SCAN_FOR_MODULES YES) and using of module import my_library;
int main()
{
do_something();
MyTemplate<int> temp{42};
MyStruct struct_ex{123, ""};
return 0;
} cmake_minimum_required(VERSION 3.31)
project(my_project CXX)
set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_SCAN_FOR_MODULES ON)
add_subdirectory(my_library)
add_executable(my_executable src/main.cpp)
target_link_libraries(my_executable PRIVATE my_library)
install(TARGETS my_executable RUNTIME DESTINATION bin)
install(TARGETS my_library LIBRARY DESTINATION lib) |
More complicated it becomes in cmake configuration to maintain backward compatibility and option either to declare interface header only library or module library option(SIMPLE_ENUM_ENABLE_MODULE "Export c++ module" ON)
if( SIMPLE_ENUM_ENABLE_MODULE)
add_library(simple_enum)
target_sources(
simple_enum
PRIVATE FILE_SET HEADERS BASE_DIRS
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
PUBLIC FILE_SET CXX_MODULES FILES
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/module/simple_enum.cxx>"
)
set_target_properties(
simple_enum
PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED YES
CXX_EXTENSIONS OFF
CXX_SCAN_FOR_MODULES YES)
else()
add_library(simple_enum INTERFACE)
target_sources(
simple_enum
INTERFACE FILE_SET
HEADERS
BASE_DIRS
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>)
target_compile_features(simple_enum INTERFACE cxx_std_23)
endif()
if(SIMPLE_ENUM_OPT_IN_STATIC_ASSERTS)
target_compile_definitions(simple_enum INTERFACE SIMPLE_ENUM_OPT_IN_STATIC_ASSERTS=1)
endif()
|
BTW It is not about 'converting' into modules but to add optional module support , so additional c++ files declaring exports that can be ignored with normal pure include interface module;
#include <simple_enum/simple_enum.hpp>
#include <simple_enum/enum_index.hpp>
#include <simple_enum/enum_cast.hpp>
#include <simple_enum/generic_error_category.hpp>
#include <simple_enum/generic_error_category_impl.hpp>
#include <simple_enum/ranges_views.hpp>
#include <simple_enum/std_format.hpp>
export module simple_enum;
export namespace simple_enum
{
using simple_enum::v0_8::enum_concept;
using simple_enum::v0_8::bounded_enum;
using simple_enum::v0_8::adl_info;
using simple_enum::v0_8::info;
using simple_enum::v0_8::enum_name;
using simple_enum::v0_8::enum_name_t;
using simple_enum::v0_8::enum_index_error;
using simple_enum::v0_8::enum_index_t;
using simple_enum::v0_8::enum_index;
using simple_enum::v0_8::consteval_enum_index;
using simple_enum::v0_8::enum_cast_error;
using simple_enum::v0_8::enum_cast_t;
using simple_enum::v0_8::enum_cast;
[..]
} // namespace simple_enum |
Don't worry, importizer aims to reduce much trivial work only, of course we still have to do some fixing and cleaning up ourselves, it's not as smart as a human :(. In addition, it is not wrapping the header interface to export things, it is literally going to modularize it into a hybrid interface, supporting the global module migration. But yeah, I do agree that linting is a bit annoying right now. |
IMHO mixing module and non module code with conditional macro definitions is low quality code hard to maintain and at the end difficult to read. EDIT: |
Btw for modules support You need to have 3.28
|
Or you can just modularize it completely and maintain both the header ones and the module one. That would be tiring and repeating lots of code, though IMHO. Or you can just wrap it, but that wouldn't help with the global module migration :(. About maintainability, I find that aside from the section on top of the file, everything else is the same, isn't it? At the end of the day, let's try and see how Stephen feels about it. Also, the tool provides many options, including toggling the output extension. See more in the setting section of the README. |
I will give You and idea if You are a dev of this tool and maybe interested in that .. With cmake You can get targets and its sources etc.. module;
// include deps required for exported type
export module my_name;
// scanned output of types, classes, functions, and variables. That would be very valuable tool for ppl with large projects, because You would be able to understand the result and maintain it. |
In ideal world we would liek to just convert our projects into module versions , but we ca not and we will not be able to do so for a long time .. IMHO separate files with explicit content being exported is better to maintain that conditional macros everywhere in code .. |
This is not feasible to maintain and not rational double the possibility of introducing bugs. |
Btw the decision is in Stephan's hands, I have nothing to do with it except to advise. |
Yeah, I am also planning for that, not only modularizing, but also wrapping the header intetface into a module. Right now I have only modularization (because I want to help with the migration), so I didn't implement wrapping yet, but it is also a good option to have. Although I feel like this is done by hand really easily, just do some #include, and a big export block, right? Actual modularizing is way harder, that's why I made the tool.
This is right, but the hybrid form doesn't have conditional macros everywhere. It's only in the top section, there is no more conditional macros down below. Also, we shouldn't be discussing extension to importizer here, can you make an issue there and we'll continue? |
What about 90% of most incldues content in detail namespaces ? export everything ? |
There is an EXPORT macro (like assert) that is defined to be export when modules are on, and nothing when it's off. This is done in a separate file (no pollution) that is included automatically, so you can just place the EXPORT before whatever you want and be done! It's just a minimal replacement for the export keyword. Don't worry, it will get just a bit dirty at the top and no more. A lot of people are probably confused like you, so maybe I should add an FAQ section 😆. |
@stephenberry I finished module support in simple_enum except glaze dedicated header .. That requires some version of glaze to be used for exporting empty/hidden content module, and then this restricts users to use that version of glaze .. module;
import glaze;
#define SIMPLE_ENUM_SKIP_INCLUDING_GLAZE // for below header skipping inclusion of glaze headers
#include <simple_enum/glaze_json_enum_name.hpp> // for declaring from/to specializations
export simple_enum.glaze;
// everything hidden |
I am sorry but I didn't pay much attention to Your project because I don't use macros 99% of the time. |
From my module simple enum experiment with modules
IMHO it is a long road to stable modules and it is not yet worth work except for clang witch is in good shape so far. |
Thanks for all the helpful thoughts and notes here. I've done some module experimentation and was waiting for better GCC support before continuing, but I am eager to see what others are able to acheive. Some thoughts:
|
I have a configuration for that in importizer
Compiler support for
I am only working on clang right now, importizer is really new. Btw, could you show me the warnings? I don't have MSVC on me :(. If you still want a header and the module, you can still wrap it in a big module interface |
Thanks for your response! Your project seems really neat. I to dig up my module testing work. For now I'll trust your expertise because you have a lot more experience with this than me. |
I have a problem. When modularizing the include directory, some of the files use pragma once, and some of them uses include guard (include/glaze/dragonbox). And that is confusing the tool. Do you know why this is the case, and can I normalize it to just pragma once or just include guard? @stephenberry |
You can change it to just use |
@stephenberry I was compiling the source code with clang(++) with -v output:
(I was trying to test with fuzzing because my gcc doesn't support libfuzzer) when I got a bunch of errors. There is no modification made to the source.
What did I miss here? Or is this an actual bug? |
Also, are you sure you want to convert standard includes to import std/std.compat? It is not that well supported: https://en.cppreference.com/w/cpp/compiler_support#C.2B.2B23_library_features:~:text=Standard%20Library%20Modules If you still insist, we can bootstrap it with a generated, fake std module. |
short answer AFIR, gnu team in libstdc++ is allowed to break any c++ rules, clang team does not support breaking rules. so compiling constexpr clang + libstdc++ is tricky. I was bullied on the r/cpp for questioning libstdc++ std::string design. |
what's the point of that ? it does not change anything on user of glaze side. If it exists in system then it may speedup development of glaze when used but ... |
To get more out of the compile time benefits of modules using |
@msqr1 Your Clang compilation error is probably because you need to build with |
A big +1 to this. It will be best to support pure modules if possible. The export-and-using style is actually a wrapper only. And also it will be best to use
|
I'm trying my best, but both a pure module one, and a header one is tedious to maintain. I feel like my hybrid style is not too ugly while allowing users to keep 1 version of their code. @ChuanqiXu9, do you have any suggestions? Should I continue to develop importizer in this direction? Also, we can fake a std module, and just use that until import std becomes available widely. |
It depends. But the conclusion should be, we should only support hybrid style if users allowed. It is much easier to write and maintain codes in pure modules mode. Maybe it is helpful to take a look at https://github.com/ChuanqiXu9/clang-modules-converter. I had a mode to convert headers to module partitions. |
(The hybrid style is inspired by your project second mode :))
Except for the preamble, It's quite clean and maintainable below, don't you agree? @ChuanqiXu9 |
It looks neat. But the standard don't allow |
@ChuanqiXu9 Can we discuss more here: msqr1/importizer#12 |
@stephenberry I was compiling the source code with clang(++) with -v output:
And while running tests, I got this error with UBSAN/ASAN:
|
Thanks for reporting this! I think I found the issue and fixed it in this merge: #1596. The function signature on the DLL loader was missing a |
I think this is just a signature mismatch that I can't reproduce reliably. Yesterday it stopped me from working, today it just worked again, with or without the noexcept... I can never reproduce it anymore. I hate these kind of bugs lol. |
Oh, that is strange. |
clang will not understand gcc build std modules format, and clang + gnu libstdc++ is the ** main ** use case on linux of clang as using llvm libc++ would require one to have full lib stack using llvm libc++ which is currently impossible in native linux builds, and when used in vcpkg libc++ has significantly slower implementations of some containers. And second I am not sure of a module size/contents of a case like this module;
import std;
export module glaze;
// ... As I understand it will import into hidden part of glaze module everything from std lib module while this module;
#include <string_view>
export module glaze;
// ... will include string_view deps and prebuild them in hidden part of module for user, so there is only small cost at compile time of glaze. Am I right ? |
No, private module imports can do dead code elimination. The compiler uses the precompiled information (from |
Maybe i was not clear about scenario
I build my code with import glaze using clang which tries to load dependency of import std prebuild with gcc from system which is not going to work at all. |
Ah, I see what you're getting at. Yes, developers will likely have to stick with one standard library. But, isn't that the case with linking libraries currently? |
No it is not, as clang is compatibile with ABI with gcc and it is very widly used combo on linux. |
Yes, I also use |
sure they have different symbols in different namespaces. |
I see your project using C++23, it doesn't have a C++ module support yet, and I could provide one using https://github.com/msqr1/importizer ? I would be happy to do this if you want!
The text was updated successfully, but these errors were encountered: