Add missing #includes based on C++ language server.
Add missing #includes based on IWYU service.
Mostly this adds #include <nlohmann/detail/abi_macros.hpp> since NLOHMANN_JSON_NAMESPACE_BEGIN appears
nearly everywhere.
It adds a few other includes as well.
Honestly I don't see the benefit of including nlohmann/detail/abi_macros.hpp and nlohmann/thirdparty/hedley/hedley.hpp in files that are also including nlohmann/detail/macro_scope.hpp.
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md
Every .h file you bring in when compiling a source file lengthens the time to compile, as the bytes have to be read, preprocessed, and parsed. If you're not actually using a .h file, you remove that cost. With template code, where entire instantiations have to be in .h files, this can be hundreds of thousands of bytes of code. In one case at Google, running include-what-you-use over a .cc file improved its compile time by 30%.
Here, the main benefit of include-what-you-use comes from the flip side: "don't include what you don't use."
The thing this is mostly doing is increasing the compile time by adding extra includes of header files that are already included.
refactor could break code far away from you.
This is most compelling for a very large codebase (such as Google's). In a small codebase, it's practical to just compile everything after a refactor like this, and clean up any errors you see.
I would say that this is a small codebase, and not only is it practical to just compile it, it is basically required by the amalgamation.
Removing <iostream> and <sstream> from the tests is actually helpful, as it reduces the compile time, but there is then no reason to add <cstdint>, <vector>, <cassert>, <cstddef>.
I am also hesitant: it seems to fix a problem we don't have.
These includes follow the "Include-what-you-spell" principle; c++ language servers highlight these are missing.
RE macro_scope. Shouldn't each #include of that file be matched by an include of macro_unscope?
These includes follow the "Include-what-you-spell" principle; c++ language servers highlight these are missing.
This change violates the IWYU Faster compiles principle, and for many users of this header-only library, that's a more important principle than what opinion c++ language servers have about the details of the internal headers. If you use the single include version at https://github.com/nlohmann/json/blob/develop/single_include/nlohmann/json.hpp instead of the multiple include version, then your c++ language server won't even notice.
RE macro_scope. Shouldn't each #include of that file be matched by an include of macro_unscope?
No, it's included once at https://github.com/nlohmann/json/blob/develop/include/nlohmann/json.hpp#L5256. The headers in the detail directory are not designed to be included directly by end users.
I have removed most of the #includes changes, audited the use of macro_scope vs abi_macros to only include one or the other, and to avoid all includes of hedley outside of macro scope.
I added an IWYU pragma comment to macro_scope to make the language server happy.
The other include changes are for types used in files which have no exports.
PTAL
https://clangd.llvm.org/design/include-cleaner
Can you please rebase to the develop branch? I'll check this then.
This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions!