Using operator[] on empty std::vector

Question

I was advised a while ago that is was common place to use std::vector as exception safe dynamic array in c++ rather than allocating raw arrays... for example

{
    std::vector<char> scoped_array (size);
    char* pointer = &scoped_array[0];

    //do work

} // exception safe deallocation

I have used this convention multiple times with no problems, however I have recently ported some code to Win32 VisualStudio2010 (previously it was only on MacOS/Linux) and my unit tests are breaking (stdlib throws an assert) when the vector size happens to be zero.

I understand that writing to an such an array would be a problem, but this assumption breaks this solution as a replacement to raw pointers. Consider the following functions with n = 0

void foo (int n) {
   char* raw_array = new char[n];
   char* pointer = raw_array;
   file.read ( pointer , n );
   for (int i = 0; i < n; ++i) {
      //do something
   }
   delete[] raw_array;
}

While arguably redundant, the above code is perfectly legal (I believe), while the below code will throw an assertion on VisualStudio2010

void foo (int n) {
   std::vector<char> scoped_array (n);
   char* pointer = &scoped_array[0];
   file.read ( pointer , n );
   for (int i = 0; i < n; ++i) {
  //do something
   }
}

Was I using undefined behavior all along? I was under the impression operator[] did no error checking, and this was a valid use of std::vector<>. Has anyone else encountered this problem?

--edit: Thanks for all the useful responses, in reply to people saying it is undefined behavior. Is there a way to replace the raw array allocation above which will work with n = 0?

While saying that checking for n=0 as an exceptional case will solve the problem (it will). There are many patters where no special case is needed (such as the raw pointer example above) so maybe using something other than std::vector<> would be needed?

Answer 1

MVS does range checking in operator[] even in release builds. I don't know if it's standard compliant. (I actually found debug code in their implementation which made their implementation break correct code). There is a switch to disable it though.

Answer 2

With n==0 it is undefined for both array of char and vector.

For new char[0], according to 3.7.3.1/2

The effect of dereferencing a pointer returned as a request for zero size is undefined.

EDIT: It is of course, defined until dereferencing actually takes place, if file here is std::istream, then file.read() won't attempt to dereference it since n is zero.

For the vector, a[n] is equivalent to *(a.begin() + n), according to 23.1.1/12, If the container is empty, then begin() == end(), according to 23.1/7, which means scoped_array[0] attempted to dereference a past-the-end iterator.

Answer 3

If you want to get cleaner behaviour in this scenario you could replace use of a[0] with use a.at(0), which will throw if the index is invalid.

A pragmatic solution would be to init vector with n+1 entries and constrain access to 0..n-1 (as this code already does).

void foo (int n) {
   std::vector<char> scoped_array (n+1);
   char* pointer = &scoped_array[0];
   file.read ( pointer , n );
   for (int i = 0; i < n; ++i) {
  //do something
   }
}

Answer 4

This brought an interesting question to my mind, which I promptly asked here. In your case, you can avoid using pointers the following way:

template<class InputIterator, class OutputIterator>
OutputIterator copy_n( InputIterator first, InputIterator last, OutputIterator result, std::size_t n)
{
    for ( std::size_t i = 0; i < n; i++ ) {
        if (first == last)
            break;
        else
            *result++ = *first++;
    }
    return result;
}

std::ifstream file("path_to_file");
std::vector<char> buffer(n);
copy_n(std::istream_iterator<char>(file), 
       std::istream_iterator<char>(),
       std::back_insert_iterator<vector<char> >(buffer),
       n);

This will copy the contents of the file to a buffer n chars at a time. When you iterate over the buffer, use:

for (std::vector<char>::iterator it = buffer.begin(); it != buffer.end(); it++)

instead of a counter.

Answer 5

As far as the C++ standard is concerned, operator[] isn't guaranteed not to check, it's just that (unlike at()) it's not guaranteed to check.

You'd expect that in a non-checking implementation, &scoped_array[scoped_array.size()] would result in a legal pointer either within or one-off-the-end of an array allocated by the vector. This isn't explicitly guaranteed, but for a given implementation you could verify by looking at its source. For an empty vector, there might not be an allocation at all (as an optimisation), and I don't see anything in the vector part of the standard which defines the result of scoped_array[0] other than table 68.

Going from table 68, you might say that the result of your expression is &*(a.begin() + 0), which illegally dereferences an off-the-end iterator. If your implementation's vector iterator is just a pointer then you probably get away with this - if not you might not, and obviously yours isn't.

I forget the results of the argument as to whether &*, on a pointer that must not be dereferenced, is a no-op, or not. IIRC it's not clear from the standard (some ambiguity somewhere), which provoked requests to fix the standard to make it explicitly legal. This suggests that it does in fact work on all or most known implementations.

Personally I wouldn't rely on this, and I wouldn't disable the checking. I'd rewrite your code:

char* pointer = (scoped_array.size() > 0) ? &scoped_array[0] : 0;

Or in this case just:

char* pointer = (n > 0) ? &scoped_array[0] : 0;

It just looks wrong to me to use index n of a vector without knowing that the size is at least n+1, regardless of whether it actually works in your implementation once you've disabled the checking.

Answer 6

operator [] returns a reference, and therefor invoking it on an empty vector must be undefined.

After all, which item should the reference refer to, when there are no items? operator [] would have to return a null-reference or a totally invalid reference. Both of which would result in undefined behavior.

So yes, you were using undefined behavior all along. Visual Studio's not-mandatory-but-still-comformant checks in operator [] just revealed that fact.

Answer 7

See LWG issue 464. This is a known issue. C++0x (which is partially implemented by MSVC 2010) solves it by adding a .data() member.

Answer 8

Could you use iterators instead of pointers?

{
    std::vector<char> scoped_array (size);
    std::vector<char>::iterator pointer = scoped_array.begin();

    //do work

} // exception safe deallocation