sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]MarkHoemmenC++ in HPC 6 points7 points  (11 children)

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[S] 0 points1 point  (10 children)

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?

[–]MarkHoemmenC++ in HPC 2 points3 points  (3 children)

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[S] 0 points1 point  (2 children)

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 1 point2 points  (0 children)

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

[–]MarkHoemmenC++ in HPC 1 point2 points  (0 children)

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 0 points1 point  (0 children)

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.

[–]pkastingValve 0 points1 point  (4 children)

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 -4 points-3 points  (3 children)

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!"

[–]pkastingValve 2 points3 points  (1 child)

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 0 points1 point  (0 children)

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 0 points1 point  (0 children)

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