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

Multiple versions causing conflict #3588

Closed
2 tasks
nathanieltagg opened this issue Jul 18, 2022 · 10 comments · Fixed by #3590
Closed
2 tasks

Multiple versions causing conflict #3588

nathanieltagg opened this issue Jul 18, 2022 · 10 comments · Fixed by #3590
Assignees
Labels
kind: bug release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@nathanieltagg
Copy link

Description

I'm using a 3rd party library that uses nlohmann/json under the hood. I am also using it in my code.

I have found that unless these two versions are exactly the same, there are side effects that cause the code not to function in the 3rd party library. (The failure happens with 3.9.2 in the 3rd party library and 3.10.5 in mine.) The exact failure I can't pinpoint, because it happens deep in code I'm not famililar with, but it causes no build-time error or crash, but DOES cause incorrect behavior that appears to be null JSON objects.

Keeping my version matched to the library gives me temporary respite, but is a pain to maintain, and becomes unsolvable if it happpens with yet another library.

The problem is clearly a collision in object code resolution. Could we have it customizable to change from nlohmann to MYNAMESPACE::nlhomann? Wrapping the header file in a namespace doesn't work for a variety of reasons. I CAN do a very cheap hack by doing
#define nlhomann my_nlhomann
but I suspect that might not be very sustainable.

Or perhaps there is another solution that I haven't found online?

Reproduction steps

The 3rd party library in question is depthai-core, in a dependency.

Expected vs. actual results

The exact throw is "cannot use at() with null", but this is due to a previous problem.

Minimal code example

No response

Error messages

No response

Compiler and operating system

Linux gcc

Library version

3.9.2, 3.10.5

Validation

@falbrechtskirchinger
Copy link
Contributor

What about a versioned inline namespace? (@nlohmann)

@nlohmann
Copy link
Owner

PRs welcome. We had similar issues in the past, but found no solution.

@falbrechtskirchinger
Copy link
Contributor

I might try it later, but it'll need some support from your release scripts to bump the namespace version.

@nathanieltagg
Copy link
Author

Update... the quick-and-dirty

#define nlohmann my_nlohmann

does in fact work as expected, which I'm making note of in case someone googles and finds this report. It's a reasonable stopgap solution. But I agree; if the header file used

  namespace nlohmann_3_10_5 {...}
  namespace nlohmann = nlohman_3_10_5;

then that would easily solve the problem foreevermore. It makes your code uglier, since you'll have to use a macro or something to define the namespace, but otherwise I think it would be a boon: it took FOREVER for me to figure out that out of all the things going on in my code it was the JSON header-only library...

@falbrechtskirchinger
Copy link
Contributor

falbrechtskirchinger commented Jul 18, 2022

But I agree; if the header file used

  namespace nlohmann_3_10_5 {...}
  namespace nlohmann = nlohman_3_10_5;

then that would easily solve the problem forevermore.

That's not quite what I'm suggesting.

namespace nlohmann {
inline namespace v3_10_5 {
// ...
}
}

Nothing changes for end users unless there's an ambiguity because two differently-versioned headers are included in the same translation unit.

See https://en.cppreference.com/w/cpp/language/namespace#Inline_namespaces for an explanation.

@gregmarr
Copy link
Contributor

Are you having this issue because two different versions of the library headers are being included in the same file (translation unit) or because two different files (translation units) with different versions are linked together? The former is now prevented by #3418 in the develop branch.

@falbrechtskirchinger
Copy link
Contributor

Are you having this issue because two different versions of the library headers are being included in the same file (translation unit) or because two different files (translation units) with different versions are linked together? The former is now prevented by #3418 in the develop branch.

Based on the proposed workaround, I'm assuming this is about linking different versions together. An inline namespace would result in different symbol names. JSON_DIAGNOSTICS would still pose a problem, but could be solved with an inline namespace as well, as mentioned during one of the last ABI-related discussions.

@nlohmann
Copy link
Owner

nlohmann commented Jul 18, 2022

FYI, we'll soon check whether incompatible versions are used together, see https://json.nlohmann.me/api/macros/json_skip_library_version_check/#runtime-assertions. That is, 3.11.0 will detect when it is used together with earlier versions.

Edit: @gregmarr was faster, see #3588 (comment)

@gregmarr
Copy link
Contributor

gregmarr commented Jul 18, 2022

@nlohmann That only helps with including two different versions of the header in the same file. It doesn't help with ODR violations that occur at link time when you use different versions in different files. There are things that can help with that too, but we're not doing any of them yet. One is the inline namespaces mentioned above.

Another MSVC specific one is https://docs.microsoft.com/en-us/cpp/preprocessor/detect-mismatch?view=msvc-170
This can also be used to detect one file built with diagnostics, and one without. It will only help going forward as it can only detect when the pragmas exist and are different between files. It won't prevent previous versions being used together or with the new version. I haven't found an equivalent functionality in clang/gcc.

Edit: looks like clang supports it too.

@nathanieltagg
Copy link
Author

@gregmarr The problem is specifically two object files that use diffent versions of the header-only json, so that the wrong code gets executed leading to bad data. The inlined versioning looks like it would work, and is much more elegant than my brute force #define

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation release item: 🔨 further change labels Jul 23, 2022
@nlohmann nlohmann added this to the Release 3.11.0 milestone Jul 23, 2022
@nlohmann nlohmann self-assigned this Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🔨 further change 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