I am using remove() of std::list to remove elements in a for loop. But it is creating segmentation fault. I am not using iterators. Program is given below.

#include <iostream>
#include <list>

using namespace std;

int main() {
    list <int> li = {1, 2, 3, 4, 5};
    
    for(auto x : li)
    {
        if (x == 4) {
            li.remove(x);
        }
    }

    return 0;
}

In case of iterators, I understand that iterators are invalidated if we remove an element and we need to take care of incrementing iterator properly. But here I am not using iterators and I am using remove() which doesn't return any. Can any one please let me know if we can't use remove in a loop or if there is any issue with the code.

CodePudding user response：

But here I am not using iterators and I am using remove() which doesn't return any. Can any one please let me know if we can't use remove in a loop or if there is any issue with the code.

You are mistaken. Range-based for loops are based on iterators. So after this call:

li.remove(x);

The current used iterator becomes invalid.

For example, in C 17, the range-based for loop is defined in the following way (9.5.4 The range-based for statement):

1 The range-based for statement

for ( for-range-declaration : for-range-initializer ) statement

is equivalent to

{
    auto &&__range = for-range-initializer ;
    auto __begin = begin-expr ;
    auto __end = end-expr ;
    for ( ; __begin != __end;   __begin ) {
        for-range-declaration = *__begin;
        statement
    }
}

Pay attention to that. Using the range-based for loop to call the member function remove() does not make a great sense, because the function will remove all elements with the given value.

If the compiler supports C 20, you can just write:

std::erase( li, 4 );

CodePudding user response：

Instead of walking the list and removing elements alongside, what can cause invalidation of iterators, you can use the erase-remove idiom:

[Demo]

#include <algorithm>  // remove_if
#include <fmt/ranges.h>
#include <list>

int main() {
    std::list<int> li = {1, 2, 3, 4, 5};

    li.erase(
        std::remove_if(std::begin(li), std::end(li), [](int i){ return i == 4; }),
        std::end(li)
    );

    fmt::print("{}", li);
}

// Outputs:
//
//   [1, 2, 3, 5]

CodePudding user response：

Yes, you're using iterators: a range-based for loop uses iterators.

See the Explanation section of https://en.cppreference.com/w/cpp/language/range-for

CodePudding user response：

If the elements in your std::list are unique (as they appear in your example) or if you wish to only remove the first one, add a break; after your remove call.

The range-based-for iterator will be invalidated but inconsequentially so if never used afterwards.