Add support for C++20 modules #1580

sharadhr · 2023-05-20T08:26:34Z

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

import vk;
import vk:raii; 

auto main() -> int {
    auto context = vk::raii::Context{};
    auto applicationInfo = vk::ApplicationInfo( AppName.c_str(), 1, EngineName.c_str(), 1, VK_API_VERSION_1_1 );
    ...
}

The implementation follows. We could have, somewhere in vulkan.hpp:

#if VULKAN_HPP_CPP_VERSION >= 20
#   define _VULKAN_HPP_EXPORT export
#else 
#   define _VULKAN_HPP_EXPORT
#endif

And in vulkan_structs.hpp:

_VULKAN_HPP_EXPORT namespace VULKAN_HPP_NAMESPACE 
{
    ...
    struct ApplicationInfo 
    {
    ...
    }
    ...
}

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

The text was updated successfully, but these errors were encountered:

theHamsta · 2023-05-20T10:15:27Z

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

sharadhr · 2023-05-21T05:55:55Z

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

theHamsta · 2023-05-23T17:53:03Z

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?

sharadhr · 2023-05-24T07:15:41Z

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 #included 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.

theHamsta · 2023-05-24T14:14:29Z

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!

sharadhr · 2023-05-25T04:24:28Z

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!

theHamsta · 2023-05-25T11:29:43Z

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. main...theHamsta:Vulkan-Hpp:cpp20-modules). Do you want to create a PR to generate such a file(s) that define C++ modules?

sharadhr · 2023-05-26T04:24:24Z

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 using libxml, and it should be straightforward to set up the generator such that it just dumps a big list of usings 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 #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.

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.

theHamsta · 2023-05-26T12:05:51Z

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 using libxml, and it should be straightforward to set up the generator such that it just dumps a big list of usings 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 #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.

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.

theHamsta · 2023-05-26T13:58:00Z

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.

sharadhr · 2023-05-28T19:47:48Z

@theHamsta, I have opened a PR, #1582. Looking forward to your feedback!

theHamsta · 2023-05-28T23:07:24Z

That you! I'll have a look on Tuesday. I also want the main maintainer of this repo to do the final review.

sharadhr mentioned this issue May 20, 2023

Migrate to fmtlib? #1579

Closed

sharadhr mentioned this issue Jun 26, 2023

Add C++20 module interface file and tests #1582

Merged

asuessenbach closed this as completed in #1582 Jun 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for C++20 modules #1580

Add support for C++20 modules #1580

sharadhr commented May 20, 2023 •

edited

theHamsta commented May 20, 2023 •

edited

sharadhr commented May 21, 2023 •

edited

theHamsta commented May 23, 2023 •

edited

sharadhr commented May 24, 2023

theHamsta commented May 24, 2023 •

edited

sharadhr commented May 25, 2023 •

edited

theHamsta commented May 25, 2023 •

edited

sharadhr commented May 26, 2023 •

edited

theHamsta commented May 26, 2023 •

edited

theHamsta commented May 26, 2023

sharadhr commented May 28, 2023

theHamsta commented May 28, 2023 •

edited

Add support for C++20 modules #1580

Add support for C++20 modules #1580

Comments

sharadhr commented May 20, 2023 • edited

theHamsta commented May 20, 2023 • edited

sharadhr commented May 21, 2023 • edited

theHamsta commented May 23, 2023 • edited

sharadhr commented May 24, 2023

theHamsta commented May 24, 2023 • edited

sharadhr commented May 25, 2023 • edited

theHamsta commented May 25, 2023 • edited

sharadhr commented May 26, 2023 • edited

theHamsta commented May 26, 2023 • edited

theHamsta commented May 26, 2023

sharadhr commented May 28, 2023

theHamsta commented May 28, 2023 • edited

sharadhr commented May 20, 2023 •

edited

theHamsta commented May 20, 2023 •

edited

sharadhr commented May 21, 2023 •

edited

theHamsta commented May 23, 2023 •

edited

theHamsta commented May 24, 2023 •

edited

sharadhr commented May 25, 2023 •

edited

theHamsta commented May 25, 2023 •

edited

sharadhr commented May 26, 2023 •

edited

theHamsta commented May 26, 2023 •

edited

theHamsta commented May 28, 2023 •

edited