Can't provide serialization for vector<int>

[vid512]

I can provide Snitch with serialization for vector of custom class (MyInt), but can't seem to provide it for vector of ints in the same way:

#define SNITCH_IMPLEMENTATION
#include "snitch_all.hpp"

#include <vector>

class MyInt {
public:
    int xx_;
    MyInt(int xx) : xx_(xx) {}
    bool operator==(const MyInt& rhs) const {return this->xx_ == rhs.xx_;}
};

bool append(snitch::small_string_span ss, const MyInt& item) {
    return append(ss, item.xx_);
}

bool append(snitch::small_string_span ss, const std::vector<MyInt>& vec) {
    if (!append(ss, "{"))
        return false; 
    bool first = true;
    for (const MyInt& item : vec) {
        if (!append(ss, first ? "" : ",", item))
            return false;
        first = false;
    }
    return append(ss, "}" );
}

bool append(snitch::small_string_span ss, const std::vector<int>& vec) {
    if (!append(ss, "{"))
        return false; 
    bool first = true;
    for (const int& item : vec) {
        if (!append(ss, first ? "" : ",", item))
            return false;
        first = false;
    }
    return append(ss, "}" );
}

TEST_CASE("vector", "") {
    CHECK( std::vector<MyInt>{1,1} == std::vector<MyInt>{1,2} );
    CHECK( std::vector<int>{1,1} == std::vector<int>{1,2} );
}

This outputs:

failed: running test case "vector"
          at a.cpp:42
          CHECK(std::vector<MyInt>{1,1} == std::vector<MyInt>{1,2}), got {1,1} != {1,2}
failed: running test case "vector"
          at a.cpp:43
          CHECK(std::vector<int>{1,1} == std::vector<int>{1,2}), got ? != ?

Why is this? Maybe there's something wrong with checking for fundamental type serialization, that causes this problem?

[cschreib]

Hi and thank you for reporting this. I would have to run some tests and read more about the topic, but I suspect this is because append() is meant to be found through ADL, and somehow that works for MyInt (append is defined in the same namespace) and not for int?

Alternatively, try calling append() yourself on your test vectors, you might get an error message explaining what is happening.

It could be an ambiguous call, since there is no append(int). You have append(std::ptrdiff_t) and append(MyInt) which are both usable, because the MyInt(int) constructor is not marked as explicit. Edit: Actually we do have append(std::signed_integral) which should be a perfect match, so maybe that's not it.

[vid512]

try calling append() yourself on your test vectors, you might get an error message explaining what is happening.

Nope. With direct call to append(), the provided serialization works.

#define SNITCH_IMPLEMENTATION
#include "snitch_all.hpp"

#include <vector>

bool append(snitch::small_string_span ss, const std::vector<int>& vec) {
    if (!append(ss, "{"))
        return false; 
    bool first = true;
    for (const int& item : vec) {
        if (!append(ss, first ? "" : ",", item))
            return false;
        first = false;
    }
    return append(ss, "}" );
}

TEST_CASE("vector", "") {
    snitch::small_string<100> s;
    std::vector<int> v{1,1};
    append(s.span(), v);
    CAPTURE(s);
    CHECK( v == std::vector<int>{1,2} );
}

result:

failed: running test case "vector"
          at a.cpp:23
          with s := {1,1}
          CHECK(v == std::vector<int>{1,2}), got ? != ?

It could be an ambiguous call, since there is no append(int)

Why? In my understanding, append(std::vector) doesn't have to call append(int) at all. It can just display static string "vector-of-ints", for example. And anyway, whatever it calls doesn't figure in the lookup process.

#define SNITCH_IMPLEMENTATION
#include "snitch_all.hpp"

#include <vector>

bool append(snitch::small_string_span ss, const std::vector<int>& vec) {
    (void)vec;
    return append(ss, "vector-of-ints");
}

TEST_CASE("vector", "") {
    snitch::small_string<100> s;
    std::vector<int> v{1,1};
    append(s.span(), v);
    CAPTURE(s);
    CHECK( v == std::vector<int>{1,2} );
}

result:

failed: running test case "vector"
          at a.cpp:16
          with s := vector-of-ints
          CHECK(v == std::vector<int>{1,2}), got ? != ?

[cschreib]

Sorry for the delay in getting back to you. I can confirm this is a "feature" of ADL. If you check the snitch README carefully, you'll find this:

If you want to serialize custom types not supported out of the box by snitch, you need to provide your own append() function. This function must be placed in the same namespace as your custom type or in the snitch namespace, so it can be found by ADL.

Indeed, if you place your append(... std::vector<int>) overload either in the std or snitch namespace, then it works as expected.

The reason it works for MyInt is because (from the standard link above):

... for every argument in a function call expression its type is examined to determine the associated set of namespaces and classes that it will add to the lookup. ... 3) For arguments whose type is a class template specialization, in addition to the class rules, the following types are examined and their associated classes and namespaces are added to the set ... b) The namespaces in which any template template arguments are members

So for std::vector<MyInt>, snitch will look for:

• snitch::append() (because of the snitch::small_string_span argument)
• std::append() (because of the std::vector argument)
• ::append() (because of MyInt template parameter)

For std::vector<int> instead we get:

• snitch::append() (because of the snitch::small_string_span argument)
• std::append() (because of the std::vector argument)
• and that's it (int doesn't bring any namespace into ADL)

My recommendation, if you find ADL too weird (you wouldn't be alone in that!), is to always place your append() functions in the snitch namespace. That will always work.

[vid512]

Thanks for explanation.

I did read the paragraph in Snitch README, but I assumed int belongs to :: namespace (eg. that it is actually ::int). Even after reading the quoted parts of standard, it isn't obvious to me, that it doesn't.

I suggest you add an extra sentence to README, warning users about this. Something like following:

Note that builtin types are not considered members of global namespace, so for example ::apend(::std::vector<int>) will not work. You must place your specializations for builtin types to "snitch" namespace.

(But of course, check standard for technical correctness).

[vid512]

After rereading the paragraph from README more carefully, I'd phrase it differently.

The README does say to place serialization for custom types in namespace of the type, or snitch namespace. However, in my case, std::vector<int> is not a custom type, so the paragraph doesn't strictly describe how to serialize it. Only by analogy can I assume, how to serialize standard non-custom types.

The ADL lookup mechanism of inner type is not described. The paragraph instructs me to place append in "same namespace as your custom type". In case of ::std::vector<::MyInt>, that would be ::std, not :: (because custom type is ::std::vector, not ::MyInt).

I still think this warrants a better description. Not sure how to best phrase it without delving into all detail. Maybe you can describe it like this: Best option is always to provide append in the snitch namespace. Optionally, you can also provide it in namespace of the custom class being serialized, or in case of template class, in namespace of any of template parameters, according to ADL rules.

This will instruct readers to place append in snitch namespace by default, and anyone who wants to use the alternative, is referenced to ADL rules.

[cschreib]

Even after reading the quoted parts of standard, it isn't obvious to me, that it doesn't.

They do say this:

For arguments of fundamental type, the associated set of namespaces and classes is empty.

I agree that the snitch README could be worded better though. I don't want to turn it into a tutorial on ADL, but we can add a bit more information to prevent people from experiencing the same issue you have. I'll look into it!