c++64-bit

Understanding the Sub-expression overflow reasoning

I am trying to understand the reasoning behind this particular suggestion in Visual Studio 2022, as it doesn't seem to make sense to me. Here's the simple code:

static constexpr uint32_t MAX_ELEMENTS = 100000;
const std::vector<int> A{ some random ints };


std::vector<bool> found(MAX_ELEMENTS);
for (int value : A)
{
    if (value > 0 && value <= MAX_ELEMENTS)
        found[value - 1] = true;             // Problem it complains about is the value - 1.
}

It suggests that a "sub-expression may overflow before being added to a wider type". Now obviously the condition-if prevents this from ever happening, but what's the reasoning here?

Now if this was a Spectre thing, I could understand that the compiler would add a memory fence to stop any speculative execution of the statement after the if, but I don't think that's the answer here.

My only thinking is that it has to do with the subscript operator on the vector; that its index is of a type larger than int and is implicitly cast to size_t?

Just seems a little unintuitive that the below is an issue:

found[value - 1]

and its perfectly fine to do this,

int a = value - 1;
found[a];

when the end result is the same. What am I missing here in terms of understanding?

Solution

  • In this case, it is a false positive, as you suspected. This is a rule that sometimes gets used in stricter code bases. (This particular warning is an error in MISRA, for example.)

    A lot of warnings are like this... the compiler writers are trying to detect a situation where the behavior of the program is unexpected or unintentional, but the warnings are not always correct. For example,

    uint64_t x = 0xffffffffu << 16;

    On a system with 32-bit int, the standard is very clear what the value of this is supposed to be... it's supposed to be:

    uint64_t x = 0xffff0000u;

    However, it does look like someone meant to write this instead:

    uint64_t x = (uint64_t)0xffff0000 << 16;

    That's what the warning is trying to catch. That's the reasoning behind this rule. In your case, the compiler is doing a bad job of it.

    You can see a more detailed justification for the warning in the compiler documentation: Warning C26451.

    Spectre has nothing to do with it.

    It's perfectly fine...

    To do this:

    int a = value - 1;
found[a];

    In this case the "intent" is more obvious, because the programmer has explicitly written out "int" as the type.

    Solutions

    This is a code style issue, so there are a few different solutions and you pick whatever one you feel comfortable with.