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

[Sam Panzer]

  I think this version captures the correct semantics for bitwise and in addition to the other operations. This was interesting, since GetExprRange would return the size of its smallest operand, when I wanted to check the size of the larger one.


================
Comment at: lib/Sema/SemaChecking.cpp:5016
@@ -4966,2 +5015,3 @@
     llvm::APSInt Value(32);
-    if (E->isIntegerConstantExpr(Value, S.Context)) {
+    if (E->isIntegerConstantExpr(Value, S.Context) ||
+        OperandMightImplicitlyNarrow(S, Value, E)) {
----------------
Richard Smith wrote:
> Use EvaluateAsInt here too.
Done.

================
Comment at: lib/Sema/SemaChecking.cpp:5204-5206
@@ -5129,4 +5203,5 @@
 
-    // And with simple assignments.
-    if (BO->getOpcode() == BO_Assign)
+    // And with simple assignments. We also do a special check for bitwise or
+    // assignments to catch overflow in bitfields.
+    if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_OrAssign)
       return AnalyzeAssignment(S, BO);
----------------
Richard Smith wrote:
> Should this also extend to the other BO_*Assign cases you handle above?
Yes. It turns out that BO->isAssignmentOp() is a better condition, since it's filtered later.

================
Comment at: lib/Sema/SemaChecking.cpp:4789
@@ +4788,3 @@
+
+  switch (Bop->getOpcode()) {
+    case BO_Or:
----------------
Richard Smith wrote:
> Sam Panzer wrote:
> > 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
> > ```
> Yes, that certainly looks suspicious, and probably worth warning on. I suppose you're only considering the 'always zero' case to avoid warning on this sort of thing:
> 
>   x &= ~0x80;
Right. I'm not sure if 'always zero' or 'has non-leading ones' will do a better job with false/actual positive rate. I implemented the latter for bitwise and.

================
Comment at: lib/Sema/SemaChecking.cpp:4785
@@ +4784,3 @@
+                                         Expr *E) {
+  BinaryOperator *Bop = dyn_cast<BinaryOperator>(E);
+  if (!Bop)
----------------
Richard Smith wrote:
> We'd usually call this BO.
Renamed.


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