[PATCH] Add a truncation warning for values under a bitwise or

Sam Panzer espanz at gmail.com
Thu Feb 21 13:29:18 PST 2013



================
Comment at: test/Sema/constant-conversion.c:92
@@ +91,3 @@
+  unsigned char y = 0;
+	x = y | 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+	x = y | 0xff; // no-warning
----------------
Dmitri Gribenko wrote:
> The message in all these cases is a bit misleading.  It suggest that we do the computation as `x = y | char(0x1ff)`, while in reality it is `x = char(int(x) | 0x1ff)`.
> 
> Do you think this is an issue?  Is there a better wording?
> 
Yes, that's been bothering me too. I do think that bitwise-or commutes with width conversions for unsigned values, though.

How about something like "implicit conversion from 'int' to 'char' may change value of [binary operation]: value of [operand] would be changed to [value] if converted before operation (add an explicit cast to suppress this warning)"? It's long, but less misleading.

Chances are, though, that most of these are bugs, even though there is some valid code that can trigger this warning. Consider this (I think) valid code:

```int16_t x = foo(); // x guaranteed to be between 0xffff and 0xff00
int8_t y = x - 0xff00;```

I would expect that it's nearly always an error for bitwise-or and multiplication, since these operations can't //decrease// the width of any operand barring undefined behavior.


================
Comment at: lib/Sema/SemaChecking.cpp:4789
@@ +4788,3 @@
+
+  switch (Bop->getOpcode()) {
+    case BO_Or:
----------------
Any thoughts if this might be worthy of a warning (bitwise-and with a too-wide value that only has ones in bits that will be thrown away)?

```int8_t x = foo();
x &= 0xff00; // bug? x = 0
```


http://llvm-reviews.chandlerc.com/D405



More information about the cfe-commits mailing list