diff --git a/libsrc/core/basearchive.hpp b/libsrc/core/basearchive.hpp index a97cd28c..79e4eaf9 100644 --- a/libsrc/core/basearchive.hpp +++ b/libsrc/core/basearchive.hpp @@ -150,17 +150,21 @@ namespace ngcore void* reg_ptr = ptr.get(); bool neededDowncast = false; - if constexpr(has_DoArchive::value) - { - if(GetArchiveRegister().count(std::string(typeid(*ptr).name())) == 0) - throw std::runtime_error(std::string("Archive error: Polymorphic type ") - + typeid(*ptr).name() - + " not registered for archive"); - else - reg_ptr = GetArchiveRegister()[typeid(*ptr).name()].downcaster(typeid(T), ptr.get()); - if(reg_ptr != (void*) ptr.get()) - neededDowncast = true; - } + // Downcasting is only possible for our registered classes + if(typeid(T) != typeid(*ptr)) + { + if constexpr(has_DoArchive::value) + { + if(GetArchiveRegister().count(std::string(typeid(*ptr).name())) == 0) + throw std::runtime_error(std::string("Archive error: Polymorphic type ") + + typeid(*ptr).name() + + " not registered for archive"); + reg_ptr = GetArchiveRegister()[typeid(*ptr).name()].downcaster(typeid(T), ptr.get()); + // if there was a true downcast we have to store more information + if(reg_ptr != (void*) ptr.get()) + neededDowncast = true; + } + } auto pos = shared_ptr2nr.find(reg_ptr); // if not found store -1 and the pointer if(pos == shared_ptr2nr.end()) @@ -168,6 +172,7 @@ namespace ngcore auto p = ptr.get(); (*this) << -1; (*this) & neededDowncast & p; + // if we did downcast we store the true type as well if(neededDowncast) (*this) << std::string(typeid(*ptr).name()); shared_ptr2nr[reg_ptr] = shared_ptr_count++; @@ -183,22 +188,27 @@ namespace ngcore { int nr; (*this) & nr; + // -2 restores a nullptr if(nr == -2) { ptr = nullptr; return *this; } + // -1 restores a new shared ptr by restoring the inner pointer and creating a shared_ptr to it else if (nr == -1) { T* p; bool neededDowncast; (*this) & neededDowncast & p; ptr = std::shared_ptr(p); + // if we did downcast we need to store a shared_ptr to the true object if(neededDowncast) { std::string name; (*this) & name; auto info = GetArchiveRegister()[name]; + // for this we use an aliasing constructor to create a shared pointer sharing lifetime + // with our shared ptr, but pointing to the true object nr2shared_ptr.push_back(std::shared_ptr(std::static_pointer_cast(ptr), info.downcaster(typeid(T), ptr.get()))); @@ -213,11 +223,15 @@ namespace ngcore (*this) & neededDowncast; if(neededDowncast) { + // if there was a downcast we can expect the class to be registered (since archiving + // wouldn't have worked else) if constexpr(has_DoArchive::value) { std::string name; (*this) & name; auto info = GetArchiveRegister()[name]; + // same trick as above, create a shared ptr sharing lifetime with + // the shared_ptr in the register, but pointing to our object ptr = std::static_pointer_cast(std::shared_ptr(other, info.upcaster(typeid(T), other.get()))); @@ -240,21 +254,20 @@ namespace ngcore { // if the pointer is null store -2 if (!p) - { - int m2 = -2; - (*this) & m2; - return *this; - } + return (*this) << -2; void* reg_ptr = (void*)p; - if constexpr(has_DoArchive::value) - { - if(GetArchiveRegister().count(std::string(typeid(*p).name())) == 0) - throw std::runtime_error(std::string("Archive error: Polimorphic type ") - + typeid(*p).name() - + " not registered for archive"); - else - reg_ptr = GetArchiveRegister()[typeid(*p).name()].downcaster(typeid(T), p); - } + if(typeid(T) != typeid(*p)) + { + if constexpr(has_DoArchive::value) + { + if(GetArchiveRegister().count(std::string(typeid(*p).name())) == 0) + throw std::runtime_error(std::string("Archive error: Polymorphic type ") + + typeid(*p).name() + + " not registered for archive"); + else + reg_ptr = GetArchiveRegister()[typeid(*p).name()].downcaster(typeid(T), p); + } + } auto pos = ptr2nr.find(reg_ptr); // if the pointer is not found in the map create a new entry if (pos == ptr2nr.end()) @@ -270,11 +283,14 @@ namespace ngcore typeid(*p).name() + " does not provide a default constructor!"); else { - // We want this special behaviour only for our classes that implement DoArchive + // if a pointer to a base class is archived, the class hierarchy must be registered + // to avoid compile time issues we allow this behaviour only for "our" classes that + // implement a void DoArchive(Archive&) member function + // To recreate the object we need to store the true type of it if constexpr(has_DoArchive::value) { if(GetArchiveRegister().count(std::string(typeid(*p).name())) == 0) - throw std::runtime_error(std::string("Archive error: Polimorphic type ") + throw std::runtime_error(std::string("Archive error: Polymorphic type ") + typeid(*p).name() + " not registered for archive"); else @@ -289,18 +305,18 @@ namespace ngcore else { (*this) & pos->second; - (*this) << std::string(typeid(*p).name()); + bool downcasted = !(reg_ptr == (void*) p); + // store if the class has been downcasted and the name + (*this) << downcasted << std::string(typeid(*p).name()); } } else { int nr; (*this) & nr; - if (nr == -2) - { + if (nr == -2) // restore a nullptr p = nullptr; - } - else if (nr == -1) + else if (nr == -1) // create a new pointer of standard type (no virtual or multiple inheritance,...) { if constexpr (std::is_constructible::value) { @@ -312,15 +328,19 @@ namespace ngcore throw std::runtime_error("Class isn't registered properly"); } - else if(nr == -3) + else if(nr == -3) // restore one of our registered classes that can have multiple inheritance,... { - // We want this special behaviour only for our classes that implement DoArchive + // As stated above, we want this special behaviour only for our classes that implement DoArchive if constexpr(has_DoArchive::value) { std::string name; (*this) & name; auto info = GetArchiveRegister()[name]; + // the creator creates a new object of type name, and returns a void* pointing + // to T (which may have an offset) p = (T*) info.creator(typeid(T)); + // we store the downcasted pointer (to be able to find it again from + // another class in a multiple inheritance tree) nr2ptr.push_back(info.downcaster(typeid(T),p)); (*this) & *p; } @@ -329,13 +349,18 @@ namespace ngcore } else { + bool downcasted; std::string name; - (*this) & name; - if constexpr(has_DoArchive::value) - { - auto info = GetArchiveRegister()[name]; - p = (T*) info.upcaster(typeid(T), nr2ptr[nr]); - } + (*this) & downcasted & name; + if(downcasted) + { + // if the class has been downcasted we can assume it is in the register + if constexpr(has_DoArchive::value) + { + auto info = GetArchiveRegister()[name]; + p = (T*) info.upcaster(typeid(T), nr2ptr[nr]); + } + } else p = (T*) nr2ptr[nr]; } diff --git a/tests/catch/archive.cpp b/tests/catch/archive.cpp index d34e3878..dc31be16 100644 --- a/tests/catch/archive.cpp +++ b/tests/catch/archive.cpp @@ -6,10 +6,11 @@ using namespace std; class CommonBase { - public: +public: + int a; virtual ~CommonBase() {} - virtual void DoArchive(Archive& archive) { } + virtual void DoArchive(Archive& archive) { archive & a; } }; class SharedPtrHolder : virtual public CommonBase @@ -19,7 +20,11 @@ public: virtual ~SharedPtrHolder() { } - virtual void DoArchive(Archive& archive) { archive & names; } + virtual void DoArchive(Archive& archive) + { + CommonBase::DoArchive(archive); + archive & names; + } }; class PtrHolder : virtual public CommonBase @@ -28,7 +33,11 @@ public: vector numbers; virtual ~PtrHolder() {} - virtual void DoArchive(Archive& archive) { archive & numbers; } + virtual void DoArchive(Archive& archive) + { + CommonBase::DoArchive(archive); + archive & numbers; + } }; class SharedPtrAndPtrHolder : public SharedPtrHolder, public PtrHolder @@ -42,6 +51,18 @@ public: } }; +// Classes without virt. or multiple inheritance do not need to be registered +class SimpleClass : public CommonBase +{ +public: + double d; + virtual void DoArchive(Archive& ar) + { + CommonBase::DoArchive(ar); + ar & d; + } +}; + class NotRegisteredForArchive : public SharedPtrAndPtrHolder {}; class OneMoreDerivedClass : public SharedPtrAndPtrHolder {}; @@ -52,6 +73,19 @@ static RegisterClassForArchive regp; static RegisterClassForArchive regspp; static RegisterClassForArchive regom; +void testNullPtr(Archive& in, Archive& out) +{ + SharedPtrHolder* p = nullptr; + shared_ptr sp = nullptr; + out & p & sp; + out.FlushBuffer(); + SharedPtrHolder* pin; + shared_ptr spin; + in & pin & spin; + CHECK(pin == nullptr); + CHECK(spin == nullptr); +} + void testSharedPointer(Archive& in, Archive& out) { SECTION("Same shared ptr") @@ -92,6 +126,7 @@ void testMultipleInheritance(Archive& in, Archive& out) { PtrHolder* p = new OneMoreDerivedClass; p->numbers.push_back(new int(2)); + p->a = 5; auto p2 = dynamic_cast(p); p2->names.push_back(make_shared("test")); auto sp1 = shared_ptr(p); @@ -103,6 +138,8 @@ void testMultipleInheritance(Archive& in, Archive& out) CHECK(*pin2->names[0] == "test"); CHECK(*pin->numbers[0] == 2); CHECK(dynamic_cast(pin) == dynamic_cast(pin2)); + CHECK(pin->a == pin2->a); + CHECK(pin->a == 5); REQUIRE(dynamic_cast(pin2) != nullptr); CHECK(*dynamic_cast(pin2)->numbers[0] == 2); CHECK(*pin->numbers[0] == *dynamic_cast(pin2)->numbers[0]); @@ -136,6 +173,31 @@ void testMultipleInheritance(Archive& in, Archive& out) in & bin & pin; checkPtr(pin, dynamic_cast(bin)); } + SECTION("Simple class without register") + { + auto a = new SimpleClass; + a->a = 5; + a->d = 2.3; + SECTION("check pointer") + { + out & a; + out.FlushBuffer(); + SimpleClass* ain; + in & ain; + CHECK(ain->a == 5); + CHECK(ain->d == 2.3); + } + SECTION("check shared pointer") + { + auto spa = shared_ptr(a); + out & spa; + out.FlushBuffer(); + shared_ptr spain; + in & spain; + CHECK(spain->a == 5); + CHECK(spain->d == 2.3); + } + } } void testArchive(Archive& in, Archive& out) @@ -157,6 +219,10 @@ void testArchive(Archive& in, Archive& out) SharedPtrAndPtrHolder* p = new NotRegisteredForArchive; REQUIRE_THROWS(out & p, Catch::Contains("not registered for archive")); } + SECTION("nullptr") + { + testNullPtr(in, out); + } } TEST_CASE("BinaryArchive")