Why am I being told about a signed/unsigned comparison, and why only sometimes, and how can I fix it?
A customer was using the Windows Test Authoring and Execution Framework (TAEF), and they found that this line of code compiled successfully most of the time:
std::vector<int> values = CalculateValues(); VERIFY_ARE_EQUAL(values.size(), 0); // warning C4389: '==': signed/unsigned mismatch
The VERIFY_ macro compares its two parameters and reports a test failure if they are not equal. The customer found that the above code compiled okay for x86-64, but produced the indicated error when compiled for x86-32. What’s going on?
The VERIFY_ macro passes its parameters onward to a helper function, which in turn passes the parameters to another helper function, which in turn passes the parameters to yet another helper function, which looks like this:
template<typename T1, typename T2>
static bool AreEqual(const T1& expected, const T2& actual)
{
// != 0 to handle overloads of operator==
// that return BOOL instead of bool
return (expected == actual) != 0;
}
The types of the parameters are deduced as std:: and int. The first parameter is a std::, because that’s what the vector:: method returns. The second parameter is a int because that’s what 0 is.
Therefore, the compiler warning of an unsigned/signed comparison is valid. You are comparing an unsigned value (the size of the vector) against a signed value (the integer zero). The warning is present because the rules for an unsigned/signed comparison is to convert the signed value to unsigned, and then compare the two unsigned values. This is different from the mathematical result.
unsigned int v1 = 4294967295;
int v2 = -1;
if (v1 == v2) { /* true */ }
if (v1 > v2) { /* false */ }
Both results disagree with the mathematical expectations. 4294967295 ≠−1, so mathematically, the first test should fail. And 4294967295 > −1, so mathematically, the second test should succeed. What’s happening is that the value −1 is converted from a signed integer to unsigned, and that means that it becomes 4294967295. (The rule for signed-to-unsigned conversion is that negative numbers become the positive equivalent modulo 1 << bit_size.)
You and I can see that this discrepancy doesn’t apply here because the signed integer being compared against is the literal value zero, which is not negative.
What’s happening is that the back-end applies different degrees of inlining based on optimization levels and the target architecture. Higher optimization levels will consider higher degrees of inlining, and some architectures may be more conducive to inlining than others, depending on things like register pressure, the complexity of the calling convention, or other factors.
If the back-end ends up doing enough inlining that the constant 0 gets inlined into the == operator, then the compiler realizes, “Oh, I would normally complain about a signed/unsigned mismatch, but I can see that the value 0 is not negative, so I will suppress the warning because it doesn’t apply to this case.” On the other hand, if the back-end doesn’t inline aggressively enough, it won’t propagate the constant deep enough to realize that it’s never negative, and you get the warning.
The customer offered this solution but complained that it was quite unwieldy:
VERIFY_ARE_EQUAL(values.size(), (std::vector<int>::size_type)0);
That will work, but so too will casting to any other unsigned type, because the compiler is not insisting that the second parameter’s type match the first parameter’s type exactly. It just wants them to agree on signedness. You could pass a size_t or even a uint8_t; as long as it’s unsigned. And probably the most convenient way to indicate an unsigned zero is to append a U to the literal.
VERIFY_ARE_EQUAL(values.size(), 0U);
Light
Dark
6 comments
If they thought
was too unwieldy they’re going to really hate my suggestion of
Hey, at least you didn’t go for the slightly longer…
Those extended C++ casts are a perfect anti-example of “make it easy to use and people will use it”. A bit too much of C++/STL went into code “elegance” and totally missed the convenience part. If it’s unwieldy people will avoid it.
For constants casting is actually wrong, the suffix has to be used to avoid out-of-range errors.
Trouble is that people seem not to know about many of them.
The C++ variable casts are long for a reason – you read code more than write it, and the difference between each cast is quite large so you really need to be able to read it and know what’s going on. static_cast et al are pronounceable and searchable.
Yes, it makes some edge cases feel a bit unwieldy, but in many cases that is also a hint that it’s not actually the right solution – like the numeric constant suffixes.
They are unwieldy to be sure, but C-style casts (also used in C# and Java) are even worse because they require additional parentheses around the whole expression to access object’s members which pisses me off every time I need to add them.
I prefer this:
Over this:
It’s meant to be annoying to use. Casts are a code smell.