-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Incompatible with C++20 modules in MSVC #3970
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
Comments
Is it fixed now? |
it's not fixed, I got this compilation error as well with single include and modules. |
same issue in version 3.11.3 |
I have the issue, 3.11.3, Visual Studio, using C++20 modules. |
The issue is not an issue with nlohmann/json, but rather MSVC compiler. I get the same issue with c42f/tinyformat. The compiler does not like something in the header and I just have not yet figured out what it is. You can work around it if you add a "compilation firewall" by defining an interface in a header and shoving the include in a regular cpp. Too bad both libraries rely heavily on template to make the client code nice and small. |
I'm having a similar issue when trying to include
The following section of my code: nlohmann::json config = {
{"Version", "x.x.x.d"}
}; Provides the error:
I'm in a greenfield project and would love to be able to use this library to parse and export json data. @rioki, are you able to expand on your temporary fix until either this library or MSVC have a solution? Many thanks. |
For my project I moved to jsoncpp, which has been working fine with a module wrapper that I made. |
@JamieSharpe I dropped modules altogether for the time being. I ran into 1001 issues like this. I will revisit modules in a year or two. Looks promising, not there yet. |
Exactly how did you implement the compilation firewall? Perhaps you can show some code. I'm getting compilation errors when I try to do it. :/ |
For me the following works: JSONTools.ixx
DataDefs.cpp
CMakeLists.txt
For every other module I am just importing |
We are also running into this issue. Has anyone raised an issue about this with the MSVC team yet? @rioki This issue is the last roadblock for us to finally migrate parts of our codebase to C++ modules.
|
To fix this, replace in your json.hpp file (around line 20120):
|
Can anyone confirm this? If so, I'm happy to see a PR for this :) |
All I can say is that i was getting compilation error from this issue and issue mentioned here: and removing these '::' did fix my issue. Not sure if it is universal solution or only solution for my case but seems working to me. |
Hey guys, Basically what we did is putting this inside our nlohman_json module
This works fine for most of our use cases. Best case is the avoid name collisions (i.e. don't mix @nlohmann do you think it ts possible to extract all makors to a seperate file which ony uses either If that is possible the approach suggestion above should work :) |
@Ju1He1 Have you tried including https://github.com/nlohmann/json/blob/develop/include/nlohmann/detail/macro_scope.hpp? I don't know if it will work stand-alone, but it's a possibility. It should at least give some feedback on what would be needed in a standalone header. |
Hey @gregmarr |
@Ju1He1 that is an msvc limitation, which apparently is known by @StephanTLavavej and it is being worked on a "long term solution" for that issue (standard library includes coming after import std), although nothing has been done in that regard for almost two years now. It can be worked around by including all standard library headers through a pre-compiled header, thereby ensuring that all their include guards are set (which seems to be the issue with the redefinition errors that would otherwise occur). Not the prettiest workaround, but until Microsoft decides this is worth looking into, it is the best we can do to migrate the C++ ecosystem to modules. Because we as consumers cannot just fork all libraries we depend on and migrate them too, before we can use import std. |
@Optimierungswerfer |
Sorry - we've been drowning in other work. (I am currently the only maintainer able to spend time on MSVC's STL.) This is still on my priority list of things to do, but it will mostly be compiler work, and the compiler team has also been busy with other tasks. |
@Ju1He1 I did not try this with the other compilers, since we don't use them in our C++23 projects yet. My last knowledge is that msvc had the best module support so far. I can't tell whether the others caught up. This specific issue has been confirmed to be an msvc limitation though at some hard to find places (e.g. https://www.youtube.com/watch?v=Dk_C_E8AtRs&t=1990s or also a small side note in an old MSVC STL release note on GitHub) and unfortunately even the Microsoft support is confused about this as they told various people (myself included) multiple times that this was intended behavior, which led to even greater confusion. @StephanTLavavej I genuinely appreciate the clarification on this, thank you. Really looking forward to a solution there, because this has been holding my team back for years now. We optimistically tried to implement C++20 modules in our solution, since MSVC has been listed as compliant with modules for years but they never seemed to work as advertised, even in 2023 and until this day. Seeing the msvc compiler team working on C++23 and already asking for priorities for C++26, while a major feature of C++20 isn't fully done yet is a bit frustrating. |
Description
Importing a module file using the lib will cause a compilation error.
Reproduction steps
Codes below.
Expected vs. actual results
To compile successfully.
Minimal code example
network.ixx:
main.cpp:
Error messages
Compiler and operating system
Visual Studio 17 2022 MSVC 19.36.32323; Windows-10.0.22624
Library version
3.11.2
Validation
develop
branch is used.The text was updated successfully, but these errors were encountered: