Expose version module using C++20 modules #6761

kylehaokunwu · 2025-08-03T03:09:27Z

Fixes #

Proposed Changes

Added hpx_core.ixx umbrella module interface exposing the version module
Integrated version module into HPX build system using target_sources(... FILE_SET ...)
Enabled C++20 modules conditionally via HPX_USE_MODULES
Added install rules for the new hpx_core_modules FILE_SET

Any background context you want to provide?

This is part of the ongoing GSoC project to expose HPX using C++20 modules. This PR exposes the version module as a starting point. Follow-up PRs will expose additional modules one at a time. HPX was successfully built with -DHPX_USE_MODULES=ON, and the version_api_test passed

Checklist

Not all points below apply to all pull requests.

I have added a new feature

Signed-off-by: Haokun Wu <[email protected]>

StellarBot · 2025-08-03T03:10:03Z

Can one of the admins verify this patch?

hkaiser

Very nice! Could you please enable testing against using C++ modules as well? Will that require changes to. e.g. <hpx/modules/version.hpp>?

CMakeLists.txt

libs/CMakeLists.txt

libs/core/config/include/hpx/config/export_definitions.hpp

libs/core/hpx_core.ixx

libs/core/version/include/hpx/version.hpp

kylehaokunwu · 2025-08-05T03:31:28Z

Very nice! Could you please enable testing against using C++ modules as well? Will that require changes to. e.g. <hpx/modules/version.hpp>?

I updated version_api.cpp to conditionally use import HPX.Core when HPX_WITH_CXX_MODULES is defined, and it builds and runs successfully. No additional changes are needed. Would you like me to include this change in the PR?

hkaiser · 2025-08-05T12:51:02Z

Very nice! Could you please enable testing against using C++ modules as well? Will that require changes to. e.g. <hpx/modules/version.hpp>?

I updated version_api.cpp to conditionally use import HPX.Core when HPX_WITH_CXX_MODULES is defined, and it builds and runs successfully. No additional changes are needed. Would you like me to include this change in the PR?

