fix: Graph rename bug fix

[Danbr4d]

Closes #1594.

[Danbr4d]

Not sure how big an issue is but if you rename to something with a number in then it causes a crash too.

[trisyoungs]

@Danbr4d the issue with renaming I think boils down to the function which was renaming (badly) the relevant data. I have updated that particular function in the best way possible. @rprospero if you wouldn't mind taking a look over the horrible code in question, that'd be great (I'll ask for a review from you). I don't see any other way of achieving the same thing...

[rprospero]

@trisyoungs I'm not entirely sure that we need to copy into the other map. The cpp reference seems to indicate that it is a safe action. The only iterator that is invalidated is the one which is literally being moved. I think we can do

for (auto iter = m.begin(); iter < m.end(); ++m) {
    auto node = m.extract(iter);
    if (test(node.key())) {
        node.key() = new_key(node.key());
        m.insert(std::move(node));
    }
}

Even if the iteration invalidation causes an issue, we can still go O(n²) in access.

do {
    bool done = true
    for (auto iter = m.begin(); iter < m.end(); ++m) {
        auto node = m.extract(iter);
        if (test(node.key())) {
            node.key() = new_key(node.key());
            m.insert(std::move(node));
        }
        done = false;
        break;
    }
} while(!done)

It's still ugly, but at least it's constant memory.

[trisyoungs]

@rprospero You're aboslutely right - not sure why I got stuck in the copying methodology. I'm still convinced that there are issues trying to do this in a single loop, so I have reworked in line with your second suggestion, but in a slightly more std algorithm way.