0

I have a std::map<int, std::string> and my aim is to remove a key-value pair based on a certain condition. For simplicity, let say, it looks like this:

int main(int argc, char const *argv[]) {
    std::map<int, std::string> m1;

    m1[-2] = "minus two";
    m1[-1] = "minus one";
    m1[0] = "zero";
    m1[3] = "three";
    m1[4] = "four";

    std::cout << m1.size() << std::endl;

    for (auto &&[key, val] : m1) {
        std::cout << key << ": " << val << std::endl;
        if (key == -2 || key == 3) 
            m1.erase(key);
    }

    std::cout << m1.size() << std::endl;
}

Everything looks fine. However, it gives seg fault upon running:

5
-2: minus two
0: 
Segmentation fault (core dumped)

I noticed that this only occurs when I try to erase the very first key-value pair e.g. in the above case: -2: "minus two". If I try to erase any other key-value pair (e.g. second key-value pair: -1: "minus one") then I don't get seg fault, e.g.

    std::cout << m1.size() << std::endl;

    for (auto &&[key, val] : m1) {
        std::cout << key << ": " << val << std::endl;
        if (key == -1 || key == 3) 
            m1.erase(key);
    }

    std::cout << m1.size() << std::endl;

I also noticed that if I erase the first key-value pair outside of the for loop then I don't get seg fault e.g.

    std::cout << m1.size() << std::endl;
    m1.erase(-2);

    for (auto &&[key, val] : m1) {
        std::cout << key << ": " << val << std::endl;
        if (key == -2 || key == 3) 
            m1.erase(key);
    }

    std::cout << m1.size() << std::endl;

One more thing that I noticed is that if there are only two key-value pairs then I don't get seg fault:

    m1[-2] = "minus two";
    m1[4] = "four";

    std::cout << m1.size() << std::endl;

    for (auto &&[key, val] : m1) {
        std::cout << key << ": " << val << std::endl;
        if (key == -2 || key == 3) 
            m1.erase(key);
    }

    std::cout << m1.size() << std::endl;

So, is this an expected behavior? If yes then why? Also, in that case, how can I remove/erase the first key-value pair in the for loop itself? or I have to remove/erase them outside of the for loop?

Milan
  • 1,743
  • 2
  • 13
  • 36
  • 1
    In many languages it is not safe to delete from a container you are iterating over, this includes `std::map` inside a for each loop. – Quimby May 10 '21 at 15:29
  • @Quimby Thanks but why? We have unique keys and we are deleting by `key` not by index then why it's not safe? I should have asked this in the main question but as the question is closed would greatly appreciate it if you can share some more details or some useful links. Thank you so much in advance! – Milan May 10 '21 at 15:50
  • @Quimby In the case of `std::map`, first I thought it has something to do with removing the root node from the tree and the first key-value being a root node but somewhere I read that it's a balanced binary tree. So, I guess the first key-value pair doesn't have to be the root node, right? – Milan May 10 '21 at 18:27
  • 1
    Ups,sorry I missed the comment. Yes, it has to be the root, but that is not the issue. The issue is iterating over any container requires keeping a state - e.g. current index in the array which will be iteratively incremented or a pointer to tree node in `std::map`. Then, when you delete an element, the state can become invalid - e.g. current node seizes to exist and so cannot be used to get the next one. Worse, the state usually won't know it is invalid, hence the segfault. In theory it can be made to work but it is much easier to write "it is unsafe to modify containers while iterating". – Quimby May 10 '21 at 18:52
  • 1
    The implementation would have to keep track of who is currently iterating the container, that can be very expensive and really anti-C++. Just to clarify, it is not unsafe to delete the element, it is unsafe to keep iterating the container after deleting the element. See the duplicate question for the proper way how to restore the state. – Quimby May 10 '21 at 18:56

0 Answers0