Closed sharadhr closed 1 year ago
@mtavenrath has experiences with compiling Vulkan as a C++ module and can surely share his experiences. Using VulkanHpp as a C++ module was beneficial for compile times. Progress with C++20 modules in Cmake, clang and gcc as been considerable recently, tough I believe only msvc has a released (non-development) compiler version with full standard conform support.
I think we still have to be careful since not all compilers support full C++20 yet. Instead of VULKAN_HPP_CPP_VERSION >= 20
a feature test macro could be used (__cpp_modules
, https://en.cppreference.com/w/cpp/feature_test). Using modules also affects the build system, so this might also be an opt-in macro definition for the user.
@theHamsta,
Good points on modularised code being opt-in. That said, I refer to the MSVC STL as an exemplar (they have even implemented C++23 import std
). It appears users can still use the headers as-is, if they don't want to use std
as a module, so I presume Vulkan-Hpp
might have a vulkan.ixx
(or Vulkan.cppm
) similar to std.ixx
.
Also good points on the feature test macro, especially since not all compilers have landed complete support for it (even MSVC emits C1001 internal compiler errors sometimes). I used VULKAN_HPP_CPP_VERSION
since that is what Vulkan-Hpp
itself uses for feature testing (such as operator<=>
).
As for build tools, there's experimental CMake support detailed in this blog post by KitWare, and the fmtlib
implementer has some original work (used in fmtlib
10.0.0), too. Both Ninja and MSBuild also have support for C++20 modules.
I think the time is ripe to attempt modularising Vulkan-Hpp
, especially since both MSVC and LLVM have (at least) experimental implementations of std
modules.
For clang-17 (nightly, Ubuntu clang version 17.0.0 (++20230523041032+61bc3ada1f90-1~exp1~20230523041147.523), I can compile
// module;
// #include <algorithm>
export module foo;
export namespace foo {
int foo = 1;
}
with but not when I would uncomment the first few lines. As mentioned in the Kitware article, clang seems to have problems with not standard C++. For both libc++ and libstdc++, it fails to process #include_next "stddef.h"
In file included from /media/other_linux/home/stephan/projects/Vulkan-Hpp/samples/Cpp20Modules/module.cpp:3:
In file included from /usr/include/c++/v1/algorithm:1770:
In file included from /usr/include/c++/v1/__algorithm/inplace_merge.h:27:
In file included from /usr/include/c++/v1/__memory/temporary_buffer.h:17:
In file included from /usr/include/c++/v1/new:99:
In file included from /usr/include/c++/v1/cstdlib:87:
In file included from /usr/include/c++/v1/stdlib.h:94:
In file included from /usr/include/stdlib.h:32:
/usr/include/c++/v1/stddef.h:17:15: fatal error: 'stddef.h' file not found
The following would work
module;
#import <vulkan/vulkan.h>
Also CMake (3.26.4) module support is still experimental
CMake Warning (dev):
C++20 modules support via CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP is
experimental. It is meant only for compiler developers to try.
This warning is for project developers. Use -Wno-dev to suppress it.
So, for non-msvc compilers this would require a bit more on the compiler side or work-arounds for the header generation. MSVC worked fine for you?
MSVC worked fine for you?
Yes, it did!
More importantly, seeing your comment, here's an example on Compiler Explorer using clang 16.0.0, the experimental CMake API, and the #include
d headers you've mentioned (original from this Reddit comment, slightly modified); the source files successfully compile and run.
If we want to use modules without CMake, then there has to be a pre-compilation pass as seen in the Clang documentation; it's not as straightforward as clang++ main.cpp -o main
. This is because modules means that code has to be compiled in the correct dependency order. So
Otherwise, P1689R5 enables compilers to scan source code for module dependencies before-hand, thus allowing build tools to produce a dependency graph and then compile modules in the correct order. As documented in the Kitware blog post, CMake's experimental API will assist for compilers that have implemented this proposal (MSVC ≥ 19.34, Clang ≥ 16.0.0) by extracting the module dependencies without requiring that the user specify the module and implementation files in the correct order.
I am guessing it is marked 'experimental' because, like you said, compiler support can be patchy, and the documentation especially is lacking; we require clear examples to proceed with modules.
An additional -isystem /usr/lib/llvm-17/lib/clang/17/include/
convinced clang++ to compile the modules correctly even with more complex C++ standard headers. I'll have a look on how it could be integrated into the CMake build system of VulkanHpp. At the moment, I'm thinking about having a .cpp
, .cxx
, .cppm
file that would include a normal vulkan.hpp
and export the relevant symbols as a module.
Did you manage to compile the hpp
files directly as a FILE_SET
for a module (to use the approach you proposed)? For me CMake had already reserved file extensions for module files and was ignoring hpp
files just as headers (there must be a setting to change that behavior). Did you change the file extension of the headers in order to compile them as modules? It would be bit strange to generate VulkanHpp a second time just to have to right file extension.
EDIT: apparently, you can use set_source_file_properties
to treat *.hpp
as module implementations. So we can use your approach!
I'm thinking about having a .cpp, .cxx, .cppm file that would include a normal vulkan.hpp and export the relevant symbols as a module.
I think this is the most reasonable way forward. We could simply have a vk.ixx
and vk.cppm
(for compatibility between both MSVC and Clang, both of which expect different file extensions for modules, grrr...)
However, it appears my initial comment is not as straightforward; each of the *.hpp
files will need a global module fragment, the module export declaration, and the export keyword itself, so for vulkan_structs.hpp
, for instance:
VULKAN_HPP_MODULE_FRAGMENT // #defined as `module;`
#include <cstring>
VULKAN_HPP_EXPORT_MODULE // #defined as `export module vk;`
VULKAN_HPP_EXPORT namespace VULKAN_HPP_NAMESPACE
{
...
struct ApplicationInfo
{
...
}
...
}
Did you manage to compile the hpp files directly as a FILE_SET for a module (to use the approach you proposed)? ...
I have been trying to hand-convert vulkan.hpp
into an exported module and test it, but it is considerably more complex compared to the simple hello-world examples given so far.
apparently, you can use set_source_file_properties to treat *.hpp as module implementations. So we can use your approach!
Could you elaborate on this? I haven't seen this before!
I was trying your approach with
if (VULKAN_HPP_EXPERIMENTAL_CPP20_MODULES)
# See https://www.kitware.com/import-cmake-c20-modules/
cmake_minimum_required(VERSION 3.26)
set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "2182bf5c-ef0d-489a-91da-49dbc3090d2a")
set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP 1)
add_library(VulkanHppModule)
set_target_properties(VulkanHppModule PROPERTIES
CXX_STANDARD 20
CXX_EXTENSIONS OFF)
target_sources(VulkanHppModule
PUBLIC
FILE_SET cxx_modules TYPE CXX_MODULES FILES
${VulkanHeaders_INCLUDE_DIR}/vulkan/vulkan.hpp
#${VulkanHeaders_INCLUDE_DIR}/vulkan/vulkan_raii.hpp
)
set_source_files_properties(
${VulkanHeaders_INCLUDE_DIR}/vulkan/vulkan.hpp
#${VulkanHeaders_INCLUDE_DIR}/vulkan/vulkan_raii.hpp
PROPERTIES LANGUAGE CXX)
target_compile_definitions(VulkanHppModule PRIVATE -DVULKAN_HPP_EXPERIMENTAL_CPP20_MODULES=1)
endif()
to compile vulkan.hpp
as a source file (so file extensions don't matter if you do post-configuration of CMake). But this will cause problems with the dynamic dependency scanner as it will not expand preprocessor macros to parse export module
declarations
FAILED: CMakeFiles/VulkanHppModule.dir/CXX.dd
/snap/cmake/1299/bin/cmake -E cmake_ninja_dyndep --tdi=CMakeFiles/VulkanHppModule.dir/CXXDependInfo.json --lang=CXX --modmapfmt=clang --dd=CMakeFiles/VulkanHppModule.dir/CXX.dd @CM
akeFiles/VulkanHppModule.dir/CXX.dd.rsp
CMake Error: Output CMakeFiles/VulkanHppModule.dir/vulkan/vulkan.hpp.o is of type `CXX_MODULES` but does not provide a module
with
module;
#include <map>
#include <set>
#include <memory>
#include <utility> // std::exchange, std::forward
#include <cstring> // strcmp
#include <algorithm>
#include <vector>
#include <array>
#if __cpp_lib_format
# include <format> // std::format
#else
# include <sstream> // std::stringstream
#endif
#include <vulkan/vulkan.h>
export module vulkan;
#include <vulkan/vulkan.hpp>
You can get quite close to a working module. Since I'm using #include
in the Vulkan module (which is of course not recommended. I need to prevent any global module declarations to leak into the Vulkan module.
FAILED: CMakeFiles/VulkanHppModule.dir/vulkan.cppm.o CMakeFiles/VulkanHppModule.dir/vulkan.pcm
/usr/lib/ccache/clang++-17 -DCLANG_FORMAT_EXECUTABLE=\"/usr/bin/clang-format\" -DVULKAN_HPP_EXPERIMENTAL_CPP20_MODULES=1 -I/media/other_linux/home/stephan/projects/Vulkan-Hpp -fdia
gnostics-color -isystem /usr/lib/llvm-17/lib/clang/17/include/ -g -std=c++20 -MD -MT CMakeFiles/VulkanHppModule.dir/vulkan.cppm.o -MF CMakeFiles/VulkanHppModule.dir/vulkan.cppm.o.d
@CMakeFiles/VulkanHppModule.dir/vulkan.cppm.o.modmap -o CMakeFiles/VulkanHppModule.dir/vulkan.cppm.o -c /media/other_linux/home/stephan/projects/Vulkan-Hpp/vulkan.cppm
In file included from /media/other_linux/home/stephan/projects/Vulkan-Hpp/vulkan.cppm:17:
/media/other_linux/home/stephan/projects/Vulkan-Hpp/vulkan/vulkan.hpp:5959:64: error: declaration of 'getDispatchLoaderStatic' with internal linkage cannot be exported
static inline ::VULKAN_HPP_NAMESPACE::DispatchLoaderStatic & getDispatchLoaderStatic()
^
/media/other_linux/home/stephan/projects/Vulkan-Hpp/vulkan/vulkan.hpp:6621:23: error: declaration of 'throwResultException' with internal linkage cannot be exported
[[noreturn]] void throwResultException( Result result, char const * message )
^
warning: ISO C++20 does not permit using directive to be exported [-Wexport-using-directive]
which causes a problem for the functions that are declared static
/anonymous namespace within a export block which could get their own (non-exported) namespace block. But still I'll get linker errors after building the module.
I think the cleanest solution would be to just re-export the symbols from a separate module file (e.g. https://github.com/KhronosGroup/Vulkan-Hpp/compare/main...theHamsta:Vulkan-Hpp:cpp20-modules). Do you want to create a PR to generate such a file(s) that define C++ modules?
which causes a problem for the functions that are declared static/anonymous namespace within a export block which could get their own (non-exported) namespace block.
Yep, this is the same error I'm running into: https://github.com/KhronosGroup/Vulkan-Hpp/compare/main...sharadhr:Vulkan-Hpp:cpp20-modules, although my work is a lot less clean, I was still figuring out the codebase and I sort of mixed it in with my std::format
work...
I think the cleanest solution would be to just re-export the symbols from a separate module file
Agreed on this, and your implementation looks great (never thought of using xyz
within a namespace to export things). We already have all the exported types available using libxml
, and it should be straightforward to set up the generator such that it just dumps a big list of using
s into vulkan.cppm
. I also propose we don't really separate out the RAII types (because the vk::raii namespace already does), and export them together.
One question: your code #include
s vulkan.h
for constants like VK_API_VERSION_1_1
. Any thoughts on making them constexpr auto
variables, also exported in the module file, so that users can avoid #include
ing anything related to Vulkan? There's relevant discussion here; the solution appears rather unwieldy to hand-write (we need an intermediate variable foo
and then need to #undef
the macro). Somewhat similar to how Vulkan-Hpp
has been handling enums, maybe we can also take a stab at properly type-specifying some of these macros.
So, to summarise, we could have something like:
module;
#include <vulkan/vulkan.hpp> // maybe defines `VULKAN_HPP_MODULE`?
#include <vulkan/vulkan_raii.hpp>
export module VULKAN_HPP_MODULE;
export namespace VULKAN_HPP_NAMESPACE {
using vk::ApplicationInfo;
using vk::createInstance;
using vk::InstanceCreateInfo;
using vk::Instance;
using vk::SystemError;
...
namespace VULKAN_HPP_RAII_NAMESPACE {
using namespace vk::raii::Buffer;
using namespace vk::raii::Image;
...
constexpr auto cApiVersion1_1 = VK_API_VERSION_1_1; // 'c' prefix for **c**onstant
...
}
Do you want to create a PR to generate such a file(s) that define C++ modules?
Sounds good; I'll reset my code back and start working on the generator to produce vulkan.cppm
.
which causes a problem for the functions that are declared static/anonymous namespace within a export block which could get their own (non-exported) namespace block.
Yep, this is the same error I'm running into: main...sharadhr:Vulkan-Hpp:cpp20-modules, although my work is a lot less clean, I was still figuring out the codebase and I sort of mixed it in with my
std::format
work...I think the cleanest solution would be to just re-export the symbols from a separate module file
Agreed on this, and your implementation looks great (never thought of
using xyz
within a namespace to export things). We already have all the exported types available usinglibxml
, and it should be straightforward to set up the generator such that it just dumps a big list ofusing
s intovulkan.cppm
. I also propose we don't really separate out the RAII types (because the vk::raii namespace already does), and export them together.One question: your code
#include
svulkan.h
for constants likeVK_API_VERSION_1_1
. Any thoughts on making themconstexpr auto
variables, also exported in the module file, so that users can avoid#include
ing anything related to Vulkan? There's relevant discussion here; the solution appears rather unwieldy to hand-write (we need an intermediate variablefoo
and then need to#undef
the macro). Somewhat similar to howVulkan-Hpp
has been handling enums, maybe we can also take a stab at properly type-specifying some of these macros.So, to summarise, we could have something like:
module; #include <vulkan/vulkan.hpp> // maybe defines `VULKAN_HPP_MODULE`? #include <vulkan/vulkan_raii.hpp> export module VULKAN_HPP_MODULE; export namespace VULKAN_HPP_NAMESPACE { using vk::ApplicationInfo; using vk::createInstance; using vk::InstanceCreateInfo; using vk::Instance; using vk::SystemError; ... namespace VULKAN_HPP_RAII_NAMESPACE { using namespace vk::raii::Buffer; using namespace vk::raii::Image; ... constexpr auto cApiVersion1_1 = VK_API_VERSION_1_1; // 'c' prefix for **c**onstant ... }
Do you want to create a PR to generate such a file(s) that define C++ modules?
Sounds good; I'll reset my code back and start working on the generator to produce
vulkan.cppm
.
Having VK_API_VERSION_1_1
as constexpr variable sounds good to me! I also like ash's vk::make_api_version
https://docs.rs/ash/0.37.2+1.3.238/ash/ (could be vk::makeApiVersion
for VulkanHpp, https://github.com/ash-rs/ash/blob/dc562fc745e816901a9e5bba9b9f0f7befb4e706/ash/src/vk/definitions.rs#L32-L63). Maybe a constexpr function would be more flexible than a constant and less preprocessor magic version macro of vulkan.h
export module VULKAN_HPP_MODULE;
I think you aren't allow to do preprocessor magic on the module name. At least for me, the CMkae dyndep scanner, wasn't expanding preprocessor definition. It might be that this is planned to change and it will be supported in future.
One question: your code #includes vulkan.h for constants like VK_API_VERSION_1_1. Any thoughts on making them constexpr auto variables, also exported in the module file, so that users can avoid #includeing anything related to Vulkan? There's relevant discussion here; the solution appears rather unwieldy to hand-write (we need an intermediate variable foo and then need to #undef the macro). Somewhat similar to how Vulkan-Hpp has been handling enums, maybe we can also take a stab at properly type-specifying some of these macros. I think it would be fine to for a first iteration
I think it would be fine for a first iteration of VulkanHpp modules to not be "complete". We always can add more exports if we see that something is missing in terms of feature parity with vulkan.hpp
Sounds good; I'll reset my code back and start working on the generator to produce vulkan.cppm.
I have no opinion on the file extension. .cpp
/.ixx
/.cppm
or something else are all fine for me.
export module VULKAN_HPP_MODULE; I think you aren't allow to do preprocessor magic on the module name. At least for me, the CMkae dyndep scanner, wasn't expanding preprocessor definition. It might be that this is planned to change and it will be supported in future.
But you're right. We don't need dyndeps for VulkanHpp. If it stays opt-in, it would be possible to do preprocessor magic with modules.
@theHamsta, I have opened a PR, #1582. Looking forward to your feedback!
That you! I'll have a look on Tuesday. I also want the main maintainer of this repo to do the final review.
I've been wanting to add support for C++20 modules to Vulkan-Hpp ever since I began using it. I was thinking of implementing something similar to the MSVC STL macro for the C++ standard library, so that users could perform something like
The implementation follows. We could have, somewhere in
vulkan.hpp
:And in
vulkan_structs.hpp
:This is a fairly straightforward change that should massively improve compile times for modern compilers (MSVC ≥ 19.29 and Clang ≥ 16.0.0); I hope this is considered.
I've been working on adding this directly into the generator instead of hand-modifying the generated
.hpp
files, but I got sidetracked (#1579).