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

Library provides template arg for string_type but assumes std::string in some places #2059

Closed
bcollins526 opened this issue Apr 22, 2020 · 4 comments
Labels
kind: bug state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@bcollins526
Copy link

I'm using json object with Electronic Arts' EASTL library, which provides high performance implementations of std containers. I use the following typedef:

    using stdjson = nlohmann::basic_json<std::map,
                                         std::vector,
                                         stdstring,
                                         bool,
                                         int64_t,
                                         uint64_t,
                                         double,
                                         jsonallocator,
                                         nlohmann::adl_serializer>;

where stdstring is eastl::string, which is their version of std::string. Then throughout my code, I just always use stdjson type. This works great, except for when json code assumes std::string. For example, this code:

   for (auto& cam : sjson["cameras"].items()) {
        stdstring cameraname(cam.key());
    }

fails to compile because of this code in iteration_proxy.hpp:

template<typename string_type>
void int_to_string( string_type& target, std::size_t value )
{
    target = std::to_string(value);
}

which assumes std::string is the string_type. I'm using Visual Studio 2019 on Windows 10 using the latest release of json.

@ArtemSarmini
Copy link
Contributor

ArtemSarmini commented Apr 28, 2020

It assumes that string_type is move-assignable or implicitly-move-constructible from std::string which is not true for eastl::string. To fix it we may add some sfinae to construct string_type either from std::to_string(value) or from std::to_string(value).data() which I hope will return something meaningful for all string_types we care of.
Another option is to introduce like convertion_traits in json template args which will handle such stuff. If one uses nonstandard string, one provides nonstandard traits, simple and neat. (It is also a possibility to consider opting in to use std::from_chars. ) Downside is that it is a very major change in api and requires quite lots of work: new code, new tests, new docs.
@nlohmann thoughts?

@stale
Copy link

stale bot commented May 30, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label May 30, 2020
@stale stale bot closed this as completed Jun 6, 2020
@t-b
Copy link
Contributor

t-b commented Jun 6, 2020

To fix it we may add some sfinae to construct string_type either from std::to_string(value) or from std::to_string(value).data() which I hope will return something meaningful for all string_types we care of.

That sounds like a good idea.

@nlohmann nlohmann removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jun 6, 2020
@nlohmann nlohmann reopened this Jun 6, 2020
@stale
Copy link

stale bot commented Jul 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jul 6, 2020
@stale stale bot closed this as completed Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

4 participants