c++ - assuming signed overflow does not occur in if statement

Question

Why is this warning appearing? It's not really an assumption if I check the bounds. And how to fix?

If num_actions_to_skip is set to 1, instead of 2, the error goes away.

Thanks

error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow]
cc1plus: all warnings being treated as errors

On if (loc >= 0 && loc < action_list.count()) {

const QList<QAction *> &action_list = tool_menu->actions();
static const int num_actions_to_skip = 2;
const int loc = action_list.count() - num_actions_to_skip;
if (loc >= 0 && loc < action_list.count()) {
    tool_menu->insertAction(action_list.at(loc),
                            action);
}

It started with

Q_ASSERT_X(i >= 0 && i < p.size()

at qlist.h:454, which performs the same check, and throws this error as well, with just

tool_menu->insertAction(action_list.at(action_list.count() - 2),
                                action);

Answer 1

You just need to rethink your logic.

static const int num_actions_to_skip = 2;
const int loc = action_list.count() - num_actions_to_skip;
if (loc >= 0 && loc < action_list.count()) {
    // ...
}

Apparently action_list.count() is a constant value (at least it won't change as this code is executed), and the compiler is able to figure that out.

Let's simplify this a bit, replacing num_actions_to_skip by 2, reducing action_list.count() to count. We can then re-express loc as count - 2.

Your if condition becomes:

if (count - 2 >= 0 && count - 2 < count)

which is equivalent (assuming, as the compiler warning said, that no overflow occurs) to:

if (count >= 2 && -2 < 0)

The second half of that, -2 > 0 is obviously true, so you can safely drop it, which leaves us with

if (count >= 2)

Re-substituting the original terms, this gives us:

static const int num_actions_to_skip = 2;
// const int loc = action_list.count() - num_actions_to_skip;
if (action_list.count() >= num_actions_to_skip) {
    // ...
}

The compiler warned you that it was performing an optimization that might be invalid if there's an integer overflow (it's permitted to assume that there is no overflow because if there is the behavior is undefined). It was kind enough to warn you about this -- which is lucky for you, because it pointed to the fact that your code is doing something it doesn't need to.

I don't know whether you need to keep the declaration of loc; it depends on whether you use it later. But if you simplify the code in the way I've suggested it should work the same way and be easier to read and understand.

If you get a warning message from the compiler, your goal should not just be to make the message go away; it should be to drill down and figure out just what the compiler is warning you about, and beyond that why your code causes that problem.

You know the context of this code better than I do. If you look at the revised version, you may well find that it expresses the intent more clearly.