range based цикл у удаление элементов

#include <iostream>
#include <vector>


int main() {

    std::vector<int*> a;
    for (int i = 0; i < 5; i++) {
        a.push_back(new int(rand() % 100 + 5));
    }

    for (auto* l : a) {
        std::cout << l << '\t' << *l << '\n';
        delete l;
        a.erase(a.begin());
    }

}

Я не могу понять, почему этот код приводит к ошибке? Это как-то связано с тем, что erase создает новый вектор в другой памяти, а цикл использует старые begin и end?


Ответы (1 шт):

Автор решения: Stanislav Volodarskiy

В случае range-based for loop компилятор раскрывает цикл

for (auto *l : a)
{
    ...
}

в

for (auto begin = a.begin(), end = a.end(); begin != end; ++begin)
{
    auto *l = *begin;
    ...
}

Обратите внимание, что выражение a.end() вычисляется один раз в начале цикла и сохраняется в переменной end.

В теле цикла вы вызываете a.erase(a.begin()), который делает недействительным итератор a.end(). erase:

Iterators (including the end() iterator) and references to the elements at or after the point of the erase are invalidated.

Следовательно после первой итерации цикла сохранённое значение end() устарело и программа делает что угодно (неопределённое поведение).

Даже если вы замените цикл на явный, который вычисляет a.end() каждый раз, ошибка всё равно останется. В документации сказано что становятся недействительными все итераторы начиная с первого удалённого. То есть, begin в этом коде тоже недействительный. Этот код не работает и это снова неопределённое поведение, с которым шутить не стоит:

for (auto begin = a.begin(); begin != a.end(); ++begin)

Правильный способ состоит в том чтобы не хранить итераторы совсем. Так будет работать:

while (a.begin() != a.end())
{
    auto *l = *a.begin();
    std::cout << l << '\t' << *l << '\n';
    delete l;
    a.erase(a.begin());
}

Но и с ним есть проблема. Он медленный. Каждое удаление элемента в начале вектора приводит к смещению всех остальных элементов влево на одну позицию. Например для вектора из миллиона элементов в цикле будет сделано примерно пятьсот миллиардов копирований. Не надо так делать.

Правильный и быстрый код такой:

for (auto *l : a)
{
    std::cout << l << '\t' << *l << '\n';
    delete l;
}
a.clear();  // очистка вектора после цикла
→ Ссылка