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

nlohhman::to_string() default implementation shadows user defined one #4014

Open
2 tasks
namaenonaimumei opened this issue Apr 17, 2023 · 2 comments
Open
2 tasks
Labels
kind: bug state: needs more info the author of the issue needs to provide more details

Comments

@namaenonaimumei
Copy link

namaenonaimumei commented Apr 17, 2023

Description

I am not really sure how to go about using user-defined nlohhman::to_string() (in a consistent manner).

Is there any reason for this template to be implemented on nlohmann side at all (since it's pretty much .dump() alias anyway).

If this wasn't intended, is there any chance to alter this behaviour.

Reproduction steps

Example for context below, granted this template behaviour is to be expected so I'm not really sure how provided example was supposed to work in the first place, unless user version is not supposed to be implemented inside nlohmann, which seems inconsistent for using with rest of the API (that or I missed something obvious in documentation linked).

Expected vs. actual results

Removing example implementation and leaving implementing up to user or providing macro to enable default one seems like a good alternative.

Minimal code example

Example user implementation

namespace nlohmann // same for namespace "ns" in examples below
{
template<typename BasicJsonType>
std::string to_string(const BasicJsonType &j)
{
    if (j.type() == nlohmann::json::value_t::string)
        return j.template get<std::string>();
    else
        return j.dump();
}
}

Example calls

// All examples below prioritize internal/first implementation
std::cout << nlohmann::to_string(jsonobj["key"]) << std::endl;
std::cout << to_string(jsonobj["key"]) << std::endl;
{
    using namespace nlohmann;
    std::cout << to_string(jsonobj["key"]) << std::endl;
}
{
    using namespace ns;
    std::cout << to_string(jsonobj["key"]) << std::endl;
}
// Calls like these still work
std::cout << ns::to_string(jsonobj["key"]) << std::endl;

Error messages

No response

Compiler and operating system

gcc 12.2.1

Library version

3.11.2

Validation

@nlohmann
Copy link
Owner

I am not exactly sure what you propose? Removing the to_string member function?

@nlohmann nlohmann added the state: needs more info the author of the issue needs to provide more details label Apr 22, 2023
@namaenonaimumei
Copy link
Author

That (and leaving documentation as it is) or guarding default implementation with some guard macro (and updating documentation to reflect that).
I lean more towards removal as it's just another .dump() and doesn't seem necessary in its current form.
But if you deem previous behaviour as a vital part of default API later may be a better choice.

For second option something around the lines of below for actual usage

// json_ext.h (user header, included instead of json.hpp by other units)
#define JSON_TO_STRING_IMPLEMENTED
#include <json.hpp>
namespace nlohmann
{
template<typename BasicJsonType>
std::string to_string(const BasicJsonType &j)
{
    if (j.type() == nlohmann::json::value_t::string)
        return j.template get<std::string>();
    else
        return j.dump();
}
}

// json.hpp
#ifndef JSON_TO_STRING_IMPLEMENTED
/// @brief user-defined to_string function for JSON values
/// @sa https://json.nlohmann.me/api/basic_json/to_string/
NLOHMANN_BASIC_JSON_TPL_DECLARATION
std::string to_string(const NLOHMANN_BASIC_JSON_TPL& j)
{
    return j.dump();
}
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug state: needs more info the author of the issue needs to provide more details
Projects
None yet
Development

No branches or pull requests

2 participants