Segfault when using Tree::merge_with with default constructor

[captain-yoshi]

Do I have to initialise either the root node or reserve the arena to clone a Tree from the default constructor Tree() ?

// Much better THX!
ryml::substr loadFileToString(const std::string& path, ryml::Tree& tree)
{
    std::ifstream ifs(path.c_str(), std::ios::in | std::ios::binary | std::ios::ate);

    std::ifstream::pos_type size = ifs.tellg();
    ryml::substr bytes = tree.alloc_arena(size);

    ifs.seekg(0, std::ios::beg);
    ifs.read(bytes.data(), bytes.size());

    std::cout << "Read bytes finished" << std::endl;
    return bytes;
}

int main(int argc, char** argv)
{
    ryml::Tree t;
    ryml::NodeRef n = t.rootref();
    auto substr = loadFileToString("block.yaml", t);

    ryml::parse_in_place("block.yaml", substr, n);

    // create clone
    ryml::Tree clone;

    // segfault
    clone.merge_with(&t, 0, 0);

    // ok
    clone.reserve(substr.size());
    clone.merge_with(&t, 0, 0);

    // ok
    auto n_clone = clone.rootref();
    n_clone |= ryml::SEQ;
    clone.merge_with(&t, 0, 0);

Subquestion: I guess reserving the arena would speed up the merge_with with large data or is it only useful when parsing ?

[captain-yoshi]

[biojppm]

I confess I have trouble understanding what it is about. I cannot delete it, so maybe just update the question to clarify.

[biojppm]

@captain-yoshi sorry, missed the update. This one is actually easy. Looking at the definition of .merge_with():

    void merge_with(Tree const* src, size_t src_node=NONE, size_t dst_root=NONE);

It hints that you should use NONE to merge at the root. That also contemplates your situation, specifically about the prior inexistence of root in a default-constructed tree. Looking at the preamble of the function:

void Tree::merge_with(Tree const *src, size_t src_node, size_t dst_node)
{
    _RYML_CB_ASSERT(m_callbacks, src != nullptr);
    if(src_node == NONE)
        src_node = src->root_id();
    if(dst_node == NONE)
        dst_node = root_id();   // <- here. this will mutate and get you a valid root_id
    _RYML_CB_ASSERT(m_callbacks, src->has_val(src_node) || src->is_seq(src_node) || src->is_map(src_node));
   // ...
}

There is a nagging chicken-and-egg problem with having a root node. The current situation has it so that calling .root_id() on an empty tree (such as a non-const default-constructed tree) will mutate it to get you a valid root node. It's a bit of a trick, I concede.

So other than the call to .reserve() or .rootref() that you came to, any of the following would work:

clone.merge_with(&t); // the standard way
clone.merge_with(&t, t.root_id(), clone.root_id()); // calling root_id() will mutate clone and call reserve()

What you should not do is call with 0. 0 may or may not be a valid root id. You found a case where it isn't. But there's another one: it may also be that due to node shuffling, the root_id may be different from 0. So don't use 0 to signify root id; use root_id() for this.

[biojppm]

See also #272

[captain-yoshi]

Thx for the detailed answer. It makes a lot of sense now. I had misinterpreted that NONE was equal to 0.