Skip to content
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

json_fwd.hpp no longer standalone #3656

Closed
2 tasks
michaeldurland opened this issue Aug 2, 2022 · 4 comments · Fixed by #3679
Closed
2 tasks

json_fwd.hpp no longer standalone #3656

michaeldurland opened this issue Aug 2, 2022 · 4 comments · Fixed by #3679
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@michaeldurland
Copy link

Description

json_fwd.hpp now has a line:
#include <nlohmann/detail/abi_macros.hpp>
which means this file is no longer standalone. This makes it no longer possible to have a simple 2-file installation, with just json_fwd.hpp and the large single json.hpp.
This seems to be related to #3590

Reproduction steps

json_fwd.hpp now has a new external file dependency.

Expected vs. actual results

Expected:
Be able to have json_fwd.hpp standalone with no dependencies.
Actual:
json_fwd.hpp has an include file, so it can't be used by itself anymore.

Minimal code example

any code that wants to only do:
#include "json_fwd.hpp"

Error messages

can't find include file 'nlohmann/detail/abi_macros.hpp'

Compiler and operating system

any

Library version

3.11.1

Validation

@nlohmann
Copy link
Owner

nlohmann commented Aug 2, 2022

Just to understand your use-case better: do you have parts of your code that use json_fwd.hpp, but have no access to the other library headers?

@gregmarr
Copy link
Contributor

gregmarr commented Aug 3, 2022

As far as I can tell (see #700), it was never the goal of this file to make it a "standalone" file but just to make it a smaller header that handles the forward declaration of the class, for files that just want to be able to pass the objects around, but not to operate on them.

I assume that you are copying json_fwd.hpp from the include/json directory to the single_include directory. Is that accurate? Is it a significant burden to copy the abi_macros.hpp file as well? Is it the nlohmann/detail directory that's the issue? It might be possible to embed the contents of the header into json_fwd.hpp and include that instead everywhere that abi_macros.hpp is currently included.

@falbrechtskirchinger Thoughts?

@falbrechtskirchinger
Copy link
Contributor

I'm not sure where I read it, but I was under the impression that json_fwd.hpp was in fact meant to be standalone. So, big derp on my part.
Maybe we should just amalgamate json_fwd.hpp? I.e., add a step to make amalgamate and just output it to single_include/nlohmann/json_fwd.hpp. Then we need to remember to add that file to the release scripts (to replace the version number).

@michaeldurland
Copy link
Author

michaeldurland commented Aug 3, 2022

Just to understand your use-case better: do you have parts of your code that use json_fwd.hpp, but have no access to the other library headers?

Yes, my workflow is to download the package .tar.gz file, and copy the json_fwd.hpp and single_include/json.hpp files into my source tree. Now that becomes more difficult with this extra dependency. And yes, I have places that only include json_fwd.hpp in their .h headers in order to reduce dependencies and compile time, when parsing the full json.hpp is not needed, but then later down in the source tree include the full json.hpp in the .cpp source.

So perhaps one way to reframe the original description would be something more like a request for an amalgamated single_include/json_fwd.hpp. This can be in addition to the newly update json_fwd.hpp that has the extra include.
I think this is along the lines of what @falbrechtskirchinger commented on to be a possibility. This would be sufficient and great. Thanks!

@nlohmann nlohmann linked a pull request Aug 7, 2022 that will close this issue
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Aug 7, 2022
@nlohmann nlohmann added this to the Release 3.11.2 milestone Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants