Conversion operators not considered

[popizdeh]

struct Test
{
    int x;
    operator int() { return x; }
};

Test t = { 0 };
nlohmann::json j;
j["test"] = t;

This doesn't compile, I was hoping it would use the conversion operator.

[nlohmann]

Conceptually, I think the problem is that Test can be converted to an int, but the assignment j["test"] = t; does not require t to be an int. You could, for instance, add another operator bool() { return x > 0; } or operator std::string() { return std::to_string(x); } and it is ambiguous which operator to take to create a json object.

[TurpentineDistillery]

I think this may be related to the fact that basic_json has multiple constructors accepting types that int may be converted to. j["test"] = static_cast<int>(t) should work.

[popizdeh]

@nlohmann I'm a bit late to respond, sorry. I don't agree with your statement, here's an example to illustrate:

void foo(bool);
void foo(int);

struct Test {};

Test t;
foo(t); // error: Test can't be converted to bool or int

Nothing surprising here, but once you add operator int to Test the code compiles and works as expected. Once you add operator bool you run into ambiguous function call as per normal language rules.

If built-in types can be added to json I don't see why types that are convertible to builtins shouldn't be?

@TurpentineDistillery I don't think that's true, the problems seems to be the templated assignment operator that will match the exact type being assigned and not consider any conversions. Assigning an int selects a different overload.

[nlohmann]

@popizdeh What do you think of my initial comment? I still do not see how a compiler should find out how a json object can be constructed from a Test object.

[popizdeh]

I don't know how easy this would be, I might look into it a bit more when I get some spare time, but in a nutshell we need to change the template assignment operator because it's always a better match. If json supports ints then it needs to support all types that are convertible to int. We just need to find a way to write that assignment operator differently so that implicit conversions are taken into account.

[nlohmann]

So far, for each value type (array, object, string, boolean, floating-point number, unsigned integer number, and signed integer number), we have a constructor like

template<class CompatibleStringType, typename std::enable_if<
             std::is_constructible<string_t, CompatibleStringType>::value, int>::type = 0>
basic_json(const CompatibleStringType& val)
    : basic_json(string_t(val))
{
    assert_invariant();
}

is_constructible seems not to take implicit conversions into account.

I am not sure how an extension to implicit conversions would look like, and whether it would help rather than add new ambiguities.

[nlohmann]

Any ideas on this?

[popizdeh]

I had a quick look, here's what I have so far. Adding a templated assignment operator gets us a bit closer to what I had in mind.

template <typename T>
reference operator=(T t)
{
    *this = basic_json{t};
    return *this; 
}

The problem with this is that it only works for classes whose conversion operator type exactly matches one of basic_json's constructor parameter types. So having a type that implicitly converts to unsigned int still doesn't work as it doesn't know what constructor to select, bool, const int and float are all equally good.

[nlohmann]

Any ideas how to proceed here?

[jaredgrubb]

You could add a Test::operator json() to fix the original issue, but that requires that you can modify Test.

Maybe there's a way to do it via a free function? For example, add a SFINAE constructor to basic_json that accepts any type T where the call "to_json(the_t_value)" works?

[popizdeh]

I guess I could add an operator to my class, or I'll just live with static_cast.

I don't think it's worth going overboard for this. It'd be nice to have but I was hoping there'd be a simple solution. Feel free to close this Niels, I'll send you a patch if I get a better solution.

[nlohmann]

Maybe #328 will be a solution for this. Good that you don't mind closing this. I you find a solution, I'm all ears.

[theodelrieu]

I do think #423 will be the solution, I just added tests with implicit conversions, and it seems to work well.

EDIT: You will need a to_json method in order to get it working, the ambiguity with the conversion operator will remain if you don't