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

Put rapidjson under cereal namespace #534

Open
oliora opened this issue Nov 20, 2018 · 6 comments
Open

Put rapidjson under cereal namespace #534

oliora opened this issue Nov 20, 2018 · 6 comments

Comments

@oliora
Copy link

oliora commented Nov 20, 2018

Before the last update of RapidJSON, it was placed under cereal::rapidjson namespace to avoid collisions with other versions of RapidJSON in the same binary, which was very handy. Unfortunately, it's not the case anymore, now Cereal's RapidJSON is defined under its default namespace rapidjson so it conflicts with other RapidJSONs. I propose to put cereal::rapidjson namespace back.

The fix is just to update three lines in include/cereal/external/rapidjson/rapidjson.h. I can send a PR if needed.

@AzothAmmo
Copy link
Contributor

You can provide a definition for CEREAL_RAPIDJSON_NAMESPACE_BEGIN in include/cereal/external/rapidjson/rapidjson.h to override the default behavior. That file has additional explanation on this as well.

@oliora
Copy link
Author

oliora commented Jan 2, 2019

@AzothAmmo thank you! I did this already for myself but my point is that Cereal should be shipped with rapidjson under cereal namespace by default. The purpose of this is to avoid collision with target project’s rapidjson definitions, which is not a rare case since rapidjson is so widely used.

@h-2
Copy link
Contributor

h-2 commented Apr 30, 2019

IMHO it should not be shipped internally at all. This is what we have git-submodules, build systems and package managers for.

@Laro88
Copy link

Laro88 commented Sep 19, 2019

Please fix this "I include and mess up library x" issue, it is impossible to use both rapidjson and cereal in vcpkg since cereal has adds a lot of mods in rapidjson.

@dimztimz
Copy link

dimztimz commented Dec 4, 2019

A library should not bundle/package other libraries, otherwise we can very easily get the diamond problem and ODR violations.

For example, on Ubuntu or Debian, if you install libcereal-dev rapidjson-dev and try to use them both, you will get compilation errors. See this sample code.

#include <cereal/archives/json.hpp>
#include <rapidjson/document.h>

int main() {}

The real solution is to use CMake's find_package(), and rely on cmake to find system's rapidjson.

Other solution is to define CEREAL_RAPIDJSON_NAMESPACE_BEGIN and the other two defines by default to put rapidjson under the namespace of cereal.

@channingko-madden
Copy link

If #604 addresses this would really appreciate it getting merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants