Support for C++20/23 modules#395
Conversation
|
I didn't manage to get |
|
I'm having trouble pairing modules and tests with GTest, primarily related to this issue: google/googletest#4851. I'll see what I can do tomorrow. All examples appear to work |
|
Hi I am looking forward to using this. What's the problem you are seeing? if it is the import-#include ordering thing, can we just have #include before import in the tests? And it is not so strict, see: https://github.com/alibaba/async_simple/blob/CXX20Modules/async_simple_module/test/coro/SleepTest.cpp |
|
Hi. Great to see that somebody is interested in this. I’m away this weekend, so I’ll be able to finish it on Monday.
…________________________________
From: Chuanqi Xu ***@***.***>
Sent: Saturday, March 14, 2026 1:53:25 AM
To: Thalhammer/jwt-cpp ***@***.***>
Cc: Yan Romao ***@***.***>; Author ***@***.***>
Subject: Re: [Thalhammer/jwt-cpp] Support for C++20/23 modules (PR #395)
[https://avatars.githubusercontent.com/u/68680648?s=20&v=4]ChuanqiXu9 left a comment (Thalhammer/jwt-cpp#395)<#395 (comment)>
Hi I am looking forward to using this. What's the problem you are seeing? if it is the import-#include ordering thing, can we just have #include before import in the tests? And it is not so strict, see: https://github.com/alibaba/async_simple/blob/CXX20Modules/async_simple_module/test/coro/SleepTest.cpp
—
Reply to this email directly, view it on GitHub<#395 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AMEFH5FNJJXC5IR76DJXH2L4QS3RLAVCNFSM6AAAAACHQ2GNRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANJZGEZTOMJZGI>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@ChuanqiXu9 Hi. Sorry for the delay. I’ve just pushed a commit that now uses ABI-breaking wrapper style. Could you have a look at the current implementation and tell whether you're satisfied with it? Thank you. |
| // Build the public headers in module purview so imported declarations are | ||
| // attached to jwt_cpp instead of the global module. | ||
| #define JWT_CPP_MODULE_INTERFACE_BUILD 1 | ||
| export { |
There was a problem hiding this comment.
nit: export all seems slightly not good but it is your choices.
There was a problem hiding this comment.
I tried to use ABI-Breaking style, as it was recommended by people in other libraries. For example, a Vulkan-hpp contributor said switching to an ABI-Breaking approach allowed them to resolve multiple issues and make many tests that were previously failing to pass. I think the last time there were some issues using the "export using style", however I can try again.
There was a problem hiding this comment.
That document says it is better to add 'export' to different entities instead of exporting all things
There was a problem hiding this comment.
Hi @ChuanqiXu9. Would this be correct? 46b4ea6. Thanks.
|
@prince-chrismc Thank you for resolving conflicts. I'll add a workflow for modules tomorrow and do some minor cleanups, so I think this will be ready soon. Sorry for the delay. |
prince-chrismc
left a comment
prince-chrismc
left a comment
There was a problem hiding this comment.
Few small questions
| #ifndef picojson_h | ||
| #define picojson_h | ||
|
|
||
| #if defined(JWT_CPP_MODULE_INTERFACE_BUILD) && defined(JWT_USE_IMPORT_STD) |
There was a problem hiding this comment.
Picojson is a vendored dep, we shouldn't be making changes to it.
| target_compile_features(jwt-cpp ${JWT_LIBRARY_TYPE} cxx_std_20) | ||
| else() | ||
| target_compile_features(jwt-cpp ${JWT_LIBRARY_TYPE} cxx_std_23) | ||
| target_compile_definitions(jwt-cpp PRIVATE JWT_USE_IMPORT_STD) |
There was a problem hiding this comment.
If this was public, could we remove the extra defines in the examples and test apps?
| #include <iostream> | ||
| #include <sstream> | ||
|
|
||
| #include "jwt-cpp/traits/boost-json/traits.h" |
There was a problem hiding this comment.
Is there a technical reason to change the header includes? Not familiar enough with modules to know.
| option(JWT_ENABLE_COVERAGE "Enable code coverage testing" OFF) | ||
| option(JWT_ENABLE_FUZZING "Enable fuzz testing" OFF) | ||
| option(JWT_ENABLE_MODULES "Build C++ modules" OFF) | ||
| option(JWT_USE_IMPORT_STD "Use import std" OFF) |
There was a problem hiding this comment.
Not a requirement, but asking for "import std" to be available will reduce the complexity.
| cmake_policy(SET CMP0135 NEW) | ||
| endif() | ||
|
|
||
| set(CMAKE_EXPERIMENTAL_CXX_IMPORT_STD "451f2fe2-a8a2-47c3-bc32-94786d8fc91b") |
There was a problem hiding this comment.
It is better to make this an default value then users can override the value by cmake's -D option during configuraton.
4 participants
This adds support for C++20/23 modules. Please let me know if you want anything done in a different way. There is still some work left to do: update all examples, tests, docs and CI/CD. I tested the code on Windows, with MSVC latest (v18) on a print-claims example.
JWT_ENABLE_MODULES- to enable modulesJWT_USE_IMPORT_STD- to useimport stdUPD: The latest implementation was tested with MSVC (14.51) and Clang-22