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

detail namespace collision with Cereal? #1082

Closed
RPGillespie6 opened this issue May 7, 2018 · 6 comments
Closed

detail namespace collision with Cereal? #1082

RPGillespie6 opened this issue May 7, 2018 · 6 comments

Comments

@RPGillespie6
Copy link

RPGillespie6 commented May 7, 2018

I'm seeing a very strange bug when using your library in conjunction with C++ Cereal: https://github.com/USCiLab/cereal

Reproduce with the following:

#include <json.hpp>

#include <cereal/types/polymorphic.hpp>
#include <cereal/archives/binary.hpp>
#include <cereal/access.hpp>       //So we can make serialize private so developers aren't tempted to call it.
#include <cereal/types/string.hpp> //This is needed to serialize std::string. There are similar ones for the other std containers
#include <cereal/types/vector.hpp> //This is needed to serialize std::vector. There are similar ones for the other std containers

using namespace std;

//Pure virtual base class
class Serializable
{
    public:
        virtual int getType() = 0;

    protected:
        template<class Archive> void serialize(Archive & ar);
};

class Bug: public Serializable
{
    public:
        std::string text;
        int getType() {return 1;};
        void load(const nlohmann::json & s) {};

    private:
        friend class cereal::access;
        template <class Archive> void serialize(Archive &ar) {ar(text);};
};

// Register Bug
CEREAL_REGISTER_TYPE(Bug);
CEREAL_REGISTER_POLYMORPHIC_RELATION(Serializable, Bug);

int main()
{
    auto obj = make_shared<Bug>();

    std::ostringstream os;
    {
        cereal::BinaryOutputArchive oarchive(os);
        oarchive(dynamic_pointer_cast<Serializable>(obj));
    }

    shared_ptr<Serializable> obj2;
    std::istringstream is(os.str());
    {
        cereal::BinaryInputArchive iarchive(is);
        iarchive(obj2);
    }

    auto m = dynamic_pointer_cast<Bug>(obj2);

    return 0;
}

This code, when compiled with the latest nlohmann::json throws the following:

Trying to load an unregistered polymorphic type (Bug).
Make sure your type is registered with CEREAL_REGISTER_TYPE and that the archive you are using was included (and registered with CEREAL_REGISTER_ARCHIVE) prior to calling CEREAL_REGISTER_TYPE.
If your type is already registered and you still see this error, you may need to use CEREAL_REGISTER_DYNAMIC_INIT.

Curiously though, compiling against a 2.X version of nlohmann::json does not produce this exception.

I noticed this only happens when the method in the Bug class is load, which is why I'm opening an issue with cereal as well.

I noticed one change from nlohmann::json 2.X->3.X is the inclusion of the nlohmann::detail namespace. Could this be somehow conflicting with cereal::detail? I will also open an issue against cereal as well.

Still trying to figure out root cause of this. It may not be your library, just trying to figure out why I can no longer use nlohmann::json with Cereal when I upgrade from 2.X to 3.X

Cereal issue: USCiLab/cereal#499

@theodelrieu
Copy link
Contributor

The detail namespace was introduced in 2.1.0 IIRC, and unless you are doing something like:

using namespace cereal;
using namespace nlohmann;

I don't see how that could be a problem, lots of libraries have a detail namespace.

Unfortunately I've never used cereal, let's hope you get more info on the other issue!

@RPGillespie6
Copy link
Author

I've confirmed that the break occurs precisely from 2.0.10 -> 2.1.0.

The following minimum example compiles under nlohmann::json version 2.0.10 but does not compile under 2.1.0:

#include <json.hpp>

#include <cereal/archives/binary.hpp>
#include <cereal/access.hpp>       //So we can make serialize private so developers aren't tempted to call it.
#include <cereal/types/string.hpp> //This is needed to serialize std::string. There are similar ones for the other std containers
#include <cereal/types/vector.hpp> //This is needed to serialize std::vector. There are similar ones for the other std containers

using namespace std;

class Bug
{
    public:
        std::string text;
        void load(const nlohmann::json & s) {};

    private:
        friend class cereal::access;
        template <class Archive> void serialize(Archive &ar) {ar(text);};
};


int main()
{
    Bug obj;
    std::ostringstream os;
    {
        cereal::BinaryOutputArchive oarchive(os);
        oarchive(obj);
    }

    Bug obj2;
    std::istringstream is(os.str());
    {
        cereal::BinaryInputArchive iarchive(is);
        iarchive(obj2);
    }

    return 0;
}

Note that under 2.1.0 changing the name of Bug::load to Bug::loadJson also resolves the issue. Definitely some kind of collision occurring here, still trying to find root cause.

@RPGillespie6
Copy link
Author

RPGillespie6 commented May 10, 2018

Update: I've confirmed it is definitely not a namespace collision. I renamed detail to an arbitrary name and I still see the same problem. Instead, it looks like somehow Cereal thinks the nlohmann::json type is a cereal::Archive such that when passed into a function named load it thinks I am registering a new load/save function. Unfortunately both libraries are extremely template heavy so it's hard to understand what's going on

@nlohmann
Copy link
Owner

This sounds like it is more an issue on Cereal's side. I'm note sure how to proceed here.

@RPGillespie6
Copy link
Author

RPGillespie6 commented May 27, 2018

Something in the transition from 2.0.10 -> 2.1.0 seems to have broken cereal. I tried to debug, but cereal is an extremely templated library and I was having a hard time tracking down the root cause of the break. I found a workaround and moved on. We can go ahead and close this issue and I will only reopen if I conclusively find evidence it is your library's fault.

@nlohmann
Copy link
Owner

Thanks!

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

3 participants