Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dump() method doesn't work with a custom allocator #268

Closed
michsco opened this issue Jun 21, 2016 · 4 comments
Closed

dump() method doesn't work with a custom allocator #268

michsco opened this issue Jun 21, 2016 · 4 comments
Labels
kind: bug solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) state: please discuss please discuss the issue or vote for your favorite option

Comments

@michsco
Copy link

michsco commented Jun 21, 2016

Even if #161 isn't fixed, the dump() method still assumes the default allocator is used. Consider the following code:

nlohmann::basic_json<std::map, std::vector, StringWithMyAllocator, bool, int64_t, uint64_t, double, MyAllocator> j;

// add a string that is stored as std::string
j["name"] = "Niels";

// explicit conversion to string
StringWithMyAllocator s = j.dump();    // {\"happy\":true,\"pi\":3.141}

This results in a compiler error:
'return': cannot convert from 'std::basic_string<char,std::char_traits,std::allocator>' to 'std::basic_string<char,std::char_traits,MyAllocator>'

The issue is due to the stringstream that is used without specifying the allocator for the stream. As a result, dump fails to compile due to the call to ss.str().

string_t dump(const int indent = -1) const
{
    std::stringstream ss;

    if (indent >= 0)
    {
        dump(ss, true, static_cast<unsigned int>(indent));
    }
    else
    {
        dump(ss, false, 0);
    }

    return ss.str();
}
@nlohmann
Copy link
Owner

I actually never tried to use custom allocators in the string type. Your example shows that there is an apparent disconnect between the (in theory) adjustable basic_json::string_t and the (implicitly assumed) std::string returned by str().

The quick "fix" would be to replace the return type of dump from basic_json::string_t to std::string, because the type used to store strings inside a JSON value does not necessarily need to be the same type of a serialization.

Alternatively, the user-defined allocator inside basic_json::string_t would need to be passed down into the string stream.

@nlohmann
Copy link
Owner

I created a branch feature/issue268 (see a834338) which fixes the compilation error. Please let me know whether this works for you.

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jun 22, 2016
@nlohmann
Copy link
Owner

Thanks for checking back, and I am looking forward to see (the definitely needed) fixes!

@michsco
Copy link
Author

michsco commented Jun 23, 2016

I created a fork of the project and a new branch feature/custom-allocator that contains initial changes. There are still a lot of assumptions built into the code using the default allocator for std::vector, std::map, and std::string.

@nlohmann nlohmann added the solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) label Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

2 participants