11

I have a vector. I need to delete the last 3 elements in it. Described this logic. The program crashes. What could be the mistake?

vector<float>::iterator d = X.end();
    for (size_t i = 1; i < 3; i++) {
        if (i == 1) X.erase(d);
        else X.erase(d - i);
    }
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
dbUser11
  • 305
  • 1
  • 10
  • The killer here is `d` doesn't really exist. It's the one-past-the-end canary value usable only to find the end of the `vector`. You can't remove it. Next, as soon as you erase an iterator, it's gone. You can't safely use it afterward for anything, including `d - i`. – user4581301 Apr 15 '20 at 20:52

7 Answers7

11

If there are at least 3 items in the vector, to delete the last 3 items is simple -- just call pop_back 3 times:

#include <vector>
#include <iostream>

int main() 
{
    std::vector<float> v = { 1, 2, 3, 4, 5 };
    for (int i = 0; i < 3 && !v.empty(); ++i)
       v.pop_back();

    for ( const auto &item : v ) std::cout << item << ' ';
        std::cout << '\n';
}

Output:

1 2
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
11

It is undefined behavior to pass the end() iterator to the 1-parameter erase() overload. Even if it weren't, erase() invalidates iterators that are "at and after" the specified element, making d invalid after the 1st loop iteration.

std::vector has a 2-parameter erase() overload that accepts a range of elements to remove. You don't need a manual loop at all:

if (X.size() >= 3)
    X.erase(X.end()-3, X.end());

Live Demo

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
3

You could use a reverse_iterator:

#include <iostream>
#include <vector>

using namespace std;

int main()
{
    vector<float> X = {1.1, 2.2, 3.3, 4.4, 5.5, 6.6};

    // start the iterator at the last element
    vector<float>::reverse_iterator rit = X.rbegin();

    // repeat 3 times
    for(size_t i = 0; i < 3; i++)
    {
        rit++;
        X.erase(rit.base());
    }

    // display all elements in vector X
    for(float &e: X)
        cout << e << '\n';

    return 0;
}

There are few things to mention:

  • reverse_iterator rit starts at the last element of the vector X. This position is called rbegin.
  • erase requires classic iterator to work with. We get that from rit by calling base. But that new iterator will point to the next element from rit in forward direction.
  • That's why we advance the rit before calling base and erase

Also if you want to know more about reverse_iterator, I suggest visiting this answer.

sanitizedUser
  • 1,723
  • 3
  • 18
  • 33
3

First, X.end() doesn't return an iterator to the last element of the vector, it rather returns an iterator to the element past the last element of the vector, which is an element the vector doesn't actually own, that's why when you try to erase it with X.erase(d) the program crashes.

Instead, provided that the vector contains at least 3 elements, you can do the following:

X.erase( X.end() - 3, X.end() );

Which instead goes to the third last element, and erases every element after that until it gets to X.end().

EDIT: Just to clarify, X.end() is a LegacyRandomAccessIterator which is specified to have a valid - operation which returns another LegacyRandomAccessIterator.

Nikko77
  • 31
  • 3
2

This statement

    if (i == 1) X.erase(d);

has undefined behavior.

And this statement tries to remove only the element before the last element

    else X.erase(d - i);

because you have a loop with only two iterations

for (size_t i = 1; i < 3; i++) {

You need something like the following.

#include <iostream>
#include <vector>
#include <iterator>
#include <algorithm>

int main() 
{
    std::vector<float> v = { 1, 2, 3, 4, 5 };

    auto n = std::min<decltype( v.size() )>( v.size(), 3 ); 
    if ( n ) v.erase( std::prev( std::end( v ), n ), std::end( v ) );

    for ( const auto &item : v ) std::cout << item << ' ';
    std::cout << '\n';

    return 0;
}

The program output is

1 2 
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
2

The definition of end() from cppreference is:

Returns an iterator referring to the past-the-end element in the vector container.

and slightly below:

It does not point to any element, and thus shall not be dereferenced.

In other words, the vector has no element that end() points to. By dereferencing that non-element thru the erase() method, you are possibly altering memory that does not belong to the vector. Hence ugly things can happen from there on.

It is the usual C++ convention to describe intervals as [low, high), with the “low” value included in the interval, and the “high” value excluded from the interval.

jpmarinier
  • 4,427
  • 1
  • 10
  • 23
2

A comment (now deleted) in the question stated that "there's no - operator for an iterator." However, the following code compiles and works in both MSVC and clang-cl, with the standard set to either C++17 or C++14:

#include <iostream>
#include <vector>

int main()
{
    std::vector<float> X{ 1.1f, 2.2f, 3.3f, 4.4f, 5.5f, 6.6f };
    for (auto f : X) std::cout << f << ' '; std::cout << std::endl;
    std::vector<float>::iterator d = X.end();
    X.erase(d - 3, d);  // This strongly suggest that there IS a "-" operator for a vector iterator!
    for (auto f : X) std::cout << f << ' '; std::cout << std::endl;
    return 0;
}

The definition provided for the operator- is as follows (in the <vector> header):

    _NODISCARD _Vector_iterator operator-(const difference_type _Off) const {
        _Vector_iterator _Tmp = *this;
        return _Tmp -= _Off;
    }

However, I'm certainly no C++ language-lawyer, and it is possible that this is one of those 'dangerous' Microsoft extensions. I would be very interested to know if this works on other platforms/compilers.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • 2
    I think it's valid, since a vector's iterators are random access, and `-` is defined for those types of iterators. – PaulMcKenzie Apr 15 '20 at 20:58
  • @PaulMcKenzie Indeed - the clang static analyser (which can be pretty strict with standards) gave no warning about it. – Adrian Mole Apr 15 '20 at 20:59
  • 1
    Even if there were no `operator-` defined for the iterators, you could just use [`std::advance()`](https://en.cppreference.com/w/cpp/iterator/advance) or [`std::prev()`](https://en.cppreference.com/w/cpp/iterator/prev) instead. – Remy Lebeau Apr 15 '20 at 21:02