-
Notifications
You must be signed in to change notification settings - Fork 67
[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
33
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.
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
ff9d9e2
feat: Add C++20 module support infrastructure (prototype)
wdconinc 1490753
feat: Implement C++20 module template generation (80% complete)
wdconinc 2917291
fix: Implement namespace-aware module export generation (Strategy 2)
wdconinc ab84c1a
fix: Correct CollectionIterator naming pattern
wdconinc bd7ae87
refactor: Move Pythonizations implementation to separate compilation …
wdconinc 9a10f3e
fix: Add global namespace prefix for non-namespaced types in modules
wdconinc 18c1c7d
feat: Move podio.core module to src/ and enable proper installation
wdconinc be8253f
ci: Enable C++ modules in workflow for gcc15 builds
wdconinc a30d6ba
docs: Add comprehensive C++20 modules user documentation
wdconinc f335bd2
Remove duplicate podio_core_module.ixx from tests directory
wdconinc 1e273cb
Clean up license header in podio_core_module.ixx
wdconinc d749f20
Add comprehensive test suite for C++ modules functionality
wdconinc 7c3ff2b
Add C++ modules test infrastructure
wdconinc 411fe7f
Add experimental test for direct module imports in .cpp files
wdconinc 4207fd1
test: remove redundant C++ module tests, keep only import test
wdconinc f9c244b
Disable module import test due to compiler stdlib issues
wdconinc 4d4173c
fix: hide Obj, Data, CollectionData in modules
wdconinc 36aaced
docs: podio.core: generalize from ROOT- to I/O-independent functionality
wdconinc ad8256a
fix: cpp_generator._build_type_info -> generator_utils.qualified_for_…
wdconinc 84d80d8
feat: add tests/test_module_import
wdconinc b013c2f
fix: pre-commit
wdconinc 67fa013
fix: add Threads dependency to podioConfig
wdconinc c73b79d
fix: pre-commit with -DPODIO_ENABLE_CXX_MODULES=ON
wdconinc adcbf63
fix: clang-format
wdconinc a8bce2e
fix: pylint and flake8 style
wdconinc 49f661d
fix: style
wdconinc c8f0108
fix: remove generation, integration, unit tests
wdconinc 328fffb
fix: consistently use PODIO_ENABLE_CXX_MODULES (incl. in CI)
wdconinc ed5ce17
fix: update docs to avoid podio_MODULES_AVAILABLE
wdconinc a3ed905
Remove cmake arguments that are covered by presets
tmadlener d21f2f6
fix: suppress cppcheck errors for .ixx module files and pre-existing …
jmcarcell 46e13eb
fix: exclude .ixx files and module import tests from clang-tidy
jmcarcell 1c33f6e
fix: undef PODIO_JSON_OUTPUT in module interfaces to avoid TU-local n…
jmcarcell 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
Some comments aren't visible on the classic Files Changed page.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| noExplicitConstructor:* | ||
| *:*.ixx | ||
| unusedFunction:* | ||
| missingInclude:* | ||
| missingIncludeSystem:* | ||
|
|
||
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
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.