Expression 'i < 0' is always false
Asked Answered
P

4

25

For the following snippet:

size_t i = 0;
std::wstring s;
s = (i < 0)   ? L"ABC" : L"DEF";
s = (i != -1) ? L"ABC" : L"DEF";

PVS-Studio analysis logs warning for the first condition i < 0, as expected:

V547 Expression 'i < 0' is always false. Unsigned type value is never < 0. test_cpp_vs2017.cpp 19

Why PVS does not issue any warning about the second, also suspicious condition i != -1 reporting it as always true, for instance?

Pep answered 31/7, 2017 at 8:17 Comment(7)
@JeffUK No, it does not warn about the i != -1 in isolationPep
OK, viva64.com/en/w/V3022 This article seems to explain it well when it says "The analyzer doesn't warn about every condition that is always true or false; it only diagnoses those cases when a bug is highly probable."Raguelragweed
@JeffUK, condition i != -1 is not always true!Wit
Note that some compilers would warn about i != -1 as “comparing signed and unsigned”. Such warning is harmful, because somebody might silence it by changing it to i != unsigned(-1) or i != -1u, but that would break the code (i is size_t and that might be larger than unsigned). i != -1 is the most reasonable way in most cases.Wit
@JanHudec I disagree, it is not inherently false like i<0 but static analysis of this code shows that i is always 0 when this check occurs, therefore i != -1 is redundant at this point in the code. PVS-Studio simply chooses to only warn about the former not the latter.Raguelragweed
Strangely enough, while i < 0 is never true when i is a size_t, i <= -1 will always be true when i is a size_t.Roundfaced
@JeffUK, seems it fails to actually do the static analysis bit.Wit
M
36

Because that'd be a useless, invalid warning. size_t is an unsigned type, and due to the way integer conversions work (see [conv.integral]/2), -1 converted (implicitly here) to size_t is equal to SIZE_MAX.

Consider the fact that this is the actual definition of std::string::npos in libstdc++:

static const size_type  npos = static_cast<size_type>(-1);

If PVS-Studio warned about i != -1, would it also need to warn about i != std::string::npos?

On the other hand, an unsigned value can never be smaller than 0, due to it being unsigned, so i < 0 is likely not what the programmer wanted, and thus the warning is warranted.

Marksman answered 31/7, 2017 at 8:19 Comment(10)
The comparsion with i != std::string::npos does clear up my doubt indeed. That actually indicates the analyzer is clever enough treating the operators < and != in this case differentlyPep
I'd expect there to be a warning about the implicit conversion, which is potentially unintentionalFullmouthed
@BlueRaja-DannyPflughoeft, as I commented on the question a couple of minutes ago, the warning about the implicit conversion would actually be harmful, at least unless they would also warn about implicit size conversion. And in C++, that would not be practical as existing code usually does that all over the place.Wit
Does this mean that i>-1 is always false as well?Coulisse
@ratchetfreak yes.Herndon
@ratchetfreak I think it's always true, insteadJermainejerman
@ratchetfreak actually more convincing might be this.Herndon
A warning about some_unsigned_variable != -1 would not be useless! It would indeed be a nuisance in code which uses that as an idiom, but otherwise potentially very useful in code that doesn't, helping someone find a bug. However, code which uses negative constants in combination with unsigned types tends to occur in compiler and platform header files, so those would have to be cleaned not to trigger false positive warnings or use some #pragmas to disable it or whatever.Functional
Well, idiomatic/pedantic C++ would just use std::numeric_limits<decltype(whatever)>::max(), which clearly expresses intent, instead of the silly unsigned wraparound backdoor. But yeah, both work equally well for the compiler.Station
@Kaz: As a code reviewer, I would bristle at a direct comparison between an unsigned variable and -1. The code would have one of two unambiguous meanings depending upon whether the unsigned value is large enough to avoid promotion, but I would consider the code clearer if it made clear what it's actually asking for, e.g. comparing to (uint32_t)-1 if that's what's desired.Backstay
J
22

This is due to implicit integral conversions in both cases. A size_t must be an unsigned type of at least 16 bits and in your case it is of sufficient size cf. int that if one argument is size_t and the other an int, then the int argument is converted to size_t.

