Repository navigation
Consider putting the user-defined literals in a namespace #1682
Description
If I'm not mistaken, the user-defined literals are outside the nlohmann namespace. This means if the header is included twice, there will be a conflict. My use-case (or rather the library I'm contributing to) looks something like this:
// Remove the include guards because third-parties may have included their own
// version of nlohmann::json.
#undef NLOHMANN_JSON_HPP
#undef NLOHMANN_JSON_FWD_HPP
#define nlohmann library_internal_nlohmann_3_4_0
#include <nlohmann/json.hpp>The idea is that because the namespace is renamed to something that will not conflict, the library can be included multiple times. Previously, we would grab nlohmann_json as an external project and patch the user-defined literals out; however since we are moving to using find_package only, this is not possible (or would be ugly).
There is a downside to this: it would obviously break backwards compatibility. However, including it would be simple if it looked something like this:
namespace nlohmann {
inline namespace literals {
// ... define literals here
}
}Then in user-code:
using namespace nlohmann::literals; // does not pollute global namespace
I am only 50/50 for this idea and would like to hear your thoughts.
Activity
nlohmann commentedon Jul 23, 2019
I understand the issue, and #1539 could be part of the solution. However, removing the literal operator from the default public API would be a breaking change, right?
MikeGitb commentedon Jul 29, 2019
Personally, I think putting UDLs into the global namespace (in a header) is a no-go anyway. Especially, if they have very generic names like
_json.stale commentedon Aug 28, 2019
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
MikeGitb commentedon Aug 28, 2019
I don't think this should be closed until a decision for or against is made.
t-b commentedon Aug 28, 2019
@MikeGitb I'm (random json.hpp user) in favour for putting the UDLs into their own namespace. fmt does that as well, see https://fmt.dev/latest/api.html?highlight=literal#literal-based-api.
MikeGitb commentedon Aug 28, 2019
So am I
t-b commentedon Aug 28, 2019
How about the following changes:
JSON_LITERALS_SEPARATEdefine to choose if UDLs are in the global namespace (undefined, default) or not.So something like
in the global namespace.
Edited: Thanks @MikeGitb
MikeGitb commentedon Aug 28, 2019
I think you have a typo or something like that in your snippet.
That will put the UDLs unconditionally into the global namespace.
stale commentedon Sep 27, 2019
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
1 remaining item
falbrechtskirchinger commentedon Jul 24, 2022
#3590 doesn't fix this issue. I propose we follow the standard library approach to the letter: https://en.cppreference.com/w/cpp/chrono/operator%22%22h#Notes
Additionally, introduce the macro
JSON_USE_GLOBAL_UDLSwhich restores the current behavior.Edit: Removed the unnecessary
using namespacepointed out by @MikeGitb here and in PR #3605.MikeGitb commentedon Jul 24, 2022
If you are already using inline namespaces, you don't need the
using namespace literals::json_literals;do you? The reason the linked cppreference.com article mentions ausing literals::chrono_literalsis to import the litterals into a sibling namespace (std::chrono) and not just instd.falbrechtskirchinger commentedon Jul 24, 2022
You're right. I'll apply the change when I can.