A colleague writes a function to remove all negative numbers from a vector of sensor readings. It looks clean, it compiles without warnings, and it passes a quick smoke test. Ship it?

#include <vector>
#include <iostream>

void remove_negatives(std::vector<double>& readings) {
    for (auto it = readings.begin(); it != readings.end(); ++it) {
        if (*it < 0.0) {
            readings.erase(it);
        }
    }
}

int main() {
    std::vector<double> data = {1.2, -3.4, 5.6, -7.8, -9.0, 2.1};
    remove_negatives(data);
    for (double d : data) {
        std::cout << d << " ";
    }
    // Expected: 1.2 5.6 2.1
}

Run it a few times. Sometimes you get 1.2 5.6 2.1. Sometimes you get 1.2 5.6 -9.0 2.1. Sometimes it segfaults. The behavior depends on your standard library implementation, optimization level, and the phase of the moon.

The Bug

There are actually two intertwined problems here, both stemming from the same root cause: iterator invalidation.

Problem 1: Using an invalidated iterator. When you call std::vector::erase(it), every iterator at or after the erased position is invalidated. The C++ standard says that incrementing, dereferencing, or comparing an invalidated iterator is undefined behavior. The ++it in the loop header operates on an invalidated iterator. In practice, many implementations happen to make the old iterator point to the next element after erasure — but that's an implementation accident, not a guarantee. Under aggressive optimization or with hardened debug iterators, this will crash.

Problem 2: Skipping elements. Even on implementations where the invalidated iterator "works," there's a logic bug. erase shifts all subsequent elements left by one. The element that was at position it+1 is now at position it. Then ++it advances past it without ever checking it. This is why consecutive negatives like -7.8, -9.0 cause -9.0 to survive: after erasing -7.8, the value -9.0 slides into its slot and the iterator jumps over it.

The fix is straightforward. erase returns a valid iterator to the element that followed the erased one. Use it:

void remove_negatives(std::vector<double>& readings) {
    for (auto it = readings.begin(); it != readings.end(); ) {
        if (*it < 0.0) {
            it = readings.erase(it);  // erase returns next valid iterator
        } else {
            ++it;                     // only advance if we didn't erase
        }
    }
}

Note the subtle change: ++it is removed from the loop header and placed in the else branch. When we erase, erase already advances us; when we don't erase, we advance manually. This eliminates both the undefined behavior and the skipping bug.

An even more idiomatic C++ approach is the erase-remove idiom:

readings.erase(
    std::remove_if(readings.begin(), readings.end(),
                   [](double d) { return d < 0.0; }),
    readings.end());

Or in C++20, simply std::erase_if(readings, [](double d) { return d < 0.0; });

This bug is especially insidious because it often works. The undefined behavior manifests as correct output on common toolchains with small datasets. It's the kind of defect that passes unit tests for months, then surfaces in production when the data changes shape and two negatives land next to each other — or when you upgrade your compiler and the optimizer gets more aggressive.

The same invalidation rules apply to std::deque, std::string, and std::unordered_map (during rehash). Each container has its own invalidation rules — learn them or suffer intermittently.