Well, yes! Just we have to do it in a way that the test has not to be touched. We may have to change the generated (HPX) modules files for this (e.g., <hpx/modules/version.hpp>, though (see: https://github.com/STEllAR-GROUP/hpx/blob/master/cmake/templates/global_module_header.hpp.in, generated here: https://github.com/STEllAR-GROUP/hpx/blob/master/cmake/HPX_AddModule.cmake#L135-L161)

kylehaokunwu · 2025-08-07T22:33:33Z

https://github.com/STEllAR-GROUP/hpx/blob/master/cmake/templates/global_module_header.hpp.in

Very nice! Could you please enable testing against using C++ modules as well? Will that require changes to. e.g. <hpx/modules/version.hpp>?

I updated version_api.cpp to conditionally use import HPX.Core when HPX_WITH_CXX_MODULES is defined, and it builds and runs successfully. No additional changes are needed. Would you like me to include this change in the PR?

Well, yes! Just we have to do it in a way that the test has not to be touched. We may have to change the generated (HPX) modules files for this (e.g., <hpx/modules/version.hpp>, though (see: https://github.com/STEllAR-GROUP/hpx/blob/master/cmake/templates/global_module_header.hpp.in, generated here: https://github.com/STEllAR-GROUP/hpx/blob/master/cmake/HPX_AddModule.cmake#L135-L161)

Just to confirm — would it be appropriate to add the following to global_module_header.hpp.in?

#pragma once

#if defined(HPX_WITH_CXX_MODULES)
import HPX.Core;
#else
#include <hpx/version.hpp>
#endif

Then update version_api.cpp to include:
#include <hpx/global_module.hpp>

hkaiser · 2025-08-07T23:27:38Z

Just to confirm — would it be appropriate to add the following to global_module_header.hpp.in?
#pragma once

#if defined(HPX_WITH_CXX_MODULES)
import HPX.Core;
#else
#include <hpx/version.hpp>
#endif
Then update version_api.cpp to include: #include <hpx/global_module.hpp>

Yes and no. This is how it should look like for the version module (except for the #if defined(HPX_WITH_CXX_MODULES), which should be #if defined(HPX_HAVE_CXX_MODULES)). But this template file is used to generate the module headers for all modules.

So we probably need to add a if()/else()/endif() to the file cmake/HPX_AddModule.cmake that switches between different template files (one as is and one that supports C++ modules) that is controlled by a new GLOBAL_HEADER_MODULE_GEN. As we adapt HPX modules this has to be added to those we touch. We can remove this again once we're done.

This should go here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/cmake/HPX_AddModule.cmake#L157-L160, here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/libs/core/version/CMakeLists.txt#L35 (and obviously here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/cmake/HPX_AddModule.cmake#L13 and here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/.cmake-format.py#L359).

I hope this makes sense.

Signed-off-by: Haokun Wu <[email protected]>

codacy-production · 2025-08-09T04:59:15Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for `c4028b9`¹	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`c4028b9`)	Report Missing	Report Missing	Report Missing
Head commit (`d705337`)	59585	0	0.00%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6761)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

hkaiser

@kylehaokunwu Excellent work!

hkaiser · 2025-08-09T13:42:29Z

CMakeLists.txt

@@ -17,6 +17,10 @@

 cmake_minimum_required(VERSION 3.18 FATAL_ERROR)

+if(HPX_WITH_CXX_MODULES)
+  cmake_minimum_required(VERSION 3.29...3.29.99 FATAL_ERROR)


Shouldn't it work with newer versions (than 3.29) as well?

Suggested change

cmake_minimum_required(VERSION 3.29...3.29.99 FATAL_ERROR)

if (NOT (CMAKE_VERSION VERSION_GREATER_EQUAL "3.29"))

hpx_fatal("Please use a version of CMake newer than V3.28 in order to enable C++ module support for HPX")

endif()

Thanks for the catch! I will switch from the fixed range to a VERSION_GREATER_EQUAL 3.29 check. I will keep it inside if(HPX_WITH_CXX_MODULES) so non‑module builds still work with CMake ≥ 3.18.

hkaiser · 2025-08-09T13:46:00Z

cmake/HPX_AddModule.cmake

+    if(DEFINED ${modulename}_GLOBAL_HEADER_MODULE_GEN
+       AND ${modulename}_GLOBAL_HEADER_MODULE_GEN
    )
+      set(global_header
+          "${CMAKE_CURRENT_BINARY_DIR}/include/hpx/global_module.hpp"
+      )
+      set(template_file
+          "${HPX_SOURCE_DIR}/cmake/templates/global_module_header_modules.hpp.in"
+      )
+    else()
+      set(global_header
+          "${CMAKE_CURRENT_BINARY_DIR}/include/hpx/modules/${modulename}.hpp"
+      )
+      set(template_file
+          "${PROJECT_SOURCE_DIR}/cmake/templates/global_module_header.hpp.in"
+      )
+    endif()


The name of the generated file should be the same in both cases:

Suggested change

if(DEFINED ${modulename}_GLOBAL_HEADER_MODULE_GEN

AND ${modulename}_GLOBAL_HEADER_MODULE_GEN

)

set(global_header

"${CMAKE_CURRENT_BINARY_DIR}/include/hpx/global_module.hpp"

)

set(template_file

"${HPX_SOURCE_DIR}/cmake/templates/global_module_header_modules.hpp.in"

)

else()

set(global_header

"${CMAKE_CURRENT_BINARY_DIR}/include/hpx/modules/${modulename}.hpp"

)

set(template_file

"${PROJECT_SOURCE_DIR}/cmake/templates/global_module_header.hpp.in"

)

endif()

set(global_header

"${CMAKE_CURRENT_BINARY_DIR}/include/hpx/modules/${modulename}.hpp"

)

if(${modulename}_GLOBAL_HEADER_MODULE_GEN)

set(template_file

"${HPX_SOURCE_DIR}/cmake/templates/global_module_header_modules.hpp.in"

)

else()

set(template_file

"${PROJECT_SOURCE_DIR}/cmake/templates/global_module_header.hpp.in"

)

endif()

hkaiser · 2025-08-09T13:47:20Z

cmake/templates/global_module_header_modules.hpp.in

+#if defined(HPX_HAVE_CXX_MODULES)
+import HPX.Core;
+#else
+#include <hpx/version.hpp>


For all HPX modules to be converted in the future, the generated module header should still include all necessary files as before:

Suggested change

#include <hpx/version.hpp>

@module_headers@

hkaiser · 2025-08-09T13:48:49Z

libs/core/version/CMakeLists.txt

@@ -32,7 +32,8 @@ endif()
 include(HPX_AddModule)
 add_hpx_module(
  core version
-  GLOBAL_HEADER_GEN OFF
+  GLOBAL_HEADER_GEN ON
+  GLOBAL_HEADER_MODULE_GEN TRUE


Just for consistency:

Suggested change

GLOBAL_HEADER_MODULE_GEN TRUE

GLOBAL_HEADER_MODULE_GEN ON

hkaiser · 2025-08-09T13:51:04Z

libs/core/hpx_core.ixx

+
+export module HPX.Core;
+
+#include "version/include/hpx/version.hpp"


Now that we're generating the module headers correctly, shouldn't we add them here (one by one as we adapt the HPX modules):

Suggested change

#include "version/include/hpx/version.hpp"

#include <hpx/modules/version.hpp>

hkaiser · 2025-08-09T13:51:43Z

libs/core/hpx_core.ixx

+
+// Define module-specific macros before including config
+#define HPX_BUILD_MODULE
+#include "config/include/hpx/config.hpp"


I think this should be:

Suggested change

#include "config/include/hpx/config.hpp"

#include <hpx/config.hpp>

If you would like to minimize the #included content, this could be:

Suggested change

#include "config/include/hpx/config.hpp"

#include <hpx/config/defines.hpp>

which contains all the HPX_HAVE_XXX macro definitions.

hkaiser · 2025-08-09T14:00:28Z

CMakeLists.txt

+if(HPX_WITH_CXX_MODULES)
+  hpx_info("HPX will build and install C++20 module interface units for downstream users.")
+  hpx_info("Note: HPX itself will still be built using traditional headers.")
+  add_compile_definitions(HPX_HAVE_CXX_MODULES)


Generally, all preprocessor macros are put into a special file (<hpx/config/defines.hpp>) instead of being added to the compiler's command line. This ensures that all targets that depend on HPX (even external ones) will see a consistent set of preprocessor definitions.

Suggested change

add_compile_definitions(HPX_HAVE_CXX_MODULES)

hpx_add_config_define(HPX_HAVE_CXX_MODULES)

hkaiser · 2025-08-09T14:09:47Z

libs/core/version/tests/unit/version_api.cpp

@@ -4,8 +4,8 @@
 //  Distributed under the Boost Software License, Version 1.0. (See accompanying
 //  file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

+#include <hpx/global_module.hpp>


Let's stick with the conventions:

Suggested change

#include <hpx/global_module.hpp>

#include <hpx/modules/version.hpp>

Alternatively (to avoid having to touch all files that currently #include <hpx/version.hpp>), we could add #include <hpx/modules/version.hpp> in that file.

Not sure if this wouldn't create a circular #include sequence, though. In this case, we need to reshuffle the content slightly:

Move the content of the current hpx/version.hpp to a new file, e.g., hpx/version_definitions.hpp

Add this new file to the ${version_headers} here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/libs/core/version/CMakeLists.txt#L9

Change the content of hpx/version.hpp to simply #include <hpx/modules/version.hpp>

Exclude hpx/version.hpp from being added to the generated module (use EXCLUDE_FROM_GLOBAL_HEADER hpx/version.hpp here: https://github.com/kylehaokunwu/hpx/blob/expose/version-module/libs/core/version/CMakeLists.txt#L37)

hkaiser · 2025-08-09T14:56:21Z

libs/core/version/include/hpx/version.hpp

@@ -17,56 +17,56 @@
 namespace hpx {

    // Returns the major HPX version.
-    [[nodiscard]] HPX_CORE_EXPORT std::uint8_t major_version();
+    HPX_MODULE_EXTERN_CORE [[nodiscard]] std::uint8_t major_version();


I think this has to be in a reverse sequence, it may generate compilation errors otherwise.

Suggested change

HPX_MODULE_EXTERN_CORE [[nodiscard]] std::uint8_t major_version();

[[nodiscard]] HPX_MODULE_EXTERN_CORE std::uint8_t major_version();

Expose HPX version module using c++ 20 modules

11f05b0

Signed-off-by: Haokun Wu <[email protected]>

kylehaokunwu requested a review from hkaiser as a code owner August 3, 2025 03:09

hkaiser added category: core type: enhancement project: GSoC labels Aug 3, 2025

hkaiser requested changes Aug 3, 2025

View reviewed changes

WIP - addressed requested changes in PR

7a99bc7

kylehaokunwu pushed a commit to kylehaokunwu/hpx that referenced this pull request Aug 9, 2025

Address review feedback for PR STEllAR-GROUP#6761

648d7b5

Address review feedback for PR STEllAR-GROUP#6761

d705337

Signed-off-by: Haokun Wu <[email protected]>

kylehaokunwu force-pushed the expose/version-module branch from 648d7b5 to d705337 Compare August 9, 2025 04:33

hkaiser reviewed Aug 9, 2025

View reviewed changes

-  cmake_minimum_required(VERSION 3.29...3.29.99 FATAL_ERROR)
+  if (NOT (CMAKE_VERSION VERSION_GREATER_EQUAL "3.29"))
+    hpx_fatal("Please use a version of CMake newer than V3.28 in order to enable C++ module support for HPX")
+  endif()


		export module HPX.Core;

		#include "version/include/hpx/version.hpp"

	#include "version/include/hpx/version.hpp"
	#include <hpx/modules/version.hpp>

	#include "config/include/hpx/config.hpp"
	#include <hpx/config.hpp>

	#include "config/include/hpx/config.hpp"
	#include <hpx/config/defines.hpp>

	add_compile_definitions(HPX_HAVE_CXX_MODULES)
	hpx_add_config_define(HPX_HAVE_CXX_MODULES)

	#include <hpx/global_module.hpp>
	#include <hpx/modules/version.hpp>

	HPX_MODULE_EXTERN_CORE [[nodiscard]] std::uint8_t major_version();
	[[nodiscard]] HPX_MODULE_EXTERN_CORE std::uint8_t major_version();

Uh oh!

Expose version module using C++20 modules #6761

Are you sure you want to change the base?

Expose version module using C++20 modules #6761

Conversation

kylehaokunwu commented Aug 3, 2025

Proposed Changes

Any background context you want to provide?

Checklist

Uh oh!

StellarBot commented Aug 3, 2025

Uh oh!

hkaiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kylehaokunwu commented Aug 5, 2025

Uh oh!

hkaiser commented Aug 5, 2025

Uh oh!

kylehaokunwu commented Aug 7, 2025

Uh oh!

hkaiser commented Aug 7, 2025

Uh oh!

codacy-production bot commented Aug 9, 2025

Coverage summary from Codacy

Footnotes

Uh oh!

hkaiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hkaiser Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hkaiser Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hkaiser Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hkaiser Aug 9, 2025 •

edited

Loading

hkaiser Aug 9, 2025 •

edited

Loading

hkaiser Aug 9, 2025 •

edited

Loading