Skip to content

Wrap all external libs in namespace (was Consider wrapping rapidjson in a container namespace) #121

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

Closed
Iced-Sun opened this issue Sep 5, 2014 · 3 comments
Milestone

Comments

@Iced-Sun
Copy link

Iced-Sun commented Sep 5, 2014

Because both cereal and rapidjson are header-only and cereal contains a copy of a particular version of rapidjson, it will be conflicted if a project includes them both, unless the versions of rapidjson happen to be the same.

@AzothAmmo
Copy link
Contributor

Seems like a very reasonable thing for us to do. Will also apply to other external libs.

@AzothAmmo AzothAmmo changed the title Consider wrapping rapidjson in a container namespace Wrap all external libs in namespace (was Consider wrapping rapidjson in a container namespace) Sep 5, 2014
@AzothAmmo AzothAmmo added this to the v1.2.0 milestone Sep 28, 2014
@c42f
Copy link

c42f commented Jul 2, 2015

+1 for this - I have a project containing both rapidxml and rapidjson as existing dependencies.

What is the preferred implementation? One option would be to go through rapidjson and wrap all occurrences of namespace rapidjson with

RAPIDJSON_WRAP_NAMESPACE_ENTER
namespace rapidjson {

// ...

} // namespace rapidjson
RAPIDJSON_WRAP_NAMESPACE_EXIT

In rapidjson.h:

#ifdef RAPIDJSON_WRAPPER_NAMESPACE
#   define RAPIDJSON_WRAP_NAMESPACE_ENTER namespace RAPIDJSON_WRAPPER_NAMESPACE {
#   define RAPIDJSON_WRAP_NAMESPACE_EXIT }
#else
#   define RAPIDJSON_WRAP_NAMESPACE_ENTER
#   define RAPIDJSON_WRAP_NAMESPACE_EXIT
#endif

This is potentially something that could be submitted to rapidjson upstream rather than being a cereal-specific hack. Usage in cereal as follows:

#define RAPIDJSON_WRAPPER_NAMESPACE cereal
#include <cereal/external/rapidjson/prettywriter.h>
#include <cereal/external/rapidjson/genericstream.h>
// ...
#undef RAPIDJSON_WRAPPER_NAMESPACE

AzothAmmo added a commit that referenced this issue Dec 22, 2015
Covers base64 and rapidxml, still need to do json
see #121
@AzothAmmo
Copy link
Contributor

Until we upgrade rapidjson/xml to newer versions I'm implementing a quick fix to this by shoving CEREAL_ on the preprocessor stuff and throwing everything else in the cereal namespace.

AzothAmmo added a commit that referenced this issue Apr 19, 2016
Updated and made changes necessary for the new version of rapidjson.
Looks good on ubuntu under the compilers I can test with, needs MSVC testing.
We had some internal changes to rapidjson but these didn't seem necessary with
the new version. Haven't done any performance testing, initial estimates put it at
nearly the same speed for json serialization. Could probably optimize things.

relates #82, #121
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants