Add library versioning using inline namespaces

[ZbigniewRA]

Using two versions of this library in the same project causes segfaults. This is expected, but could be avoided by using inline namespaces for library versioning.

For example Google Benchmark (from v2 branch) uses this library in version 3.0.1, my program uses version 3.3.0. This causes crashes.

A solution would be to wrap all code with version specific namespaces to disambiguate them, like this:

namespace nlohmann {
    inline namespace json_3_3_0 {
    }
}

[nlohmann]

I am not sure how exactly to approach this issue. Will this break existing code? Will two such versioned libraries be able to co-exist?

[jaredgrubb]

I'm interested in this approach, but I'm very curious if anyone knows of a project that successfully uses this approach.

For example, libc++ uses this approach to avoid its symbols getting mixed up with libstdc++ (in case you link two libs together that were built against a different STL). They add an inline "__1" namespace, but I'll note that there's never been a __2.

More specifically: I'd love to know if any project uses this approach, has actually ever incremented the number -- and, most importantly, is happy with how it has worked out.

[nlohmann]

Maybe related: #813.

[ZbigniewRA]

Usage of inline namespaces does seem to be low, but I'd risk a guess this is due to people not being familiar with them. I haven't heard of any problems with them.

Applying them would be quite simple. Just change all namespace nlohmann { to namespace nlohmann { inline namespace json_3_3_0 {.

The results would be as follows:

• All symbols (as seen by the linker) will have json_3_3_0 added to them. This will guarantee no mismatch of symbols from different versions, and ability of different versions of the library to coexist.
• json_3_3_0 namespace will not change anything from the user's point of view. No code changes will be necessary in clients. ADL will work as it does now. User specialized templates inside nlohmann namespace (like adl_serializer) will work without any changes.
• User's will be able to target specific version of the library by explicitly importing from json_3_3_0 namespace.

Note: I do not suggest to put many versions of json library in one header. While it's possible, I don't see much of a benefit in that.

A short introduction to inline namespaces is here: https://foonathan.net/blog/2018/11/22/inline-namespaces.html

Google Benchmark library intended to (but didn't yet) rename nlohmann namespace in it's copy of json library to benchmark in order to avoid conflicts with user code. Inline namespaces will solve this problem without need of renaming anything.

I will try to prepare a PR for this.

[theodelrieu]

Hello, I would like to know which symbol causes the crash in the first place, was it a subtle API breaking change? ODR violation caused by an API compatible but ABI breaking one?

I'm supporting the idea of having a single inline namespace for major versions (this is what I've always seen in libraries, e.g. ranges-v3, libcxx). I'm skeptical about having one namespace for each version (minor and patch included). But for your specific use-case, you want to have ABI-related inline namespaces as mentioned in the blog post you linked.

I would rather see something like:

namespace nlohmann {
inline namespace v3 {
inline namespace abi_v2 {
class stuff {};
}
}
}

I've never used ABI inline namespaces though, you should take it with a grain of salt

[ZbigniewRA]

The crash was in simple json.dump(). There was no API incompatibility I believe - pure ABI break.
I propose introducing inline namespaces specifically to guard against ABI incompatibility.

As for introducing two inline namespaces (one for API changes, one for ABI changes) I'm not sure it's needed. It would be difficult to track ABI changes. Using full release version for namespace name is the simplest solution in this case.
A compromise would be to use this scheme:

namespace nlohmann {
inline namespace v3 {
inline namespace v3_3_0 {

But personally I would keep just the "v3_3_0" namespace.

[theodelrieu]

One of the initial purposes of inline namespaces is to allow users to explicitly choose an old behavior when a new major version is introduced:

// imagine v4 removed the deprecated stream operations
nlohmann::v3::json j;
std::istream& s = /*...*/;

s >> j;

With your proposed change, this is not possible anymore, since there is a single namespace that is bumped on every release (major, minor and patch).

Having the two inline namespaces solves that, v3 will stop becoming inline when v4 is released, but users can still explicitly use it.

As for the always-inline one, whether it is abi_vX or the vX_Y_Z is more of a stylistic choice than something else.

[theodelrieu]

That being said, I wonder why benchmark does not hide json's symbols, IIRC it's used internally so it should not be exposed to its consumers...

[theodelrieu]

I'd like to avoid having an ABI inline namespace to be honest

[nlohmann]

As the library is already quite large and we only have limited resources, I would like to avoid having a single header with multiple versions of the library in it. I'm not sure how to proceed here.

[ZbigniewRA]

My proposal is to have just one version in the header.
The inline namespace would simply prevent mismatching different versions of json library during linking with third-party libraries.

If anyone would want to use different versions of json API in different places of his code he could always do this:

// a.cpp
#include <nlohmann/json_2.0/json.hpp>
...

// b.cpp
#include <nlohmann/json_3.0/json.hpp>
...

I think the only argument against my idea is that, as theodelrieu said, it should be responsibility of a third-party library to hide any symbols it doesn't need to expose publicly.
But this is not a common practice (as Google Benchmark example shows). It's much safer to just protect against it.

[prazek]

If more libs would use inline namespace in this way then it would be much simpler to resolve such conflicts, so I agree with this idea.

[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.

[ghost]

Is this implemented? It says in the changelog but I can't find the associated PR or code.

[nlohmann]

It was not implemented. The issue is mentioned in the generated ChangeLog, because it was closed since the previous version.

[MikeGitb]

I'd like to see this too.
Unless one promises to not break ABI within a major version, the namespace needs to contain the full version name, but I don't see a problem with this (except one mist must remember to always update it consistently).

Ironically, the fact that it is so easy to integrate this library into your project via copy paste, is exactly what makes it more likely that you'll end up with conflicting versions.

[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.