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

[Richard Smith]

  This essentially looks fine to me; a couple of possible tweaks below.

  Regarding Arthur O'Dwyer's comment, has your testing found any cases of the form

      char c = ~0x1f | x;

  (and were they bugs or not)? I'm conflicted on this: it's a questionable way of writing that expression, but it doesn't seem to clearly signal a bug.


================
Comment at: lib/Sema/SemaChecking.cpp:4834-4836
@@ +4833,5 @@
+// problematic sub-expression if it is a strict subset of E.
+static bool OperandMightImplicitlyNarrow(ASTContext &Ctx, llvm::APSInt &Value,
+                                         Expr *E, IntRange TargetRange,
+                                         Expr **NarrowedExpr) {
+  BinaryOperator *BO = dyn_cast<BinaryOperator>(E);
----------------
Maybe this should recurse on operands that are themselves binary operators? Consider a case like:

    enum E { K = 0x200 };
    extern char c1, c2;
    char c3 = K | c1 | c2;

It'd be nice to use an on-by-default warning for this case, as we will if K is the last term.

================
Comment at: lib/Sema/SemaChecking.cpp:5064-5065
@@ +5063,4 @@
+  // narrowing-prone binary operation with a constant.
+  if (OperandMightImplicitlyNarrow(S.Context, Value, E, TargetRange,
+                                   &NarrowedExpr)) {
+    std::string PrettySourceValue = Value.toString(10);
----------------
Can you guard this case by the `SourceRange.Width > TargetRange.Width` check? That'd allow us to avoid the extra `EvalAsInt` calls in cases which we've already determined can't narrow. I think that would change our results in some corner cases:

    char c1 = 0x1234 ^ 0x1256;
    // and particularly...
    char c2 = 0x1234 & 0x7f;

... would no longer warn. I think that's an improvement.


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