Give std.json an opEquals

[smolt]

Add opEquals to struct JSONValue. Needed for correct comparison of the union used for member store. Without this change, some std.json unittests will fail on iOS armv7.

[smolt]

BTW, just noticed related Issue 14107 "compiler shouldn't allow to compare unions without custom opEquals".

[monarchdodra]

So, INTEGER and UINTEGER will always compare false. Is this the correct behavior? If yes, I thnk it warrants documenting the operator's semantics.

Other than that, seems good.

[smolt]

Without opEquals, that was the old behavior. But I hadn't thought of that case. Maybe it is better to match D native types here? Which ever way to got (as is, or change so INTEGER and UINTEGER could match), I think I should add a test for it too.

Thoughts on best way to go?

[smolt]

Ok, I think UINTEGER and INTEGER should always compare false because:

1. that's the current behavior, and
2. std.variant.Variant behaves in a similar fashion

    JSONValue s = 42;
    JSONValue u = 42u;
    assert(s != u);
    assert(s.integer == u.uinteger);

    Variant sv = 42;
    Variant uv = 42u;
    assert(sv != uv);
    assert(sv.get!int == uv.get!uint);

I'll add some tests and comments.

[monarchdodra]

OK. That makes sense to me. As long as we justify our choices and document them, I'm fine.

[MartinNowak]

Hit something similar in Higgs recently. higgsjs/Higgs#177

[MartinNowak]

All the numbers should probably be compared as double precision float, because that's the sematic of JSON. https://en.wikipedia.org/wiki/JSON#Data_types.2C_syntax_and_example
That belongs in a separate pull though, and might break code/assumptions.

[MartinNowak]

How is it even decided what to parse as int/uint/float?

[MartinNowak]

Auto-merge toggled on

[smolt]

I didn't add doc to explain opEqual behavior. Should I still do it and update pull request?

I just looked at parseJSON(). It deduces:

• FLOAT if number has a decimal point or has exponent,
• UINTEGER if value is > long.max
• else INTEGER

To me it seems like too much for a user of std.json to understand. Maybe just use double always? And cast when asking for integer or uinteger property?

At this point, I just wanted to get changes needed for iOS to pass unittests in master. Should I file an enhancement for changing json number handling? (I am new to this part of the D society).

Also, I noticed round trip yields a different result:

    JSONValue js0 = parseJSON(`{"f" : 42.0}`);
    assert(js0["f"].type == JSON_TYPE.FLOAT);

    auto str = j.toPrettyString;
    JSONValue js1 = parseJSON(str);
    assert(js1["f"].type == JSON_TYPE.INTEGER);

Anyway, thanks for help.

[JakobOvrum]

Providing no documentation for JSONValue.opEquals surely signals that its behaviour is so obvious it warrants no explanation. But we know the implementation includes murky details like signed vs unsigned integers, and equality comparison between floating-point numbers (ugh). This function desperately needs documentation.

As this PR has already been merged, you'd have to make a new PR with further changes.

edit:

It also needs unit tests...