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

Register of chainable relations fix #335

Merged
merged 3 commits into from
Oct 11, 2016

Conversation

dlardi
Copy link
Contributor

@dlardi dlardi commented Aug 25, 2016

There is a case when we could find several different relations to write to 'unregisteredRelations' map with one 'otherBase'.

@AzothAmmo
Copy link
Contributor

Can you provide pseudo code or real code for an example demonstrating this so it can be added to a test case as well?

@AzothAmmo AzothAmmo added this to the v1.3.0 milestone Aug 25, 2016
@dlardi
Copy link
Contributor Author

dlardi commented Aug 30, 2016

Example code:

struct A
{
  virtual ~A() {}
  template <class Archive>
  void serialize( Archive & )
  {
  }
};

struct B : A
{
  template <class Archive>
  void serialize( Archive & ar )
  {
    ar( cereal::base_class<A>(this) );
  }
};

struct C : B
{
  template <class Archive>
  void serialize( Archive & ar )
  {
    ar( cereal::base_class<B>( this ) );
  }
};

struct D : C
{
  template <class Archive>
  void serialize( Archive & ar )
  {
    ar( cereal::base_class<C>( this ) );
  }
};

CEREAL_REGISTER_TYPE(B)
CEREAL_REGISTER_TYPE(D)

template <class IArchive, class OArchive>
void test_polymorphic_relations()
{
  std::shared_ptr<A> o_shared = std::make_shared<D>();
  std::ostringstream os;
  {
    OArchive oar(os);
    oar( o_shared);
  }
}

std::multimap instead of std::map for unregistered relations will save some iterations. To check this you could extend the above hierarchy to say G class.

@AzothAmmo AzothAmmo modified the milestones: v1.2.2, v1.3.0 Sep 21, 2016
@AzothAmmo AzothAmmo merged commit c83a7f9 into USCiLab:develop Oct 11, 2016
@AzothAmmo
Copy link
Contributor

Made some small changes when I merged this. I don't think the multimap is necessary, since it seems it was used to catch multiple potential paths during each iteration, when we really only desire the shortest new path. So went back to a map, but now check to see if we already have a shorter path we haven't commit to the main map we can replace.

I think your do-while loop was what was catching most of the missed paths.

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

Successfully merging this pull request may close these issues.

2 participants