breese / trial.protocol

Network wire protocols
11 stars 4 forks source link

reader.value<std::string>() should allow the allocator to be provided #42

Closed vinipsmaker closed 4 years ago

vinipsmaker commented 4 years ago

So the returned string uses the provided allocator. A new overload like this:

template<class Alloc> inline
std::basic_string<char, std::char_traits<char>, Alloc>
reader::value<std::basic_string<char, std::char_traits<char>, Alloc>>(
    const Alloc& alloc) const;
breese commented 4 years ago

Sounds good.

Does 189c484e2a7c942fb55067d63db3aa7060645173 fix this?

vinipsmaker commented 4 years ago

Does 189c484 fix this?

Not quite. This commit only customizes the type. std::string is an AllocatorAware container, so it should have stateful allocators where the passed instance is used (not only the type).

A new overload that allows the allocator instance to be passed as an extra argument is still lacking.

Something like (piece copied and edited from the commit's test):

std::scoped_allocator_adaptor<std::allocator<char>> alloc;
TRIAL_PROTOCOL_TEST_EQUAL(reader.value<string_type>(alloc), "alpha");
breese commented 4 years ago

Rather than having a special solution for passing an allocator to a string, we could make a more general solution where the result is provided as an output parameter.

For example

std::scoped_allocator_adaptor<std::allocator<char>> alloc;
std::string result(alloc);
reader.value(result); // throws on error
assert(result == "alpha");

The solution can also be extended to integers, floating-points, etc.

vinipsmaker commented 4 years ago

It works for me. And indeed is more general as it can allow patterns such as:

std::scoped_allocator_adaptor<std::allocator<char>> alloc;
std::string result(alloc);
result.reserve(reader.literal().size());
reader.value(result); // throws on error
assert(result == "alpha");
breese commented 4 years ago

Added in 9d27f7fb530c244e139bf2bbd4d0423e39f86cd8