Ser/Des-ialize does not rebuild ptrs etc. - an another day with namespaces

[ChemistAion]

Please take a look on this example: https://wandbox.org/permlink/l2iEAmTpiCWFtLb6

Works like a charm, i.e.: CISTA::vector<CISTA::ptr<ElementTest>> paths_ pointers are perfectly rebuild during deserialization. No issue...

Exactly same stuff under latest VS 17.2.6 with all features beyond C++20 enabled gives me two things:

• static_assert need for a custom serializer for ElementTest - whereas wandbox not;
• after ser/des empty fn provided for ElementTest, paths_ still points to memory before serialization;

I am still investigating this...

[felixguendling]

First question: are you using the latest release or the latest master commit of cista?

[felixguendling]

Cannot reproduce with MSVC. https://godbolt.org/z/EMzj9EfYo (just copy&pasted the whole cista.h there instead of #include "cista.h")

Or did I miss something?

[ChemistAion]

Sourced through "Download" button on: https://cista.rocks/

[felixguendling]

Let me know when you find a reproducible code. For our uses on MSVC here, it seems to work fine until now.

[ChemistAion]

OK, I have some time now... I have a reproducible example (MSVC native with latest stuff), but it is deeply embedded into my working project:

I will dissect it more deeply... As you can see unique_ptr are restored whereas ptr inside vector are not; ...my bet is that: I am using additional layer of namespace, using and auto to access CISTA features.

btw: I was forced (to make it work as above) by "static_assert failed: 'Please implement custom serializer'" to implement these dummy/empty serdes:

    template <typename Context>
    inline void serialize(Context& context, const ElementTest* test, const cista::offset_t offset) {};

    template <typename Context>
    inline void deserialize(const Context& context, ElementTest* test) {};

[ChemistAion]

Virtually the same ex. from:

Cannot reproduce with MSVC. https://godbolt.org/z/EMzj9EfYo (just copy&pasted the whole cista.h there instead of #include "cista.h")

Or did I miss something?

[STEP1] https://godbolt.org/z/WE8WTbGr9 Introduction of namespace TEST will lead to series of my mistakes? ;)

    namespace TEST
    {
        struct VEC2
        {
            float                                   x, y;
            constexpr VEC2()                      : x(0.0f), y(0.0f) { }
            constexpr VEC2(float _x, float _y)    : x(_x), y(_y) { }
        };
    }

[felixguendling]

to implement these dummy/empty serdes

This is not the right way to do it. If you add custom constructor or do something that removes the C++ aggregate property, you need to take care for serializing all members on your own. Having an empty body does not work in this case!

This makes your bug reproducible: https://godbolt.org/z/q5Ghd7Pd5

You need to implement a proper serialize and deserialize method, calling serialize/deserialize properly for all members.

If you opt-out of "cista is doing everything for you automatically" and implement your own stuff, it's your responsibility to not miss any member. Having custom (de-)serialization functions is very error-prone.

Here's the program with proper (de-)serialization for the ElementTest type. Don't forget to adapt it if you add/remove members!

https://godbolt.org/z/81TWKojzv

    template <typename Context>
    inline void serialize(Context& context, const ElementTest* test, const cista::offset_t offset) {
        using cista::serialize;
        serialize(context, &test->node_, offset + offsetof(ElementTest, node_));
        serialize(context, &test->paths_, offset + offsetof(ElementTest, paths_));
    }

    template <typename Context>
    inline void deserialize(const Context& context, ElementTest* test) {
        using cista::deserialize;
        deserialize(context, &test->node_);
        deserialize(context, &test->paths_);
    }

[ChemistAion]

You have been too fast: https://godbolt.org/z/jvrTzKd37

[ChemistAion]

...yep, now it is obvious - thx for your time! As an example here - you should consider to add some more adv. docu sections? ;)

[felixguendling]

Yep, thank you for the hint. I'll put a similar example + explanation in the docs.

[felixguendling]

Added a section to this Wiki page: https://github.com/felixguendling/cista/wiki/Custom-(De-)Serialization-Functions

[ChemistAion]

Finally, to show you exactly why I ended up here - please take a look on this case: https://godbolt.org/z/cxqG8qMEK As you can see, there is no need for custom ser/des for this thing: (it is non-aggregate since it has custom TEST:VEC2 (with ctors) embedded into NodeTest):

    struct ElementTest
    {
        NodeTest node_;
        CISTA::vector<CISTA::ptr<ElementTest>> paths_;
    };

It (struct ElementTest) works in semi-auto with no custom ser/des because serialize(..., const VEC2* element, ...) is provided and visible/used by cista, no issue. Additionally, there is no need for custom des since it is a simple thing, cist auto des takes over perfectly here.

    namespace TEST
    {
        struct VEC2
        {
            float                                   x, y;
            constexpr VEC2()                      : x(0.0f), y(0.0f) { }
            constexpr VEC2(float _x, float _y)    : x(_x), y(_y) { }
        };

        template <typename Context>
        void serialize(Context& context, const VEC2* element, const cista::offset_t)
        {
            context.write(element, (cista::offset_t)sizeof(VEC2));
        }        
    }

Whereas moving TEST::VEC2 serialization outside namespace TEST makes it not visible/usable and leads to a need of custom ser/des for the whole struct ElementTest - the rest is as above in this issue:

    using namespace TEST; //dummy

    template <typename Context>
    void serialize(Context& context, const TEST::VEC2* element, const cista::offset_t)
    {
        context.write(element, (cista::offset_t)sizeof(TEST::VEC2));
    }

To make it work (semi-auto mode for non-aggregate struct ElementTest) I need to inject ser/des for custom types directly into their namespaces - so it looks like providing a full type scope for ser does not help to be catched/used out of type namespace.

[felixguendling]

This is not cista specific. It's C++ ADL - you can look up how it works on Google. Basically what the compiler does when looking up functions it considers also the namespace of the function arguments. For this to work, you need to place the serialization functions in the same namespace as the type itself. However, I don't see the problem with this approach. There are alternatives (tag based dispatch for example) but in this case ADL should be fine. It does not justify breaking the cista library interface.

[ChemistAion]

Thank you for time and clarification.