Do not use array subscript when the index is not an integer constant expression; use gsl::at() instead

Question

I was trying to create some example code in Microsoft Visual Studio which looks like that

int main()
{
    const size_t size = 10;
    int arr[size];

    for (size_t i = 0; i < size; ++i)
        arr[i] = i;

    return 0;
}

Now JetBrains ResharperC++ emits the following warning in line arr[i] = i;

Do not use array subscript when the index is not an integer constant expression; use gsl::at() instead

I fail to understand what I this means and how to resolve this warning.

As this is a scheme I was using fairly often, I am a little concerned about the warning.

Could anyone advice or point me in the right direction?

EDIT: Changing the loop to:

for (size_t i = 0; i < size; ++i)
    arr[i] = 0;

still produces the warning.

I think it complains that the index (i in your case) is not an int. depending on compiler and architecture, not using an int may have strange effects. so always better use an int, not a size_t, when indexing an array — loonytune, Aug 17 '18 at 12:39
@loonytune Changing all size_t to int does not fix the problem. — , Aug 17 '18 at 12:40
On another side note, u are assigning an size_t to an int[], which will go terribly wrong on every 64 bit architecture (because there sizeof(size_t) > sizeof(int)). — loonytune, Aug 17 '18 at 12:40
just for fun, can u try replacing the arr[i] = ... with *(arr + i) = ...? — loonytune, Aug 17 '18 at 12:42
See https://stackoverflow.com/questions/1951519/when-should-i-use-stdsize-t, you should use an unsigned int if you want to be on the safe side — Korni, Aug 17 '18 at 12:43
well, *(arr + i) is the same doing pointer arithmetics. maybe jetbrains complains that u are using array semantics with a variable index, although the array is fixed in size in your case. and by using pointer magic it doesnt detect that anymore. another experiment, try to replace the the declaration of int arr[size] with int[] arr = (int[]) malloc(sizeof(int) * size); — loonytune, Aug 17 '18 at 12:45
This is the consequence of two C++ language design mistakes: 1) using exceptions to report logical bugs; 2) use unsigned integer for arrays size and subscripts. Just don't care two much about it, it will be fixed. — Oliv, Aug 17 '18 at 12:47
@P i I suppose they have used the standard semantic. `an_array.at(index)` is equivalent to `if (index>=an_array.size()) throw a_logical_error{}; return an_array[index];` — Oliv, Aug 17 '18 at 13:11

score 3 · Answer 1 · answered Aug 17 '18 at 12:51

3

Its a warning that arr[i] doesn't do any bounds checking and that you should use gsl::at(arr, i) from https://github.com/Microsoft/GSL instead as it does bounds checking and is safer.

answered Aug 17 '18 at 12:51

Alan Birtles

32,622
4
31
60

score 3 · Accepted Answer · edited Jun 03 '20 at 18:05

3

In general

for (size_t i = 0; i < size; ++i)
    arr[i] = something;

is dangerous. You can't tell if arr[i] is going to go out of bounds of the array. This is why the C++ Core Guidelines suggest you use gsl::at() as it will do bounds checking to make sure you do not go out of bounds on the array.

This isn't the only solution though. If you just need to iterator over the range you can use a range based for loop like

for (const auto& e : arr)
    //e is each element of the array and is not mutable here

or

for (auto& e : arr)
    //e is each element of the array and is mutable here

And for a case like yours where you need to fill the array you can use std::iota like

std::iota(std::begin(arr), std::end(arr), 0);

and all of these are guaranteed to not go out of bounds.

edited Jun 03 '20 at 18:05

paleonix

2,293
1
13
29

answered Aug 17 '18 at 13:15

NathanOliver

171,901
28
288
402

Sometimes you just have to mute the linter, because it clearly cannot tell the difference. It will still be nagging at you when calling some code from a C library where for example a variable is declared as double p[4][2]; how the hell does C++ want you to iterate over that? – Mehdi Dec 14 '22 at 11:02
@Mehdi If you need read write access `for (auto& row : p) for (auto& elem : row) { do_something_with_each_element; }` – NathanOliver Dec 14 '22 at 13:07

score 1 · Answer 3 · answered Aug 17 '18 at 12:51

I think the reason for the warning is that operator[] does not boundary check, while gsl::at() could.

Since size is known in compile time, if the index was constexpr, you could get a warning, but if the value is determined in runtime you can't. In my opinion pretty unnecessary.

Azeem · Answer 4 · 2018-08-17T13:03:45.273

Accessing an array element by index, without any bounds checking, is not considered a good practice. It is a direct memory access which is not safe and may cause segmentation fault.

For an STL container like std::vector, there's this at() member function which performs the bounds-checking and is the recommended way to access elements.

You may ignore this warning for this trivial example. But, for non-trivial code, use std::vector. But, for C-style arrays, you may download and use gsl::at() and explore its other facilities as well.

References:
C++ Core Guidelines
GSL (Guideline Support Library)

Ron · Answer 5 · 2018-08-17T14:17:07.000

1

It is not a (compiler) warning. It's one of the C++ Core Guidelines incorporated into a 3rd party IDE / analysis tool.

edited Aug 17 '18 at 14:17

answered Aug 17 '18 at 13:02

Ron

14,674
4
34
47

score 0 · Answer 6 · answered Aug 17 '18 at 13:34

It is a warning as operator[] doesn't check bound, contrary to at. Whereas in your case, the code is correct, "small" change might break your loop (int arr[2 * size];)

There are several good alternatives using iterators (explicitly or implicitly) in your case:

const size_t size = 10;
int arr[size];

std::iota(std::begin(arr), std::end(arr), 0);

or

int i = 0;
for (auto& e : arr) {
    e = i;
    ++i;
}

score 0 · Answer 7 · answered Nov 10 '20 at 09:34

0

int array[] = {0, 1, 2};
for (auto element : array){
   if(element == 1){
   //.....
   }
}

Try above code.

answered Nov 10 '20 at 09:34

Hieu Le

79
2
8

score 0 · Answer 8 · answered Sep 27 '21 at 00:24

This seems to need a more updated answer.

Rather than use a handwritten loop, it is suggested to use standard alogos. In this case iota does exactly what is needed: an auto incrementing assignment.

We also, for a static array should be using std::array instead of 'C' style arrays. But this would work just as well with vector, since iota takes runtime args.

template<size_t SZ>
auto<int, SZ> CountInit()
{
    std::array<int, SZ> arr; // Note we only use SZ once

    std::iota(arr.begin(), arr.end(), 0);
    return arr;
}

This can be done in a more compile time way using integer_sequence and some helpers:

// Note taken from : https://jgreitemann.github.io/2018/09/15/variadic-expansion-in-aggregate-initialization/
template <typename Container, int... I>
Container iota_impl(std::integer_sequence<int, I...>) {
    return {I...};
} 

template <typename T, std::size_t N>
auto iota_array() {
    using Sequence = std::make_integer_sequence<int, N>;
    return iota_impl<std::array<T, N>>(Sequence{});
}



auto foo2()
{
    return iota_array<int, 10>();
}

int goo2()
{
    auto x=foo2();
    return std::accumulate(x.begin(), x.end(), 0);
}

According to https://godbolt.org/z/59hqqdEYj These are equivalent (as per c++20 iota is constexpr)

Do not use array subscript when the index is not an integer constant expression; use gsl::at() instead

8 Answers8