-
Notifications
You must be signed in to change notification settings - Fork 62
[WIP] Support installation of C++20 modules (with non-ROOT functionality) #907
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
Open
wdconinc
wants to merge
30
commits into
AIDASoft:master
Choose a base branch
from
wdconinc:cxx-modules
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+978
−3
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
1782778
feat: Add C++20 module support infrastructure (prototype)
wdconinc eeeca16
feat: Implement C++20 module template generation (80% complete)
wdconinc 9219cdd
fix: Implement namespace-aware module export generation (Strategy 2)
wdconinc 8aec23b
fix: Correct CollectionIterator naming pattern
wdconinc 528d5c4
refactor: Move Pythonizations implementation to separate compilation …
wdconinc a9141d4
fix: Add global namespace prefix for non-namespaced types in modules
wdconinc 59a0e30
feat: Move podio.core module to src/ and enable proper installation
wdconinc e5cf1a9
ci: Enable C++ modules in workflow for gcc15 builds
wdconinc 2aab78a
docs: Add comprehensive C++20 modules user documentation
wdconinc e69da7c
Remove duplicate podio_core_module.ixx from tests directory
wdconinc c5ab024
Clean up license header in podio_core_module.ixx
wdconinc 5a049b7
Add comprehensive test suite for C++ modules functionality
wdconinc dd06c41
Add C++ modules test infrastructure
wdconinc 60f19ea
Add experimental test for direct module imports in .cpp files
wdconinc 6a68383
test: remove redundant C++ module tests, keep only import test
wdconinc 3d37bb6
Disable module import test due to compiler stdlib issues
wdconinc 8db7160
fix: hide Obj, Data, CollectionData in modules
wdconinc 88c80f1
docs: podio.core: generalize from ROOT- to I/O-independent functionality
wdconinc fcbc75c
fix: cpp_generator._build_type_info -> generator_utils.qualified_for_…
wdconinc 14e46d3
feat: add tests/test_module_import
wdconinc 5dc9a50
fix: pre-commit
wdconinc 048ec04
fix: add Threads dependency to podioConfig
wdconinc 621116a
fix: pre-commit with -DPODIO_ENABLE_CXX_MODULES=ON
wdconinc 61d424f
fix: clang-format
wdconinc d5d9138
fix: pylint and flake8 style
wdconinc 739b12e
fix: style
wdconinc 2e991ad
fix: remove generation, integration, unit tests
wdconinc 6c6dafb
fix: disable modules in pre-commit again
wdconinc dfde5d6
fix: consistently use PODIO_ENABLE_CXX_MODULES (incl. in CI)
wdconinc 0fac8aa
fix: update docs to avoid podio_MODULES_AVAILABLE
wdconinc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| #--------------------------------------------------------------------------------------------------- | ||
| # CMake support for C++20 modules in podio | ||
| # | ||
| # This file provides macros and options for generating and using C++20 module interfaces | ||
| # for podio-generated datamodels. | ||
| #--------------------------------------------------------------------------------------------------- | ||
|
|
||
| # Option to enable C++20 module generation | ||
| option(PODIO_ENABLE_CXX_MODULES "Generate C++20 module interface files (.ixx) for datamodels" OFF) | ||
|
|
||
| # Check if modules are actually supported | ||
| if(PODIO_ENABLE_CXX_MODULES) | ||
| # Check CMake version | ||
| if(CMAKE_VERSION VERSION_LESS 3.29) | ||
| message(WARNING "C++20 modules require CMake 3.29 or later (found ${CMAKE_VERSION}). Disabling module support.") | ||
| set(PODIO_ENABLE_CXX_MODULES OFF CACHE BOOL "C++20 modules disabled due to CMake version" FORCE) | ||
| endif() | ||
|
|
||
| # Check generator | ||
| if(CMAKE_GENERATOR STREQUAL "Unix Makefiles") | ||
| message(WARNING "C++20 modules are not supported with the Unix Makefiles generator. Please use Ninja. Disabling module support.") | ||
| set(PODIO_ENABLE_CXX_MODULES OFF CACHE BOOL "C++20 modules disabled due to generator" FORCE) | ||
| endif() | ||
|
|
||
| # Check compiler support | ||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") | ||
| if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 14.0) | ||
| message(WARNING "C++20 modules require GCC 14 or later (found ${CMAKE_CXX_COMPILER_VERSION}). Disabling module support.") | ||
| set(PODIO_ENABLE_CXX_MODULES OFF CACHE BOOL "C++20 modules disabled due to compiler version" FORCE) | ||
| endif() | ||
| elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") | ||
| if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 18.0) | ||
| message(WARNING "C++20 modules require Clang 18 or later (found ${CMAKE_CXX_COMPILER_VERSION}). Disabling module support.") | ||
| set(PODIO_ENABLE_CXX_MODULES OFF CACHE BOOL "C++20 modules disabled due to compiler version" FORCE) | ||
| endif() | ||
| else() | ||
| message(WARNING "C++20 modules have only been tested with GCC 14+ and Clang 18+. Found ${CMAKE_CXX_COMPILER_ID}. Proceed at your own risk.") | ||
| endif() | ||
| endif() | ||
|
|
||
| if(PODIO_ENABLE_CXX_MODULES) | ||
| message(STATUS "C++20 module support is ENABLED for podio datamodels") | ||
| message(STATUS " - CMake version: ${CMAKE_VERSION}") | ||
| message(STATUS " - Generator: ${CMAKE_GENERATOR}") | ||
| message(STATUS " - Compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}") | ||
| else() | ||
| message(STATUS "C++20 module support is DISABLED for podio datamodels") | ||
| endif() | ||
|
|
||
| #--------------------------------------------------------------------------------------------------- | ||
| # PODIO_ADD_MODULE_INTERFACE( target module_name module_file ) | ||
| # | ||
| # Add a C++20 module interface to a target | ||
| # | ||
| # Arguments: | ||
| # target - The target to which the module should be added | ||
| # module_name - The name of the module (e.g., "podio.core") | ||
| # module_file - The .ixx file implementing the module interface | ||
| #--------------------------------------------------------------------------------------------------- | ||
| function(PODIO_ADD_MODULE_INTERFACE target module_name module_file) | ||
| if(NOT PODIO_ENABLE_CXX_MODULES) | ||
| return() | ||
| endif() | ||
|
|
||
| message(STATUS "Adding C++20 module '${module_name}' to target '${target}'") | ||
|
|
||
| # Add the module file to the target as a CXX_MODULES file set | ||
| # Use PUBLIC so the module gets installed and can be used by downstream projects | ||
| target_sources(${target} | ||
| PUBLIC | ||
| FILE_SET CXX_MODULES | ||
| BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} | ||
| FILES ${module_file} | ||
| ) | ||
| endfunction() | ||
|
|
||
| #--------------------------------------------------------------------------------------------------- | ||
| # PODIO_GENERATE_MODULE_INTERFACE( datamodel OUTPUT_FILE ) | ||
| # | ||
| # Generate a C++20 module interface for a podio datamodel | ||
| # This is currently a placeholder - actual generation will be added later | ||
| # | ||
| # Arguments: | ||
| # datamodel - Name of the datamodel (e.g., "TestDataModel") | ||
| # OUTPUT_FILE - Variable name to store the generated .ixx file path | ||
| #--------------------------------------------------------------------------------------------------- | ||
| function(PODIO_GENERATE_MODULE_INTERFACE datamodel OUTPUT_FILE) | ||
| if(NOT PODIO_ENABLE_CXX_MODULES) | ||
| set(${OUTPUT_FILE} "" PARENT_SCOPE) | ||
| return() | ||
| endif() | ||
|
|
||
| # For now, just set the expected output location | ||
| # Actual generation will be implemented in the template | ||
| set(module_file "${CMAKE_CURRENT_BINARY_DIR}/${datamodel}_module.ixx") | ||
| set(${OUTPUT_FILE} ${module_file} PARENT_SCOPE) | ||
|
|
||
| message(STATUS "Will generate module interface: ${module_file}") | ||
| endfunction() |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering whether this should be an explicit argument to the datamodel generation. For my understanding: This requires
podio.coreto be available as a module? Or would it still be possible to have podio built without modules and have the datamodel compiled into a module?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the data model module definition (from
python/templates/datamodel_module.ixx.jinja2) uses just regular includes, not another module import. That means that it is not necessary to have apodio.coremodule in order to use theedm4hep.datamodelmodule.The only thing from podio that is re-exported is:
That is significantly less than what's in
src/podio_core_module.ixxwhich turns into thepodio.coremodule.As long as we don't re-export a different entity under the same name this is fine. They are ultimately just
usingstatements.