Arithmetic operators are not working as expected

[JunseongAHN]

Description

The default arithmetic operators, +, - and / for json object consider the value of the json object as int regardless of the actual value type.

This behavior is not specified in the documentation. It should be mentioned (or warned) in the documentation. This unexpected behavior from the default operators can easily mislead people.

Note: It can be naively fixed with the casting operator (e.g. double(json_object))

Reproduction steps

Straight forward. run the minimum reproducible example.

Expected vs. actual results

Expected Result

3
0
2.25
1

Actual Result

2.5
0.5
1.5
1.5

Minimal code example

main.cpp

#include <iostream>
#include <nlohmann/json.hpp>
using json = nlohmann::json;

void main()
{
    json data;
    data["test"] = 1.5;
    std::cout << 1.5 + data["test"]<< std::endl;
    std::cout << 1.5 - data["test"] << std::endl;
    std::cout << 1.5 * data["test"] << std::endl;
    std::cout << 1.5 / data["test"] << std::endl;
}

Error messages

No response

Compiler and operating system

MSVC C++ 20, Windows 10

Library version

3.11.2

Validation

• The bug also occurs if the latest version from the develop branch is used.
• I can successfully compile and run the unit tests.

[falbrechtskirchinger]

There are no operator overloads, so you're simply observing the interaction of implicit conversion with the default arithmetic operators. One more reason to disable implicit conversion.

It's not immediately apparent to me where to best document this behavior.

Another way to address this would be to add operator overloads and do the Right Thing:tm:. People will disagree on what that is when it comes to non-numeric types. (I can't find the issue that discussed adding operator+ for objects/arrays. It's not #1019.)

[JunseongAHN]

Agree that no apparent place to mention this behavior. However, https://json.nlohmann.me/features/types/number_handling/ might be an option to consider...

It would be great if the library provide the default arithmetic operators at least for json numeric types (e.g json::number_float_t). It is also natural since the operator == for json::number_float_t is already overloaded with default == in double (as discussed in #703),

[falbrechtskirchinger]

I had some more time to think about this. Forwarding the operators to the underlying/stored type seems doable and sidesteps the question of what + applied to arrays and objects should do. It will simply fail with the default types.

The only reason I still hesitate is the sheer number of operators: There are 13 arithmetic operators and 10 compound assignment operators. Each operator needs several implementations for the various combinations of types, but not all operators have to be implemented for every type (e.g., bitwise operators are only needed for integral types).

These operator overloads should only be defined if implicit conversion is enabled ('JSON_USE_IMPLICIT_CONVERSIONS == 1`).

[JunseongAHN]

Assuming from what you commented, it is required to implement the basic_json operators for any value types at once...? (I don't understand how the basic_json class works...) but I ask to make sure... Could implementing these be done step by step?

I am wondering if it is possible to overload operators that can be apply for any numeric types first. There are a lot of operators for the numeric type, but the only few the arithmetic / compound assignment operators exist to apply to any numeric types. Additionally, Implementing operators for objects needs many efforts to define what operators mean with everyone's agreement, and it needs some careful consideration.

[falbrechtskirchinger]

Assuming from what you commented, it is required to implement the basic_json operators for any value types at once...?

Currently, without any operator overloads defined, any arithmetic operation with a basic_json and a numeric operand is well defined. That doesn't mean it's intuitive, but the rules of language dictate the outcome.
Once some (but not all) operators are implemented, that would no longer be the case and lead to unnecessary confusion.

(I don't understand how the basic_json class works...) but I ask to make sure... Could implementing these be done step by step?

It could, but I don't think that's a good idea for the reason stated above.

I am wondering if it is possible to overload operators that can be apply for any numeric types first.

As long as all operators are implemented, I have no objections to supporting numeric types only.

[gregmarr]

Since we've said that we want to eventually get rid of implicit conversions, I don't think adding operator overloads is a good thing. Even under JSON_USE_IMPLICIT_CONVERSIONS == 1, the addition would be condoning the use of implicit conversions.

[JunseongAHN]

@gregmarr I read the documentation regarding JSON_USE_IMPLICIT_CONVERSIONS and realized the library will change the default value of JSON_USE_IMPLICIT_CONVERSIONS to 0 in the next update.
In that case, consequences of changing JSON_USE_IMPLICIT_CONVERSIONS is on developer's hand, which means operator overloads are not necessary as you mentioned...

However, as long as JSON_USE_IMPLICIT_CONVERSIONS option exists, I believe it is necessary to deal with this behavior under JSON_USE_IMPLICIT_CONVERSIONS == 1 somehow (e.g., warn people in the docs. It is because it is quite hard to catch this.

For example, for the data json object in the minimal reproducible example, cout << data["test"] displays float value 1.5. A user easily misunderstands data["test"] as the actual float type value...

By the way, I am wondering the major reason to turn off the implicit conversions by default. This is was one of features I got impressed of, compared to other json parsing libraries.

[falbrechtskirchinger]

By the way, I am wondering the major reason to turn off the implicit conversions by default. This is was one of features I got impressed of, compared to other json parsing libraries.

See #958.

[gregmarr]

I have no problem with adding something to the docs.

FYI, both Clang and GCC consider your sample to be ambiguous and fail to compile.

[JunseongAHN]

@gregmarr Oh, I made a mistake when copying and pasting. I just edited it, but it is not related to ambiguity of operators. You mean the following example failed to compile in GCC? If the sample only works on the specific compilers, operator overloads for implicit conversion don't make sense.

json test;
test = 1.5;
std::cout << test + 1.5 << std::endl;

[JunseongAHN]

By the way, I am wondering the major reason to turn off the implicit conversions by default. This is was one of features I got impressed of, compared to other json parsing libraries.

See #958.

Thanks, there were already careful, considerate discussion regarding this issue. There are caveats in the both options, but turning off the implicit conversion looks like a better option for stability and maintenance of the library.

I believe adding operator overloads when implicit conversion is on only for numeric types can mitigate this issue.... @gregmarr a lot of issues in #958 were from implicit conversion from complex types (e.g std vectors, user defined classes) There is one issue related to boolean type, but I believe this was caused since implicit conversion is on by default, but implicit conversion itself.

[nlohmann]

I don't think investing effort into all these conversions makes sense.

[JunseongAHN]

Agree. I just realized that it is a compiler dependent issue. Sorry for the confusion.