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

8.9k Views Asked by At

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;

enter image description here 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.

9

There are 9 best solutions below

2
NathanOliver On BEST ANSWER

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.

0
Kostas On

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.

0
Alan Birtles On

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.

0
Azeem On

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)

0
Ron On

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

0
Jarod42 On

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;
}
0
Hieu Le On

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

Try above code.

0
Glenn Teitelbaum On

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)

0
gohar94 On

This happens because accessing a raw array using [] does not perform bounds check. Using std::array could be a good alternative; you can access elements at a particular index using the .at() method.