CODEWITHSUNDEEP
c++vectorauto
Ionut Alexandru
Ionut Alexandru

Reputation: 806

What is wrong with auto?

vector<int> function(vector<int> &arr)
{
    for(auto i = arr.size() - 1; i >= 0; i--)
        std::cout << arr[i] << " ";

    return arr;
}

int main()
{
    vector<int> arr{1,2,3,4};
    function(arr);
}

Why does the above program cycle? if I change auto with int everything is ok

Upvotes: 3

Views: 415

Answers (3)

Blaze
Blaze

Reputation: 16876

arr.size() is an unsigned data type, usually size_t. With i being unsigned, i >= 0 is always true. Subtracting 1 from an unsigned variable that is 0 results in the biggest amount that the type can hold. As a result, it will cycle forever.

What then happens is uncertain, since your array index will turn into a gigantic value, and arr[i] will have undefined behavior for values >= arr.size(). If you have an int instead of auto, it works because the i-- will cause it to eventually be -1 and then i >= 0 will be false, exiting the loop.

An explanation of this rollover behavior can be found here:

Unsigned integer arithmetic is always performed modulo 2n where n is the number of bits in that particular integer. E.g. for unsigned int, adding one to UINT_MAX gives ​0​, and subtracting one from ​0​ gives UINT_MAX.

So, for size_t, subtracting 1 from 0 results in SIZE_MAX, which commonly has a value of 18446744073709551615.

Upvotes: 9

sklott
sklott

Reputation: 2849

What is you problem was already answered by Blaze and rafix07, but I wanted to add that in modern C++ its better to use iterators whenever possible. This has few advantages including code portability, better performance and more readable code.

Your code can look something like this:

std::vector<int> function(std::vector<int> &arr)
{
    for(auto it = arr.rbegin(); i != arr.rend(); ++i)
        std::cout << *it << " ";

    return arr;
}

or like this

std::vector<int> function(std::vector<int> &arr)
{
    std::for_each(arr.rbegin(), arr.rend(), [](int val) {
        std::cout << val << " ";
    });

    return arr;
}

or even like this

std::vector<int> function(std::vector<int> &arr)
{
    std::copy(arr.rbegin(), arr.rend(), std::ostream_iterator<int>(std::cout, " "));

    return arr;
}

Upvotes: 3

Michael Chourdakis
Michael Chourdakis

Reputation: 11158

When you have a loop that goes the other way ( from max to 0 ) then you usually have this problem:

  • Either the max is size_t so i >= 0 is always true
  • Or you change i to int so you have a comparison of a signed with an unsigned which would result in a compiler warning, or a comparison of an int to a larger size_t in x64.

Redesign the loop:

  • Use a new type for i which would be long in x86 and long long in x64, now i >= 0 is good when your objects are not above 2^63 in x64 (most likely).
  • when i == 0, break inside the loop.
  • Change to the normal i = 0 and i < obj.size() method.

Upvotes: 2

Related Questions