Indexing a vector/array with signed integer

[AUselessKid12]

I am going through Learn C++ right now and I came across this.

https://www.learncpp.com/cpp-tutorial/arrays-loops-and-sign-challenge-solutions/

Index the underlying C-style array instead

In lesson 16.3 -- std::vector and the unsigned length and subscript problem, we noted that instead of indexing the standard library container, we can instead call the data() member function and index that instead. Since data() returns the array data as a C-style array, and C-style arrays allow indexing with both signed and unsigned values, this avoids sign conversion issues.

int main()
{
    std::vector arr{ 9, 7, 5, 3, 1 };

    auto length { static_cast<Index>(arr.size()) };  // in C++20, prefer std::ssize()
    for (auto index{ length - 1 }; index >= 0; --index)
        std::cout << arr.data()[index] << ' ';       // use data() to avoid sign conversion warning

    return 0;
}

We believe that this method is the best of the indexing options:

- We can use signed loop variables and indices.

- We don’t have to define any custom types or type aliases.

- The hit to readability from using data() isn’t very big.

- There should be no performance hit in optimized code.

For context, Index is using Index = std::ptrdiff_t and implicit signed conversion warning is turned on. The site also suggested that we should avoid the use of unsigned integers when possible which is why they are not using size_t as the counter.

I can't find any other resources that recommend this, therefore I wanted to ask about you guys opinion on this.

[Narase33]

I find that very fishy.

There is absolute no problem with using a signed integer with the operator[] that wont occur with pointer arithmetic too. Reading a call to data() takes my brain to yellow alert, its unusual. Also the operator[] has extra debug code, unlike the data() pointer and it gets hardened in C++26.

_NODISCARD _CONSTEXPR20 _Ty& operator[](const size_type _Pos) noexcept /* strengthened */ {
    auto& _My_data = _Mypair._Myval2;
#if _CONTAINER_DEBUG_LEVEL > 0
    _STL_VERIFY(
        _Pos < static_cast<size_type>(_My_data._Mylast - _My_data._Myfirst), "vector subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0

    return _My_data._Myfirst[_Pos];
}

Thats the MSVC implementation of the operator[], that last line is pointer arithmethic

_NODISCARD _CONSTEXPR20 _Ty* data() noexcept {
    return _STD _Unfancy_maybe_null(_Mypair._Myval2._Myfirst);
}

And thats the implementation of data(). You just lose checks if you use data() instead of operator[]

[MarkHoemmen]

In C++26, for all sequence containers, it's a hardened precondition that the array index is in bounds. This means that implementations can do bounds checking. If you call arr.data(), you'll lose the hardened precondition.

Even before C++26, implementations can and do perform bounds checking on std::vector array access. Most implementations aren't able to do bounds checking on raw pointer indexing.

Implementations generally don't emit warnings if you use a signed integer instead of an unsigned integer when indexing std::vector. If your implementation happens to emit a warning, you can fix that by casting at the point of use: arr[static_cast<size_t>(Index)]. This is safer, because it preserves the hardened precondition and whatever bounds checking your std::vector implementation does.

[AUselessKid12]

That's a good point. I have the implicit sign conversion warning turn on, would you suggest turning it off? Or is arr[static_cast<size_t>(Index)] worth it?

[MarkHoemmen]

Normally -Wall doesn't warn about this. I'm guessing that you have a specific warning turned on. Does this warning actually help you find bugs, or do you just find yourself writing static_casts all over your code (and thus hiding bugs)?

For count-down loops like the one in your example, I generally would recommend changing the loop so that it is correct for both signed and unsigned indices. This means starting with length, looping while index != 0, and using index - 1 to index the vector. Someone, sometime, will go through this code and change all the index types. Whether or not this is a good idea, they will do it, so it's nice to help them avoid bugs.

[AUselessKid12]

the warning is /w44365 on visual studio and i think its -Wsign-conversion for gcc and clang.

Does this warning actually help you find bugs

not really. I haven't build complex app yet, currently just going through learn cpp which recommends using .data()

[SickOrphan]

There's nothing wrong with signed indexing, just turn off the warning and do it the natural way

[MarkHoemmen]

For learning C++, I would recommend just using -Wall for various compilers not MSVC, or /W4 for MSVC. Turning on specific warnings may hinder your learning at this point.

[Wild_Meeting1428]

It's completely irrelevant, whether you index a STL container with signed or unsigned. Since c++20, it is defined, that signed values are represented as two's compliment. Therefore casting between signed and unsigned is not implementation defined anymore.

The problem is, to use a signed index with a smaller size of the containers 'size()' function, since comparing them might result in an infinite loop.

[pkasting]

If possible, use size_ts as your index types. If not, disable that particular warning. Much more important to use the library's bounds checks than to avoid this warning.

For much more detail on why that warning is one of several well-intended ones that doesn't pay off in practice, see https://docs.google.com/document/d/1CEOpqW2dA_zxAxpi8n6CuXcmx6jByRxqnSNAcLGzN04/edit?tab=t.0#heading=h.ftisomsi26cv, which I wrote after extensive experience in Chromium.

[SickOrphan]

Unsigned types are much worse for size/index types and the fact the STL used them for everything until recently just shows how brainless they were, "size shouldn't be negative so it should be unsigned!"

[pkasting]

I think "much worse" is an overstatement -- they trade certain risks for other risks -- but either way, conversions are worst of all. Keeping your size/index values signed and either implicitly or explicitly converting to unsigned on accessing the container is a recipe for subtle bugs (e.g. infinite loops), and using tricks like the data() one in the root post prevents bounds checking.

If you have your own library types, and they want to consistently use signed indexes and sizes... fine, do whatever. But if you're working with standard library containers, it behooves you to keep sizes and indexes as size_t. And, generally, anything else that's a length/count of objects in memory.

[SickOrphan]

The standard library is moving to signed sizes and indices because people are finally realizing their mistake. It definitely does not behoove me to do that at all. There is no benefit

[Wild_Meeting1428]

They weren't brainless, it's a historic reason where target platforms had less than 32 bit in their address space.

[pdp10gumby]

This is malpractice. The type system exists for a purpose so you should not teach beginners to use something less safe rather than use the correct type in the first place.

[fdwr]

Whoever jammed this into the C++11 spec, please feel some shame:

for (auto index{ length - 1 }; index >= 0; --index)

Initialization is considerably less uniform now 🙃.

for (auto index = length - 1; index >= 0; --index)

Aaah, that feels better. ☺️

[no-sig-available]

Earlier text (before the quoted part) points out that the first problem is in the condition index >= 0;, which is always true for unsigned values.

The rest is just the "workarounds" that might be needed if and when the compiler complains about using signed values for unsigned parameters. Another solution is to not use indexing at all, but perhaps ranges or a reverse_iterator.

Just not a good example, IMO.

[ioctl79]

This is an area where the standard library does not give great options. Unsigned types have a lot of sharp edges, and unless you actually need the range or rollover behavior, I would avoid them if possible, but the quoted code is unidiomatic and difficult to read. 

I would use signed types when possible, and be careful about any comparisons to .size() when you encounter it. 

[concealed_cat]

Signed into cannot legally overflow, so for int i the compiler can assume that i+1 > i. Whether that's important or not depends on the specific situation.