What is wrong with auto?

Question

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

Move the decrement into the condition: `(auto i = arr.size(); (i--) > 0;)`. @M.M Better? — Bob__, Feb 25 '20 at 07:14
@Bob__ https://stackoverflow.com/questions/1642028/what-is-the-operator-in-c — M.M, Feb 25 '20 at 07:15
What's the point of returning the unchanged `std::vector` that was passed into the function? Why not just return `void`? — Jesper Juhl, Feb 25 '20 at 07:49
IMHO, the best solution would be to use std::vector's const reverse iterator [for (auto i = arr.crend(); i != arr.crbegin(); i--)]. Also, as you don't change arr, its best to pass it as const ref. Otherwise, I agree with Jesper Juhl that you should return void, and with Blaze's answer. — Uri Raz, Feb 25 '20 at 08:05
The One way of using a range based for loop to iterate in reverse is to use `boost::adaptors::reverse`. [Demo](https://gcc.godbolt.org/z/8GsZEg) — Aykhan Hagverdili, Feb 25 '20 at 09:33
The `return arr` is very distracting from the issue, can we please remove it from your example and make the function return void? — Wyck, Feb 27 '20 at 15:43

score 9 · Answer 1 · edited Feb 25 '20 at 09:01

9

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 2ⁿ 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.

edited Feb 25 '20 at 09:01

Remy Lebeau

555,201
31
458
770

answered Feb 25 '20 at 07:12

Blaze

16,736
2
25
44

1

Although if it is `int` the loop will fail if the vector has more than `INT_MAX` objects – M.M Feb 25 '20 at 07:14
@M.M that's right. The best solution would probably be to change the loop condition so it doesn't have to rely on `i` becoming `-1`. Of course, for OP's example, `INT_MAX` is plenty. – Blaze Feb 25 '20 at 07:16
Thanks i understand what is the problem. – Ionut Alexandru Feb 25 '20 at 07:31

score 3 · Answer 2 · answered Feb 25 '20 at 07:23

3

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;
}

answered Feb 25 '20 at 07:23

sklott

2,634
6
17

2

Very often you need the index. – Michael Chourdakis Feb 25 '20 at 07:24
@walnut You cannot traverse the reverse direction with range-for. – VLL Feb 25 '20 at 07:28
@vll Ah right, sorry. Still the wording "*in modern C++ its better to use iterators whenever possible*" seems strange to me when iterators have always been used in C++ and modern approaches go towards ranges instead. – walnut Feb 25 '20 at 07:30
@MichaelChourdakis, yes, it happens sometimes, though not that often in my practice, but anyway I really missed Python `enumerate()` at times. Fortunately we will have ranges soon... – sklott Feb 25 '20 at 07:32
@walnut, of course ranges are better, but they still not accessible to many projects due to requirements to use older C++ standard yet... – sklott Feb 25 '20 at 07:33
1

@vll you can, with a little help https://gcc.godbolt.org/z/6HWBHF – Aykhan Hagverdili Feb 25 '20 at 07:38
@MichaelChourdakis you can get the index by subtracting addresses – Aykhan Hagverdili Feb 25 '20 at 07:48
If index is needed, [std::distance](https://en.cppreference.com/w/cpp/iterator/distance) could also be useful. – Phil Brubaker Feb 25 '20 at 09:05

score 2 · Answer 3 · answered Feb 25 '20 at 07:22

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.

What is wrong with auto?

3 Answers3