Consider putting the user-defined literals in a namespace

[ghost]

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.

[nlohmann]

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?

[ghost]

However, removing the literal operator from the default public API would be a breaking change, right?

Indeed, which probably means I don't expect this change until the next major release.

[MikeGitb]

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]

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]

I don't think this should be closed until a decision for or against is made.

[t-b]

@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]

So am I

[t-b]

How about the following changes:

• Move UDLs into nlohmann::json::literals namespace
• Introduce JSON_LITERALS_SEPARATE define to choose if UDLs are in the global namespace (undefined, default) or not.

So something like

#ifndef JSON_LITERALS_SEPARATE
using namespace nlohmann::json::literals;
#endif // JSON_LITERALS_SEPARATE

in the global namespace.

Edited: Thanks @MikeGitb

[MikeGitb]

I think you have a typo or something like that in your snippet.
That will put the UDLs unconditionally into the global namespace.

[stale]

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.

[falbrechtskirchinger]

#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

NLOHMANN_JSON_NAMESPACE_BEGIN
inline namespace literals {
inline namespace json_literals {
// define UDLs here
}
}
NLOHMANN_JSON_NAMESPACE_END

Additionally, introduce the macro JSON_USE_GLOBAL_UDLS which restores the current behavior.

#if JSON_USE_GLOBAL_UDLS
using namespace nlohmann::literals::json_literals;
#endif

Edit: Removed the unnecessary using namespace pointed out by @MikeGitb here and in PR #3605.

[MikeGitb]

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 a using literals::chrono_literals is to import the litterals into a sibling namespace (std::chrono) and not just in std.

[falbrechtskirchinger]

You're right. I'll apply the change when I can.