From 8e29d38fc1750749fa23efc9fcbeae1e09152587 Mon Sep 17 00:00:00 2001 From: Christopher Lackner Date: Mon, 3 Dec 2018 16:28:04 +0100 Subject: [PATCH] archive works for pointers and shared_ptrs (even with mult. inheritance and virtual base classes) --- CMakeLists.txt | 6 + cmake/SuperBuild.cmake | 1 + cmake/external_projects/catch.cmake | 18 +++ libsrc/core/CMakeLists.txt | 4 +- libsrc/core/archive.hpp | 2 + libsrc/core/basearchive.cpp | 4 +- libsrc/core/basearchive.hpp | 196 ++++++++++++++++++++++++---- libsrc/meshing/CMakeLists.txt | 3 +- tests/CMakeLists.txt | 1 + tests/catch/CMakeLists.txt | 30 +++++ tests/catch/archive.cpp | 176 +++++++++++++++++++++++++ tests/catch/main.cpp | 3 + 12 files changed, 414 insertions(+), 30 deletions(-) create mode 100644 cmake/external_projects/catch.cmake create mode 100644 tests/catch/CMakeLists.txt create mode 100644 tests/catch/archive.cpp create mode 100644 tests/catch/main.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6f4bfd9e..3e8f78c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,6 +17,7 @@ option( INTEL_MIC "cross compile for intel xeon phi") option( INSTALL_PROFILES "install environment variable settings to /etc/profile.d" OFF ) option( USE_CCACHE "use ccache") option( USE_INTERNAL_TCL "Compile tcl files into the code and don't install them" ON) +option( ENABLE_UNIT_TESTS "Enable Catch unit tests") option( USE_SUPERBUILD "use ccache" ON) @@ -341,6 +342,11 @@ execute_process(COMMAND hdiutil create -volname Netgen -srcfolder ${CMAKE_INSTAL enable_testing() include(CTest) +if(ENABLE_UNIT_TESTS) + include(${CMAKE_CURRENT_LIST_DIR}/cmake/external_projects/catch.cmake) +endif(ENABLE_UNIT_TESTS) + + ####################################################################### add_subdirectory(libsrc) diff --git a/cmake/SuperBuild.cmake b/cmake/SuperBuild.cmake index 78be04ec..c5b3cb47 100644 --- a/cmake/SuperBuild.cmake +++ b/cmake/SuperBuild.cmake @@ -140,6 +140,7 @@ set_vars( NETGEN_CMAKE_ARGS INTEL_MIC CMAKE_PREFIX_PATH CMAKE_INSTALL_PREFIX + ENABLE_UNIT_TESTS ) # propagate all variables set on the command line using cmake -DFOO=BAR diff --git a/cmake/external_projects/catch.cmake b/cmake/external_projects/catch.cmake new file mode 100644 index 00000000..0f79e6c7 --- /dev/null +++ b/cmake/external_projects/catch.cmake @@ -0,0 +1,18 @@ +include (ExternalProject) +find_program(GIT_EXECUTABLE git) +ExternalProject_Add( + project_catch + PREFIX ${CMAKE_BINARY_DIR}/catch + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG v2.0.1 + TIMEOUT 10 + UPDATE_COMMAND "" # ${GIT_EXECUTABLE} pull + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + INSTALL_COMMAND "" + LOG_DOWNLOAD ON + ) + +# Expose required variable (CATCH_INCLUDE_DIR) to parent scope +ExternalProject_Get_Property(project_catch source_dir) +set(CATCH_INCLUDE_DIR ${source_dir}/single_include CACHE INTERNAL "Path to include folder for Catch") diff --git a/libsrc/core/CMakeLists.txt b/libsrc/core/CMakeLists.txt index d49da46b..8b14b211 100644 --- a/libsrc/core/CMakeLists.txt +++ b/libsrc/core/CMakeLists.txt @@ -1,8 +1,10 @@ add_definitions(-DNGINTERFACE_EXPORTS) -add_library(ngcore OBJECT basearchive.cpp) +add_library(ngcore basearchive.cpp) set_target_properties(ngcore PROPERTIES POSITION_INDEPENDENT_CODE ON ) +install(TARGETS ngcore DESTINATION ${NG_INSTALL_DIR} COMPONENT netgen) + install(FILES ngcore.hpp archive.hpp basearchive.hpp DESTINATION ${NG_INSTALL_DIR_INCLUDE}/core COMPONENT netgen_devel ) diff --git a/libsrc/core/archive.hpp b/libsrc/core/archive.hpp index 639102f4..7b213b8d 100644 --- a/libsrc/core/archive.hpp +++ b/libsrc/core/archive.hpp @@ -16,6 +16,7 @@ namespace ngcore : BinaryOutArchive(std::make_shared(filename)) {} virtual ~BinaryOutArchive () { FlushBuffer(); } + using Archive::operator&; virtual Archive & operator & (double & d) { return Write(d); } virtual Archive & operator & (int & i) @@ -81,6 +82,7 @@ namespace ngcore BinaryInArchive (std::string filename) : BinaryInArchive(std::make_shared(filename)) {} + using Archive::operator&; virtual Archive & operator & (double & d) { Read(d); return *this; } virtual Archive & operator & (int & i) diff --git a/libsrc/core/basearchive.cpp b/libsrc/core/basearchive.cpp index 1fcbe8b8..434fbab5 100644 --- a/libsrc/core/basearchive.cpp +++ b/libsrc/core/basearchive.cpp @@ -3,9 +3,9 @@ namespace ngcore { - std::map>& GetArchiveRegister() + std::map& GetArchiveRegister() { - static std::map> type_register = {}; + static std::map type_register; return type_register; } } diff --git a/libsrc/core/basearchive.hpp b/libsrc/core/basearchive.hpp index a43c4bc0..a97cd28c 100644 --- a/libsrc/core/basearchive.hpp +++ b/libsrc/core/basearchive.hpp @@ -20,7 +20,31 @@ namespace ngcore static constexpr bool value = type::value; }; - std::map>& GetArchiveRegister(); + // Info stored by registering a class using the RegisterClassForArchive struct in the map + // stored in GetArchiveRegister + struct ClassArchiveInfo + { + // create new object of this type and return a void* pointer that is points to the location + // of the (base)class given by type_info + std::function creator; + // This caster takes a void* pointer to the type stored in this info and casts it to a + // void* pointer pointing to the (base)class type_info + std::function upcaster; + // This caster takes a void* pointer to the (base)class type_info and returns void* pointing + // to the type stored in this info + std::function downcaster; + }; + + // Returns a map of from the mangled typeids to the ClassArchiveInfo + std::map& GetArchiveRegister(); + + // Helper class for up-/downcasting + template + struct Caster + { + static void* tryUpcast(const std::type_info& ti, T* p); + static void* tryDowncast(const std::type_info& ti, void* p); + }; // Base Archive class class Archive @@ -76,11 +100,12 @@ namespace ngcore size = v.size(); (*this) & size; if(!is_output) - v.reserve(size); + v.resize(size); Do(&v[0], size); return (*this); } // Archive arrays ===================================================== + // this functions can be overloaded in Archive implementations for more efficiency template Archive & Do (T * data, size_t n) { for (size_t j = 0; j < n; j++) { (*this) & data[j]; }; return *this; }; @@ -122,32 +147,87 @@ namespace ngcore // save -2 for nullptr if(!ptr) return (*this) << -2; - auto pos = shared_ptr2nr.find((void*) ptr.get()); + + 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; + } + auto pos = shared_ptr2nr.find(reg_ptr); // if not found store -1 and the pointer if(pos == shared_ptr2nr.end()) { - shared_ptr2nr[(void*) ptr.get()] = shared_ptr_count++; auto p = ptr.get(); - return (*this) << -1 & p; + (*this) << -1; + (*this) & neededDowncast & p; + if(neededDowncast) + (*this) << std::string(typeid(*ptr).name()); + shared_ptr2nr[reg_ptr] = shared_ptr_count++; + return *this; } - // if found store the position - return (*this) << pos->second; + // if found store the position and if it has to be downcasted and how + (*this) << pos->second << neededDowncast; + if(neededDowncast) + (*this) << std::string(typeid(*ptr).name()); + return (*this); } else // Input { int nr; (*this) & nr; if(nr == -2) - ptr = nullptr; + { + ptr = nullptr; + return *this; + } else if (nr == -1) { T* p; - (*this) & p; + bool neededDowncast; + (*this) & neededDowncast & p; ptr = std::shared_ptr(p); - nr2shared_ptr.push_back(ptr); + if(neededDowncast) + { + std::string name; + (*this) & name; + auto info = GetArchiveRegister()[name]; + nr2shared_ptr.push_back(std::shared_ptr(std::static_pointer_cast(ptr), + info.downcaster(typeid(T), + ptr.get()))); + } + else + nr2shared_ptr.push_back(ptr); } else - ptr = std::reinterpret_pointer_cast(nr2shared_ptr[nr]); + { + auto other = nr2shared_ptr[nr]; + bool neededDowncast; + (*this) & neededDowncast; + if(neededDowncast) + { + if constexpr(has_DoArchive::value) + { + std::string name; + (*this) & name; + auto info = GetArchiveRegister()[name]; + ptr = std::static_pointer_cast(std::shared_ptr(other, + info.upcaster(typeid(T), + other.get()))); + } + else + throw std::runtime_error("Shouldn't get here..."); + } + else + ptr = std::static_pointer_cast(other); + } } return *this; } @@ -165,11 +245,21 @@ namespace ngcore (*this) & m2; return *this; } - auto pos = ptr2nr.find( (void*) p); + 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); + } + auto pos = ptr2nr.find(reg_ptr); // if the pointer is not found in the map create a new entry if (pos == ptr2nr.end()) { - ptr2nr[(void*) p] = ptr_count++; + ptr2nr[reg_ptr] = ptr_count++; if(typeid(*p) == typeid(T)) if constexpr (std::is_constructible::value) { @@ -183,7 +273,7 @@ namespace ngcore // We want this special behaviour only for our classes that implement DoArchive if constexpr(has_DoArchive::value) { - if(GetArchiveRegister().count(typeid(*p).name()) == 0) + 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"); @@ -199,13 +289,13 @@ namespace ngcore else { (*this) & pos->second; + (*this) << std::string(typeid(*p).name()); } } else { int nr; (*this) & nr; - // cout << "in, got nr " << nr << endl; if (nr == -2) { p = nullptr; @@ -215,9 +305,8 @@ namespace ngcore if constexpr (std::is_constructible::value) { p = new T; - // cout << "create new ptr, p = " << p << endl; - (*this) & *p; nr2ptr.push_back(p); + (*this) & *p; } else throw std::runtime_error("Class isn't registered properly"); @@ -230,16 +319,25 @@ namespace ngcore { std::string name; (*this) & name; - p = reinterpret_cast(GetArchiveRegister()[name]()); - nr2ptr.push_back(p); + auto info = GetArchiveRegister()[name]; + p = (T*) info.creator(typeid(T)); + nr2ptr.push_back(info.downcaster(typeid(T),p)); + (*this) & *p; } else throw std::runtime_error("Class isn't registered properly"); } else { - p = (T*)nr2ptr[nr]; - // cout << "reuse ptr " << nr << ": " << p << endl; + std::string name; + (*this) & name; + if constexpr(has_DoArchive::value) + { + auto info = GetArchiveRegister()[name]; + p = (T*) info.upcaster(typeid(T), nr2ptr[nr]); + } + else + p = (T*) nr2ptr[nr]; } } return *this; @@ -253,14 +351,62 @@ namespace ngcore (*this) & ht; return *this; } + + virtual void FlushBuffer() {} + }; + + template + class RegisterClassForArchive + { + public: + RegisterClassForArchive() + { + static_assert(std::is_constructible_v, "Class registered for archive must be default constructible"); + ClassArchiveInfo info; + info.creator = [this,&info](const std::type_info& ti) -> void* + { return typeid(T) == ti ? new T : Caster::tryUpcast(ti, new T); }; + info.upcaster = [this](const std::type_info& ti, void* p) -> void* + { return typeid(T) == ti ? p : Caster::tryUpcast(ti, (T*) p); }; + info.downcaster = [this](const std::type_info& ti, void* p) -> void* + { return typeid(T) == ti ? p : Caster::tryDowncast(ti, p); }; + GetArchiveRegister()[std::string(typeid(T).name())] = info; + } }; template - void RegisterClassForArchive() + struct Caster { - static_assert(std::is_constructible_v, "Class registered for archive must be default constructible"); - GetArchiveRegister()[std::string(typeid(T).name())] = []() -> void* { return new T; }; - } + static void* tryUpcast (const std::type_info& ti, T* p) + { + throw std::runtime_error("Upcast not successful, some classes are not registered properly for archiving!"); + } + static void* tryDowncast (const std::type_info& ti, void* p) + { + throw std::runtime_error("Downcast not successful, some classes are not registered properly for archiving!"); + } + }; + + template + struct Caster + { + static void* tryUpcast(const std::type_info& ti, T* p) + { + try + { return GetArchiveRegister()[typeid(B1).name()].upcaster(ti, (void*) (dynamic_cast(p))); } + catch(std::exception) + { return Caster::tryUpcast(ti, p); } + } + + static void* tryDowncast(const std::type_info& ti, void* p) + { + if(typeid(B1) == ti) + return dynamic_cast((B1*) p); + try + { return GetArchiveRegister()[typeid(B1).name()].downcaster(ti, (void*) ((B1*)p)); } + catch(std::exception) + { return Caster::tryDowncast(ti, p); } + } + }; } #endif // NG_BASEARCHIVE_HPP diff --git a/libsrc/meshing/CMakeLists.txt b/libsrc/meshing/CMakeLists.txt index e4eb56d9..bc9f7f18 100644 --- a/libsrc/meshing/CMakeLists.txt +++ b/libsrc/meshing/CMakeLists.txt @@ -4,7 +4,6 @@ if(NOT WIN32) $ $ $ - $ ) endif(NOT WIN32) @@ -31,7 +30,7 @@ if(APPLE) endif(APPLE) if(NOT WIN32) - target_link_libraries( mesh ${ZLIB_LIBRARIES} ${MPI_CXX_LIBRARIES} ${PYTHON_LIBRARIES} ${METIS_LIBRARY}) + target_link_libraries( mesh ngcore ${ZLIB_LIBRARIES} ${MPI_CXX_LIBRARIES} ${PYTHON_LIBRARIES} ${METIS_LIBRARY}) install( TARGETS mesh ${NG_INSTALL_DIR}) endif(NOT WIN32) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f898217f..a0783363 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1 +1,2 @@ +add_subdirectory(catch) add_subdirectory(pytest) diff --git a/tests/catch/CMakeLists.txt b/tests/catch/CMakeLists.txt new file mode 100644 index 00000000..d980d7f8 --- /dev/null +++ b/tests/catch/CMakeLists.txt @@ -0,0 +1,30 @@ + +if(ENABLE_UNIT_TESTS) +add_custom_target(unit_tests) + +# Build catch_main test object +message("netgen include dir = ${NETGEN_INCLUDE_DIR_ABSOLUTE} --------------------------------------") +include_directories(${CATCH_INCLUDE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../../libsrc/include}) +add_library(catch_main STATIC main.cpp) +set_target_properties(catch_main PROPERTIES CXX_STANDARD 17) +add_dependencies(unit_tests catch_main) +add_dependencies(catch_main project_catch) + +# ensure the test targets are built before testing +add_test(NAME unit_tests_built COMMAND ${CMAKE_COMMAND} --build . --target unit_tests --config ${CMAKE_BUILD_TYPE} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../.. ) + +macro(add_unit_test name sources) + add_executable(test_${name} ${sources} ) + if (WIN32) + target_link_libraries(test_${name} ngcore catch_main) + else(WIN32) + target_link_libraries(test_${name} ngcore catch_main) + endif(WIN32) + + add_dependencies(unit_tests test_${name}) + add_test(NAME unit_${name} COMMAND test_${name}) + set_tests_properties(unit_${name} PROPERTIES DEPENDS unit_tests_built) +endmacro() + +add_unit_test(archive archive.cpp) +endif(ENABLE_UNIT_TESTS) diff --git a/tests/catch/archive.cpp b/tests/catch/archive.cpp new file mode 100644 index 00000000..d34e3878 --- /dev/null +++ b/tests/catch/archive.cpp @@ -0,0 +1,176 @@ + +#include "catch.hpp" +#include <../core/ngcore.hpp> +using namespace ngcore; +using namespace std; + +class CommonBase +{ + public: + virtual ~CommonBase() {} + + virtual void DoArchive(Archive& archive) { } +}; + +class SharedPtrHolder : virtual public CommonBase +{ +public: + vector> names; + virtual ~SharedPtrHolder() + { } + + virtual void DoArchive(Archive& archive) { archive & names; } +}; + +class PtrHolder : virtual public CommonBase +{ +public: + vector numbers; + virtual ~PtrHolder() {} + + virtual void DoArchive(Archive& archive) { archive & numbers; } +}; + +class SharedPtrAndPtrHolder : public SharedPtrHolder, public PtrHolder +{ +public: + virtual ~SharedPtrAndPtrHolder() {} + virtual void DoArchive(Archive& archive) + { + SharedPtrHolder::DoArchive(archive); + PtrHolder::DoArchive(archive); + } +}; + +class NotRegisteredForArchive : public SharedPtrAndPtrHolder {}; + +class OneMoreDerivedClass : public SharedPtrAndPtrHolder {}; + +static RegisterClassForArchive regb; +static RegisterClassForArchive regsp; +static RegisterClassForArchive regp; +static RegisterClassForArchive regspp; +static RegisterClassForArchive regom; + +void testSharedPointer(Archive& in, Archive& out) +{ + SECTION("Same shared ptr") + { + static_assert(has_DoArchive::value, ""); + SharedPtrHolder holder, holder2; + holder.names.push_back(make_shared("name")); + holder2.names = holder.names; // same shared ptr + out & holder & holder2; + out.FlushBuffer(); + SharedPtrHolder inholder, inholder2; + in & inholder & inholder2; + CHECK(inholder.names.size() == 1); + CHECK(inholder.names[0] == inholder2.names[0]); + CHECK(inholder.names[0].use_count() == 3); // one shared ptr is still kept in the archive + CHECK(*inholder.names[0] == "name"); + } +} + +void testPointer(Archive& in, Archive& out) +{ + SECTION("Same pointer") + { + PtrHolder holder, holder2; + holder.numbers.push_back(new int(3)); + holder2.numbers = holder.numbers; // same shared ptr + out & holder & holder2; + out.FlushBuffer(); + PtrHolder inholder, inholder2; + in & inholder & inholder2; + CHECK(inholder.numbers.size() == 1); + CHECK(inholder.numbers[0] == inholder2.numbers[0]); + CHECK(*inholder.numbers[0] == 3); + } +} + +void testMultipleInheritance(Archive& in, Archive& out) +{ + PtrHolder* p = new OneMoreDerivedClass; + p->numbers.push_back(new int(2)); + auto p2 = dynamic_cast(p); + p2->names.push_back(make_shared("test")); + auto sp1 = shared_ptr(p); + auto sp2 = dynamic_pointer_cast(sp1); + auto checkPtr = [] (auto pin, auto pin2) + { + CHECK(typeid(*pin) == typeid(*pin2)); + CHECK(typeid(*pin) == typeid(OneMoreDerivedClass)); + CHECK(*pin2->names[0] == "test"); + CHECK(*pin->numbers[0] == 2); + CHECK(dynamic_cast(pin) == dynamic_cast(pin2)); + REQUIRE(dynamic_cast(pin2) != nullptr); + CHECK(*dynamic_cast(pin2)->numbers[0] == 2); + CHECK(*pin->numbers[0] == *dynamic_cast(pin2)->numbers[0]); + REQUIRE(dynamic_cast(pin) != nullptr); + CHECK(dynamic_cast(pin)->names[0] == pin2->names[0]); + }; + SECTION("Archive ptrs to leaves of mult. inh.") + { + out & p & p2; + out.FlushBuffer(); + PtrHolder* pin; + SharedPtrHolder* pin2; + in & pin & pin2; + checkPtr(pin, pin2); + } + SECTION("Archive shared ptrs to leaves of mult. inh.") + { + out & sp1 & sp2; + out.FlushBuffer(); + shared_ptr pin; + shared_ptr pin2; + in & pin & pin2; + checkPtr(pin.get(), pin2.get()); + } + SECTION("Virtual base class") + { + CommonBase* b = dynamic_cast(p); + out & b & p; + PtrHolder* pin; + CommonBase* bin; + in & bin & pin; + checkPtr(pin, dynamic_cast(bin)); + } +} + +void testArchive(Archive& in, Archive& out) +{ + SECTION("SharedPtr") + { + testSharedPointer(in, out); + } + SECTION("Pointer") + { + testPointer(in, out); + } + SECTION("Multiple inheritance") + { + testMultipleInheritance(in, out); + } + SECTION("Not registered") + { + SharedPtrAndPtrHolder* p = new NotRegisteredForArchive; + REQUIRE_THROWS(out & p, Catch::Contains("not registered for archive")); + } +} + +TEST_CASE("BinaryArchive") +{ + auto stream = make_shared(); + BinaryOutArchive out(stream); + BinaryInArchive in(stream); + testArchive(in, out); +} + +TEST_CASE("TextArchive") +{ + auto stream = make_shared(); + TextOutArchive out(stream); + TextInArchive in(stream); + testArchive(in, out); +} diff --git a/tests/catch/main.cpp b/tests/catch/main.cpp new file mode 100644 index 00000000..de419564 --- /dev/null +++ b/tests/catch/main.cpp @@ -0,0 +1,3 @@ + +#define CATCH_CONFIG_MAIN +#include