4

This perfectly good program fails in debug mode in Visual Studio 2013:

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

using namespace std;

void main()
{
  vector<int> v = {3, 1, 4, 1, 5, 9, 2, 6, 5, 3};

  for (auto iFrom = v.cbegin(), iTo = iFrom+5; iFrom != v.cend(); iFrom = iTo, iTo += 5)
    cout << *max_element(iFrom, iTo) << '\n';
}

with vector iterator + offset out of range assertion failure. It fails because iTo > v.cend(), which is harmless here. What is the point of debugger testing the value of iterator, which is not being dereferenced?

BTW, I know that I can rewrite the loop above as:

for (auto i = v.cbegin(); i != v.cend(); i += 5)
  cout << *max_element(i, i+5) << '\n';

but I was trying to make a simple example out of a piece of more complex real-life code, where calculating the new iterator value is computationally expensive.

I also realize that one can change _ITERATOR_DEBUG_LEVEL value to affect this behavior, but it creates problems with binary versions of some libraries, which are built with default debug settings.

Paul Jurczak
  • 7,008
  • 3
  • 47
  • 72

3 Answers3

9

It's not harmless... it's undefined behaviour to attempt to move the iterator past end(), which you do on your third and final iteration. The fact that immediately after doing so with iTo += 5 you terminate your loop as iFrom != v.cend() without dereferencing the iterator is not relevant.

If efficiency is really necessary and you're prepared the bet the bank on the number of elements being a multiple of 5:

for (auto iFrom = v.cbegin(), iTo = iFrom; iFrom != v.cend(); iFrom = iTo)
{
    iTo += 5;
    cout << *max_element(iFrom, iTo) << '\n';
}
Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • BTW, I'm prepared to bet the whole banking system on _the number of elements being a multiple of 5_ - the number is 10, which obviously is a multiple of 5 ;-) – Paul Jurczak Aug 05 '14 at 03:46
  • @PaulJurczak: I didn't mean in your hard-coded sample vector, but in whatever real-world code you've simplified the posted code down from - where it may or may not be hard-coded. ;-P Anyway, even though I work in the industry I'd happily bet the whole banking system on there being a rainbow and 3 shooting stars visible from Tokyo SkyTree at exactly 2am tomorrow, so I'm not sure what to make of your bet...! – Tony Delroy Aug 05 '14 at 03:51
  • I'm novice.I'm trying to add a element in 'v',when run it crash. Can you tell ? – Chuanhang.gu Apr 28 '17 at 13:01
  • @Chuanhang.gu: you haven't provided enough information to tell. You should read about how to post your own question with minimal sample code to reproduce the problem. – Tony Delroy Apr 28 '17 at 18:30
  • @TonyD What I said is base on above example, the only modify is add one element in 'v',making code like bellow: void main() { vector v = { 3, 1, 4, 1, 5, 9, 2, 6, 5, 3,-1 }; for (auto iFrom = v.cbegin(), iTo = iFrom; iFrom != v.cend(); iFrom = iTo) { iTo += 5; cout << *max_element(iFrom, iTo) << '\n'; } } .Recently I meet a strange question. If you can,please help answer. http://stackoverflow.com/questions/43681922/vectorreserve-cause-vector-iterator-offset-out-of-range – Chuanhang.gu Apr 28 '17 at 22:53
  • @Chuanhang.gu Your code above has undefined behaviour, as your vector has 11 elements which is not a multiple of 5. As I say in my answer, you've "bet the bank" and lost the bet. If you remove the "-1" it will start working. – Tony Delroy Apr 29 '17 at 00:58
1

Not completely sure what the question is here, but the "point" is that VS is trying to aid in removing dangerous code. Yes, in principle a pointer could point anywhere if you didn't dereference it, but iterators are a higher level abstraction and have the ability to detect whether dereference would be valid and thus raise runtime errors. VS has done this with vector<T>::iterator at least since VS 2007.

sfjac
  • 7,119
  • 5
  • 45
  • 69
  • 2
    My point is that _ability to detect whether dereference would be valid_ is very nice and useful, but why not do it only when the iterator **is** actually dereferenced, not before, when it doesn't matter. – Paul Jurczak Aug 05 '14 at 03:03
0

max_element() takes two iterators and goes over the entire range between them. If you think about it, the range isn't really valid if either of the iterators are invalid. Both SGI and Microsoft mention this as a prerequisite in their respective documentation:

SGI:

Preconditions

  • [first, last) is a valid range.

MSDN:

Remarks

  • The range referenced must be valid; all pointers must be dereferenceable and within each sequence the last position is reachable from the first by incrementation.

If I were in your situation, I'd just add a check to see if iTo was still valid as the first step in my loop. If it's not, set it to the last position in v.

MrEricSir
  • 8,044
  • 4
  • 30
  • 35
  • That is true, but I never call `max_element()` with invalid range. The problem is in incrementing an iterator past `end()`. – Paul Jurczak Aug 05 '14 at 03:00