When evaluating i < 0, 0 is converted to an size_t type. Both operands are size_t so the expression is always false.

When evaluating i != -1, the -1 is converted to size_t too. This value will be std::numeric_limits<size_t>::max().

Reference: http://en.cppreference.com/w/cpp/language/implicit_conversion

Jezabel answered 31/7, 2017 at 8:21 Comment(0)
S
7

When a value is converted to unsigned, if that value is not representable by the unsigned type, then the value will be converted to a value (or rather, the value) that is representable, and is congruent to the original value modulo the number of representable values (which is the maximum representable value + 1 == 2n where n is the number of bits).

Therefore there is nothing to warn about, because there is some value for which the condition can be false (as long as we only analyze that expression in isolation. i is always 0, so the condition is always true, but to be able to prove that, we must take the entire execution of the program into account).

-1 is congruent with m - 1 modulo m, therefore -1 is always converted to the maximum representable value.

Singleminded answered 31/7, 2017 at 8:27 Comment(3)
> there is nothing to warn about Nonsense. The conversion of -1 to a really large value, due to the unsigned type on the opposite side of the operator, is potentially surprising. There is plenty to warn about in correct, standard-conforming constructs of all sorts. Also, the behavior is not fully specified here; the result is implementation-defined. If size_t is 16 bits wide, you get 65535, and so on; the nice and portable -1 constant has turned into a compiler-specific number.Functional
@Kaz: Worse, there is nothing in the Standard that would mandate that would forbid something like a DSP with 32-bit int but a 16-bit address space from making size_t an unsigned short whose values are all representable as int. In that case, if the left-hand operator were (size_t)-1, it would get promoted to an int value 65535, which would compare unequal to -1.Backstay
@Backstay Indeed; another straightforward example of the same thing: given unsigned char uc = -1, uc == -1 fails. uc is 255 (on many platforms) which promotes to int type. We have an int == int comparison between 255 and -1, which fails. Comparing -1 to unsigned has pitfalls if they can be typedef-d to small types which promote to int.Functional
D
1

There were right significant answers, but I would like to make some clarifications. Unfortunately, a test example was formed incorrectly. We can write this way:

void F1()
{
  size_t i = 0;
  std::wstring s;
  s = (i < 0)   ? L"ABC" : L"DEF";
  s = (i != -1) ? L"ABC" : L"DEF";
}

In such case the analyzer will issue two V547 warnings:

  • V547 Expression 'i < 0' is always false. Unsigned type value is never < 0. consoleapplication1.cpp 15
  • V547 Expression 'i != - 1' is always true. consoleapplication1.cpp 16

(V519 will also take place, but it does not relate to the issue.)

So, the first V547 warning is print because the unsigned variable cannot be less than zero. It also doesn’t matter what value the variable has. The second warning is issued because the analyzer reacts that 0 is assigned to the variable i and this variable does not change anywhere.

Now let's write another test example so that the analyzer knew nothing about the value of the variable i:

void F2(size_t i)
{
  std::wstring s;
  s = (i < 0)   ? L"ABC" : L"DEF";
  s = (i != -1) ? L"ABC" : L"DEF";
}

Now there will be only one V547 warning:

  • V547 Expression 'i < 0' is always false. Unsigned type value is never < 0. consoleapplication1.cpp 22

The analyzer can say nothing about the (i != -1) condition. It is perfectly normal and it can be, for example, the comparison with npos, as have already noticed.

I wrote this in case if someone decides to test a source example using PVS-Studio, taking it out of the question. This person will be surprised, when he sees two warnings, though it is discussed that there will be only one.

Deprave answered 1/8, 2017 at 14:4 Comment(3)
Thanks for the answer. I would argue, that phrase "test example was formed incorrectly" is unfortunate. There is nothing incorrect in the code snippet from my question. It might be not sufficient example to understand the PVS-Studio behaviour, but it does not mean it is incorrect.Pep
Perhaps, my answer was too tough. English is not my native language, so, I can use sometimes use not very proper words. I just wanted to note that an example and a question can confuse a programmer.Deprave
AndreyKarpov Understood. Same here. Your notes are perfectly valid. ThanksPep

© 2022 - 2024 — McMap. All rights reserved.