dynamic_array_ref::data() should not fail if empty

[ujos]

Following is a body of the dynamic_array_ref::data() member function.

    SBEPP_CPP14_CONSTEXPR pointer data() const noexcept
    {
        SBEPP_ASSERT(!empty());
        return data_checked();
    }

As the first line of the code, there is a check for the emptiness. I'm not sure it should be there.

Consider the following code:

template <typename Byte, typename Len, sbepp::endian E>
void set(::sbepp::detail::dynamic_array_ref<Byte, char, Len, E> dest, std::string_view value) {

    dest.resize(value.size(), sbepp::default_init);
    memcpy(std::data(dest), std::data(value), std::size(value));
}

It is a proper code if you use for example std::vector instead of dynamic_array_ref. The issue is you cannot assign an empty string.

Could you remove that check?

Meanwhile, what is the proper way to initialize all the fields of the message? Is there a some kind of default constructor?

[OleksandrKvl]

The intent was that data doesn't return "usable" pointer if empty. Actually, in your case assertion is helpful because passing nullptr to memcpy is UB even if size is 0. You can just use dest.assign(std::begin(value), std::end(value)) which is clearer and shorter.

Also, it's not good to use types from detail, they are implementation details. Let me know if there's no way you can avoid it and I'll think about making them public. For example here, can't you just do msg.dataField().assign(std::begin(str), std::end(str)); or at least use auto for the first parameter?

Meanwhile, what is the proper way to initialize all the fields of the message? Is there a some kind of default constructor?

Nope, sbepp generates a low-level wrapper that avoids doing anything beyond what's written explicitly. SBE doesn't have a notion of default values, the best we can do is to set null values for optional top-level fields, initialize group headers with 0 entries and data fields to length 0. Most of that you can achieve by filling your buffer with zeros. However, there are no good default values for required fields, enums, sets and composites so it's possible to have a really good "default constructor".

[ujos]

The intent was that data doesn't return "usable" pointer if empty. Actually, in your case assertion is helpful because passing nullptr to memcpy is UB even if size is 0. You can just use dest.assign(std::begin(value), std::end(value)) which is clearer and shorter.

In this case you can return a pointer right after the varLength header. It is like an end pointer in the STL container.

Also, it's not good to use types from detail, they are implementation details. Let me know if there's no way you can avoid it and I'll think about making them public. For example here, can't you just do msg.dataField().assign(std::begin(str), std::end(str)); or at least use auto for the first parameter?

https://github.com/OleksandrKvl/sbepp/issues/24 :)

Nope, sbepp generates a low-level wrapper that avoids doing anything beyond what's written explicitly. SBE doesn't have a notion of default values, the best we can do is to set null values for optional top-level fields, initialize group headers with 0 entries and data fields to length 0. Most of that you can achieve by filling your buffer with zeros. However, there are no good default values for required fields, enums, sets and composites so it's possible to have a really good "default constructor".

Maybe it is not really a good idea to have a default constructor because of variable length fields...

[OleksandrKvl]

In this case you can return a pointer right after the varLength header. It is like an end pointer in the STL container.

This has an overhead because dynamic_array_ref is essentially a pointer, it can be nullptr, in this case there's no way I can return "a pointer after varLength". This would require an if-statement which is not free. I need some time to think about it, maybe it really makes sense to remove !empty() check and just say in documentation that in case size() == 0, the returned pointer is not dereferenceable. Want to check how std::vector::data behaves with debug iterators being enabled.

[ujos]

How it can be null? The header is always there, no? And the data is right after the header, as far as I remember.

[OleksandrKvl]

dynamic_array_ref has reference semantics, it only points to the memory in buffer where data is stored. Default constructed dynamic_array_ref holds nullptr since it doesn't point anywhere.

[ujos]

--In this case you may want to see how string_view is implemented--

Ah.. string_view is a read only object. It may point on empty string. dynamic_array_ref cannot point on some const static object, as one may want to change the size.

[OleksandrKvl]

Yes, dynamic_array_ref is closer to std::span. I checked both libstdc++ and libc++, neither of them triggers assertion when data is used on empty object so I will follow the same approach. Will make the change when I'll have free time.