Skip to content

Make library work with C++20 modules - #4764

Merged
nlohmann merged 3 commits into
developfrom
issue4759-module
Apr 28, 2025
Merged

nlohmann merged 3 commits into
developfrom
issue4759-module

Conversation

@nlohmann

@nlohmann nlohmann commented Apr 27, 2025 •

Copy link
Copy Markdown
Owner

Closes #4759.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@coveralls

coveralls commented Apr 27, 2025 •

Copy link
Copy Markdown

Coverage Status

coverage: 99.188%. remained the same
when pulling 5dfd121 on issue4759-module
into 51a77f1 on develop.

* 🐛 add missing header

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* 🚨 fix warning

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* 🚨 fix warning

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann marked this pull request as ready for review April 27, 2025 16:24
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Apr 27, 2025

@slavek-kucera slavek-kucera left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@nlohmann
nlohmann merged commit eef76c2 into develop Apr 28, 2025
@nlohmann
nlohmann deleted the issue4759-module branch April 28, 2025 19:52
@nlohmann nlohmann added this to the Release 3.12.1 milestone May 21, 2025
@nlohmann nlohmann linked an issue May 21, 2025 that may be closed by this pull request
2 tasks
@ChuanqiXu9

Copy link
Copy Markdown

Given you already make this, would you like to offer module interfaces for this library officially?

@nlohmann

Copy link
Copy Markdown
Owner Author

What would make it "official"?

@ChuanqiXu9

Copy link
Copy Markdown

What would make it "official"?

If users can import json; (or import nlohmann.json;) after they installed the repo. Concretely. I think it is fine to move tests/module_cpp20/json.cpp (I feel better to rename it to json.cppm) in this patch to the source directory and add an cmake option NLOHMANN_JSON_MODULES, and when this option is ON, the module interface unit file will be installed to the target path.

In another word, if this project offers the module interface file, I think the project offers modules officially.

@nlohmann

Copy link
Copy Markdown
Owner Author

Thanks for the clarification. I am hesitant here, because currently we lack MSVC support. And before offering an official module, I would need to make sure that the whole test suite runs then, too. So I think it requires some more effort, but I think we're on a good track.

@slavek-kucera

Copy link
Copy Markdown

Both MSVC and gcc still struggle with modules. I opened a case for MSVC in April. And there is a case for gcc opened back in 2021.

@mikomikotaishi mikomikotaishi mentioned this pull request May 29, 2025
3 of 5 tasks
jwnimmer-tri pushed a commit to jwnimmer-tri/json that referenced this pull request Sep 21, 2026
* ✅ add test for C++20 modules

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* 🚨 fix warning

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Add missing header (nlohmann#4763)

* 🐛 add missing header

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* 🚨 fix warning

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* 🚨 fix warning

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI CMake documentation M review needed It would be great if someone could review the proposed changes. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler error exposes TU-local entity on gcc-trunk while compiling the library as a module Doesn't seem to work with Clang C++20 modules

4 participants