Iterative destruction of JSON containers (objects and arrays)

[mistersandman]

Deeply nested JSON hierarchies may lead to stack overflows during destruction, since JSON containers call the destructor of their children recursively (see issue #832).

This change provides an iterative destruction of JSON containers, avoiding a deep call stack by maintaining an explicit stack on the heap.

[nlohmann]

Thanks for the PR!

[coveralls]

Coverage decreased (-0.02%) to 99.976% when pulling df42d48 on mistersandman:iterative_container_destruction into e326df2 on nlohmann:develop.

[jaredgrubb]

I applaud your effort here and the PR looks correct. However I found it very difficult to audit and read -- which makes me worry that it will be difficult to maintain.

The hardest thing here is that there are many levels of indentation, and worse, two nested while loops.

I think splitting out the loops into separate functions (and avoiding having to split the top-level switch-case) makes it much easier to audit and understand what it's doing.

Eg, a rough sketch:

               case value_t::object:
               {
                    if (!this->object->empty())
                    {
                         destroy_subelements_depth_first(this);
                    }

                    AllocatorType<object_t> alloc;
                    std::allocator_traits<decltype(alloc)>::destroy(alloc, object);
                    std::allocator_traits<decltype(alloc)>::deallocate(alloc, object, 1);
                    break;
                }
                //// Ditto for array ....

And then:

void destroy_subelements_depth_first(json_value *value)
{
    std::vector<std::pair<json_value*, value_t>> stack;
    stack.emplace_back(this, t);
    while (!stack.empty())
    {
        json_value* value;
        value_t type;
        std::tie(value, type) = stack.back();
        bool all_done;
        if (type == value_t::object)
        {
             all_done = destroy_object_subelements(stack, value);
        }
        else
        {
             all_done = destroy_array_subelements(stack, value);
        }
        if (all_done)
        {
             stack.pop_back();
        }
    }
}

[mistersandman]

@jaredgrubb Thank you very much for your feedback and sorry for the late reply! I changed the code pretty much exactly according to your suggestion and added a few comments. It is indeed much better readable now 👍

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.