[Core] Fix module compilation #87
Conversation
- Configure Ninja generator for C++ module builds - Add gcc-15/llvm-21 toolchains in CI - Stabilize CI dependencies and pin CMake 4.0.2 - Update CMake gates for version 4.2 - Harden CI gcc-15 installation and module links - Fix header file set install for interface targets - Build libc++ modules on macOS when missing - Align CI toolchain and CMake installs - Make std-module patch resilient and non-fatal Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix module export leakage to consumers - Fix INTERFACE target wiring for libc++ module detection - Add fallback path for libc++ modules.json - Trim amgcl module and fix MPI communication - Fix module exports for clang and GCC - Adjust ext.amgcl module for GCC compatibility - Mute thread-safety warnings in modules - Propagate MPI definitions to module builds Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Provide dummy MPI types when MPI is disabled - Fix MPI logging macro guards - Guard dummy MPI typedef when MPI headers present - Include mpi.h when available in non-MPI builds - Disable MPI C++ bindings globally - Initialize MPI when headers are available - Expose MPI init in module builds when headers exist - Guard -mfma flag for non-x86 architectures - Add macOS SDK sysroot for Homebrew clang - Fix macOS libc++ modules.json detection - Restore macOS gcc matrix and fix boost cstdint includes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update copyright notices in source files from 2025 to 2026. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace std::print with std::format for portability - Avoid chrono time zone APIs (require C++20 timezone support) - Fix amgcl test includes and std::print usage - Update C++ headers to use standard library consistently - Apply code formatting across the codebase Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Luohaothu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the core compilation process, updating copyright information, enhancing MPI compatibility, and ensuring unique output directories for example runs. The changes contribute to a more robust and maintainable codebase. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of changes to fix module compilation issues and improve overall build system compatibility and robustness. Key changes include:
- Conditional CMake logic for experimental features based on CMake version.
- Correct handling of architecture-specific compiler flags.
- Replacement of C++23
std::printand C++20 chrono formatting instd::formatwith more widely supported alternatives (std::coutandstrftime), enhancing compiler compatibility. - Refactoring of CMake scripts to use a consistent
OPFLOW_TARGET_SCOPEvariable, improving maintainability. - Conditional includes in test files to support both module and header-only builds.
- Fixes for MPI C++ bindings and vendored library
amgcl.
The changes are well-thought-out and address the goal of fixing compilation issues effectively. I have one minor suggestion to improve code clarity in the CMake scripts by removing a redundant variable definition.
| if(OPFLOW_ENABLE_MODULE) | ||
| set(OPFLOW_TARGET_SCOPE PUBLIC) | ||
| else() | ||
| set(OPFLOW_TARGET_SCOPE INTERFACE) | ||
| endif() |
There was a problem hiding this comment.
This if/else block to set OPFLOW_TARGET_SCOPE is redundant. The same logic is already present in the root CMakeLists.txt file, and the variable is set before this script is included. Removing this redundant block will make the build configuration cleaner and easier to maintain.
No description provided.