danielaparker / jsoncons

A C++, header-only library for constructing JSON and JSON-like data formats, with JSON Pointer, JSON Patch, JSON Schema, JSONPath, JMESPath, CSV, MessagePack, CBOR, BSON, UBJSON
https://danielaparker.github.io/jsoncons
Other
699 stars 158 forks source link

Allocator support incompatible with scoped allocator adaptor #419

Closed ergpudb closed 1 year ago

ergpudb commented 1 year ago

Using a scoped allocator adaptor (i.e. an allocator wrapped with std::scoped_allocator_adaptor) with jsoncons results in compilation errors.

Example:

template<typename T>
class TestAllocator
{
public:
    using value_type = T;

    TestAllocator(int i) noexcept :
        m_i(i)
    {
    }

    TestAllocator(const TestAllocator& other) noexcept :
        m_i(other.m_i)
    {
    }

    template<typename U>
    TestAllocator(const TestAllocator<U>& other) noexcept :
        m_i(other.get_i())
    {
    }

    T* allocate(std::size_t n)
    {
        return (T*)::operator new(n * sizeof(T));
    }

    void deallocate(T* ptr, std::size_t n) noexcept
    {
        ::operator delete(ptr);
    }

    int get_i() const noexcept
    {
        return m_i;
    }

private:
    int m_i;
};

template<typename T, typename U>
bool operator==(const TestAllocator<T>& a, const TestAllocator<U>& b) noexcept
{
    return a.get_i() == b.get_i();
}

template<typename T, typename U>
bool operator!=(const TestAllocator<T>& a, const TestAllocator<U>& b) noexcept
{
    return a.get_i() != b.get_i();
}

template<typename T>
using ScopedTestAllocator = std::scoped_allocator_adaptor<TestAllocator<T>>;

...

using xjson = jsoncons::basic_json<char, jsoncons::sorted_policy, ScopedTestAllocator<char>>;
xjson test(0, jsoncons::semantic_tag::none, ScopedTestAllocator<char>(1));

Not being super familiar with the structure of the code I'm not quite sure where the problem lies; however I suspect that there are situations where the allocator model isn't implemented quite correctly.

(One thing that jumps out at me is that e.g. the basic_json(basic_json&& other, const Allocator&) constructor ignores the allocator parameter, but the provided allocator should be used instead of other's allocator; similarly, the move assignment operator should either move the source's allocator or not depending on propagate_on_container_move_assignment. These may or may not be related to scoped allocator adaptor failing, but they are ways in which the implementation diverges from the standard model that scoped allocator adaptor relies upon.)

Tested with v0.170.0, C++17, gcc 11.3, x64, Linux.

danielaparker commented 1 year ago

Thanks for raising this, I understand the issue with scoped allocator adaptor, but it requires some thought. It will take a while to look into this.

danielaparker commented 1 year ago

std::scoped_allocator_adaptor is now supported in branch PolymorphicAllocator, see #424

danielaparker commented 1 year ago

This is now on master.