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

[Sam Panzer]

  Added the new test according to Dmitri's suggestion.

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D405?vs=975&id=983#toc

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/constant-conversion.c

Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -4778,6 +4778,18 @@
   }
 }
 
+// Determine if E is a bitwise or expression with a constant on either side.
+static bool IsBitwiseOrWithConstant(Sema &S, llvm::APSInt &Value, Expr *E) {
+  E = E->IgnoreImpCasts();
+  if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+    if (BO->getOpcode() != BO_Or)
+      return false;
+    return BO->getLHS()->isIntegerConstantExpr(Value, S.Context) ||
+      BO->getRHS()->isIntegerConstantExpr(Value, S.Context);
+  }
+  return false;
+}
+
 void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
                              SourceLocation CC, bool *ICContext = 0) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
@@ -4962,9 +4974,12 @@
 
   if (SourceRange.Width > TargetRange.Width) {
     // If the source is a constant, use a default-on diagnostic.
+    // Also use a default-on diagnostic if the source is bitwise-or'ed with
+    // a constant.
     // TODO: this should happen for bitfield stores, too.
     llvm::APSInt Value(32);
-    if (E->isIntegerConstantExpr(Value, S.Context)) {
+    if (E->isIntegerConstantExpr(Value, S.Context) ||
+        IsBitwiseOrWithConstant(S, Value, E)) {
       if (S.SourceMgr.isInSystemMacro(CC))
         return;
 
@@ -5114,6 +5129,19 @@
   if (E->getType() != T)
     CheckImplicitConversion(S, E, T, CC);
 
+  // For CompoundAssignmentOperators, check the conversion from the computed
+  // LHS type to the type of the assignee.
+  if (CompoundAssignOperator *CAO = dyn_cast<CompoundAssignOperator>(E)) {
+    QualType LHST = CAO->getComputationLHSType();
+    // This CheckImplicitConversion would trigger on NULLs, though their
+    // warnings should be trigger elsewhere. We filter those out here.
+    if (CAO->getOpcode() == BO_OrAssign && LHST != T &&
+        CAO->getRHS()->isNullPointerConstant(S.Context,
+                                             Expr::NPC_ValueDependentIsNotNull)
+        != Expr::NPCK_GNUNull)
+      CheckImplicitConversion(S, CAO->getRHS(), T, CC);
+  }
+
   // Now continue drilling into this expression.
 
   // Skip past explicit casts.
@@ -5128,7 +5156,7 @@
       return AnalyzeComparison(S, BO);
 
     // And with simple assignments.
-    if (BO->getOpcode() == BO_Assign)
+    if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_OrAssign)
       return AnalyzeAssignment(S, BO);
   }
 
Index: test/Sema/constant-conversion.c
===================================================================
--- test/Sema/constant-conversion.c
+++ test/Sema/constant-conversion.c
@@ -66,17 +66,31 @@
 	struct {
 		unsigned int twoBits1:2;
 		unsigned int twoBits2:2;
-		unsigned int reserved:28;
+		unsigned int twoBits3:2;
+		unsigned int reserved:26;
 	} f;
 
 	f.twoBits1 = ~1; // expected-warning {{implicit truncation from 'int' to bitfield changes value from -2 to 2}}
 	f.twoBits2 = ~2; // expected-warning {{implicit truncation from 'int' to bitfield changes value from -3 to 1}}
 	f.twoBits1 &= ~1; // no-warning
 	f.twoBits2 &= ~2; // no-warning
+	f.twoBits3 |= 4; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 4 to 0}}
+	f.twoBits3 |= 1; // no-warning
 }
 
 void test8() {
   enum E { A, B, C };
   struct { enum E x : 1; } f;
   f.x = C; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 2 to 0}}
 }
+
+void test9() {
+  unsigned char x = 0;
+  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
+	x = 0x1ff | y; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+	x = 0xff | y; // no-warning
+	x = (y | 0x1ff); // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+	x = (y | 0xff); // no-warning
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D405.3.patch
Type: text/x-patch
Size: 4030 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130214/cd2d0def/attachment.bin>