Skip to content

feat: Add C++ modules support - #1729

Open
mikomikotaishi wants to merge 2 commits into
open-source-parsers:masterfrom
mikomikotaishi:master
Open

mikomikotaishi wants to merge 2 commits into
open-source-parsers:masterfrom
mikomikotaishi:master

Conversation

@mikomikotaishi

Copy link
Copy Markdown

This PR adds support for C++20 modules, which can be enabled if the version of C++ is at least C++20 (and is opt-in, therefore not raising the minimum C++ version). It is enabled using JSONCPP_WITH_MODULE and creates a module jsoncpp.

@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The PR adds an optional module path for CMake users.

The installed package needs module export information before this is safe to merge.

Findings

  1. P1 Installed module imports fail ▶

Summary

Adds an opt-in C++20 module named jsoncpp.

  • CMake builds JsonCpp as an importable C++20 module when enabled.

Reviews (5) · Last reviewed commit: "Address Greptile suggestions for module ..." · Reviewed by Greptile

Comment thread src/modules/CMakeLists.txt Resolved
Comment thread src/modules/CMakeLists.txt Resolved
Comment thread src/modules/jsoncpp.cppm Resolved
Comment thread src/modules/jsoncpp.cppm Resolved
Comment thread src/modules/CMakeLists.txt Resolved
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 40347579800

Coverage remained the same at 89.926%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2740
Covered Lines: 2615
Line Coverage: 95.44%
Relevant Branches: 2640
Covered Branches: 2223
Branch Coverage: 84.2%
Branches in Coverage %: Yes
Coverage Strength: 23835.16 hits per line

💛 - Coveralls

Comment on lines +34 to +39
install(TARGETS jsoncpp_module
${MODULE_INSTALL_EXPORT}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
FILE_SET CXX_MODULES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/jsoncpp/modules

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Installed module imports fail

This installs the module source, but the existing install(EXPORT jsoncpp ...) call does not write the information CMake needs to connect import jsoncpp to that source. A separate project can find and link jsoncpp::module but cannot compile code that imports it. Add CXX_MODULES_DIRECTORY to the package export when modules are enabled.

Knowledge Base Used: Consumer package integration

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants