stephenberry / glaze

Extremely fast, in memory, JSON and interface library for modern C++
MIT License
1.23k stars 123 forks source link

Request for auto numeric variant deduction #1400

Closed sweihub closed 4 weeks ago

sweihub commented 4 weeks ago

Hi Dear Author

I tested the variant, it seems not working at all, code

void test()
{
    using MixedType = std::variant<std::string, int, double, bool>;
    std::map<std::string, MixedType> map;

    auto data = R"( { "key1": 33, "key2": "foobar", "key3": true, "key4": 3.14} )";
    glz::read_json(map, data);

    for (auto &[key, value] : map) {
        std::cout << key << std::endl;
    }

    assert(std::get<int>(map["key1"]) == 33);
    assert(std::get<std::string>(map["key2"]) == "foobar");
    assert(std::get<bool>(map["key3"]) == true);
    assert(std::get<double>(map["key4"]) == 3.14);
}

I'm using the main which I updated around Date: Tue Oct 24 11:07:28 2023 +0800, the latest main completely breaks my build, so not testing it.

stephenberry commented 4 weeks ago

The issue is that your variant is not auto-deducible because you have both an int and a double. There is no way to know which one to decode from a JSON number. When a type is not auto-deducible then Glaze will try to decode the active type, which in this case will be a std::string because that will be default initialized in the map as is the first type in your variant.

See Variant Handling documentation for more details.

If you printed your error code then you would see:

1:12: expected_quote
    { "key1": 33, "key2": "foobar", "key3": true, "key4": 3.14} 
              ^

This is saying that it expects a string, but you have provided an integer.

There is an active issue concerning numeric variant auto-deduction: #1079 I'm currently optimizing the integer parsing and making it more restrictive, once this is finished it will be more straightforward to implement this feature. However, it means that every numeric value will need to be parsed twice if it is a floating point value, given that there are both integer and floating point types in the double. First, Glaze will have to attempt an integer parse, and if that fails, then parse into the floating point field. This will increase overhead, but will solve the problem you have encountered.

Is performance critical here for you? If so, I would remove your int and just cast your double as needed. Otherwise, I can try to speed up the fix for auto numeric variant deduction.

sweihub commented 4 weeks ago

Hi, Stephen

That solved my issue, thanks for so prompt response. I think remove the int is reasonable, since the JSON standard just supports NUMBER which is either int or double. In order to make it simple, I would recommend glaze to only support string, double and bool primitives, and just document it. So no hurry to support int.