-Wconversion does not produce warnings when it should

Hi,

consider the following code:

uint16_t testa4(uint8_t x, uint8_t y) {
    return ( (uint16_t)x << 0 )
            > ( (uint16_t)y << 8 );
}
uint16_t testb3(uint16_t x, uint16_t y) {
    return x|y;
}
uint16_t testb4(uint16_t x, uint16_t y) {
    return x+y;
}
int testb4x(uint16_t x, uint16_t y) {
    return x+y;
}

I'm using clang 11.0.0 on Intel x86_64 and when I compile the above code
with -Wconversion enabled, then I see only one warning, namely for testa4.

However, according to the C standard, the expression after the return
statement has type int in all 4 cases. So why is the warning not
triggered for testb3 and testb4?

In fact, it's convenient that testb3 does not trigger a warning. Yes, x
and y are promoted to int before the bitwise or is performed, but the
result indeed fits into an uint16_t, regardless of x and y. So if clang
is smart and keeps track of the range of the expressions, then it is
understandable that there is no warning for testb3. However, then the
"keeping track of ranges" feature seems to fail for testa4, because
there a warning is indeed produced even though the two operands of the
bitwise-or are both uint16_t.

However, the case of testb4 is different. If we set x an y to 0xFFFF,
then the result will be 0x1FFFE. You can confirm that this is the case
by running testb4x. Then, shouldn't testb4 trigger a warning? The result
is clearly and int _and_ the result may not fit into an uint16_t.

I'm lost. As far as I understand, -Wconversion should issue a warning
when an int is converted to uint16_t without an explicit cast. For all I
know, x+y and x|y are ints, yet there is no warning in testb3 and
testb4. This seems like a bug to me.

Can you elaborate what is happening here?

Kind Regards,
  Sven

Hi,

even though the two operands of the bitwise-or are both uint16_t.

That's not true. I had many more versions of the test code before I
posted here and I got confused. In testa4, the operands of the
bitwise-or are both ints due to the bitshift which performs and integer
promotion.

Kind Regards,
  Sven

I believe/would guess the goal is to reduce false positives (where a
false positive is defined as "warning on code where the code does what
the author intended").

In the taste of testb4 this code is "as good as" the unsigned int
equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
- wraparound occurs in both cases, so the code perhaps isn't too
surprising/the semantics of widening/narrowing conversions don't
really impact the behavior. The uint16 version of the code and the
unsigned int version of the code would appear to the user to work in
the same way.

But, yeah, once the expressions are sufficiently complicated, as in
testa4, clang's attempt to silence benign cases of the warning gives
up and you get warnings. It's best-effort sort of stuff.

Can you elaborate what is happening here?

I believe/would guess the goal is to reduce false positives (where a
false positive is defined as "warning on code where the code does what
the author intended").

In the taste of testb4 this code is "as good as" the unsigned int
equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
- wraparound occurs in both cases, so the code perhaps isn't too
surprising/the semantics of widening/narrowing conversions don't
really impact the behavior.

I do appreciate your explanation. But I do not appreciate the compiler
guessing that wrap around is OK.

The C standard could also be interpreted as "wrap around only occurs
with types of rank equal to (unsigned) int and above" as all other types
are promoted to int prior to any arithmetic.
(My guess for the origin of this promotion would be that int is the
natural word size of the underlying CPU.)

With most modern architectures, int is 32 bit which is a reasonable size
for wraparounds. Unfortunately, int doesn't always have a fixed size.

The uint16 version of the code and the
unsigned int version of the code would appear to the user to work in
the same way.

I would appreciate that only if my understanding of the C language was
that arithmetic on two uint16_t operands is performed with uint16_t
precision. But that is simply false. It is performed with 32 bit
precision (if int is 32 bit).

But, yeah, once the expressions are sufficiently complicated, as in
testa4, clang's attempt to silence benign cases of the warning gives
up and you get warnings. It's best-effort sort of stuff.

I would vote for -Wconversion=strict or -Wimplicit-int-conversion=strict
where we get a warning for every int conversion that loses precision.

I would be very happy if there was a clang option to generate warnings
in such cases.

In general, Clang warnings are designed to have very low false
positive rates so they're more likely to be enabled by users even on
existing codebases (eg: where the work to cleanup existing violations
of the warning is fruitful in terms of fixing bugs rather than only
suppressing a warning where it doesn't indicate any bug was present) -
so it may not be especially desirable to have such a strict form of
the warning in clang. Not to say "certainly not", but that it might be
a bit trickier to motivate/justify.

Might be that something more like a clang-tidy check could be more
suited to your needs.

- Dave

If it hasn’t already been mentioned in this thread, people should be made aware of “Value Sanitizer”.
(Which, curiously, doesn’t seem to be in the clang docs)

Anyway, it checks (at runtime), every time a value is assigned from a larger integral type to a smaller one.
If the value changes, then it raises an error.

So if you have an int with the value 23, and assign it to a char, nothing happens.

So if you have an int with the value 523, and assign it to a char, you get an error.

— Marshall